Hi Al, Steve Grubb wrote: [Thu Feb 09 2006, 10:09:16AM EST] Looks good, except in 3rd patch fs/inotify.c should have: @@ -628,7 +628,7 @@ s32 inotify_add_watch(struct inotify_dev mask_add = 1; /* don't let user-space set invalid bits: we don't want flags set */ - mask &= IN_ALL_EVENTS; + mask &= IN_ALL_EVENTS | IN_ONESHOT; if (unlikely(!mask)) { ret = -EINVAL; goto out; Here's the full diff with this change: diff --git a/fs/inotify.c b/fs/inotify.c index 3041503..ab57df6 100644 --- a/fs/inotify.c +++ b/fs/inotify.c @@ -85,14 +85,18 @@ struct inotify_device { wait_queue_head_t wq; /* wait queue for i/o */ struct idr idr; /* idr mapping wd -> watch */ struct semaphore sem; /* protects this bad boy */ - struct list_head events; /* list of queued events */ struct list_head watches; /* list of watches */ atomic_t count; /* reference count */ + u32 last_wd; /* the last wd allocated */ + /* userland consumer API */ + struct list_head events; /* list of queued events */ struct user_struct *user; /* user who opened this dev */ unsigned int queue_size; /* size of the queue (bytes) */ unsigned int event_count; /* number of pending events */ unsigned int max_events; /* maximum number of events */ - u32 last_wd; /* the last wd allocated */ + /* kernel consumer API */ + void (*callback)(struct inotify_event *, const char *, struct inode *, + void *); /* event callback */ }; /* @@ -124,6 +128,7 @@ struct inotify_watch { struct inode *inode; /* associated inode */ s32 wd; /* watch descriptor */ u32 mask; /* event mask for this watch */ + void *callback_arg; /* callback argument - kernel API */ }; #ifdef CONFIG_SYSCTL @@ -175,8 +180,10 @@ static inline void get_inotify_dev(struc static inline void put_inotify_dev(struct inotify_device *dev) { if (atomic_dec_and_test(&dev->count)) { - atomic_dec(&dev->user->inotify_devs); - free_uid(dev->user); + if (dev->user) { + atomic_dec(&dev->user->inotify_devs); + free_uid(dev->user); + } idr_destroy(&dev->idr); kfree(dev); } @@ -344,6 +351,24 @@ static void inotify_dev_event_dequeue(st } /* + * inotify_callback_event - notify kernel consumers of events + */ +static void inotify_callback_event(struct inotify_device *dev, + struct inotify_watch *watch, + u32 mask, u32 cookie, const char *name, + struct inode *inode) +{ + struct inotify_event event; + + event.wd = watch->wd; + event.mask = mask; + event.cookie = cookie; + event.len = 0; /* kernel consumers don't need length */ + + dev->callback(&event, name, inode, watch->callback_arg); +} + +/* * inotify_dev_get_wd - returns the next WD for use by the given dev * * Callers must hold dev->sem. This function can sleep. @@ -387,12 +412,13 @@ static int find_inode(const char __user * Both 'dev' and 'inode' (by way of nameidata) need to be pinned. */ static struct inotify_watch *create_watch(struct inotify_device *dev, - u32 mask, struct inode *inode) + u32 mask, struct inode *inode, + void *callback_arg) { struct inotify_watch *watch; int ret; - if (atomic_read(&dev->user->inotify_watches) >= + if (dev->user && atomic_read(&dev->user->inotify_watches) >= inotify_max_user_watches) return ERR_PTR(-ENOSPC); @@ -408,6 +434,7 @@ static struct inotify_watch *create_watc dev->last_wd = watch->wd; watch->mask = mask; + watch->callback_arg = callback_arg; atomic_set(&watch->count, 0); INIT_LIST_HEAD(&watch->d_list); INIT_LIST_HEAD(&watch->i_list); @@ -425,7 +452,8 @@ static struct inotify_watch *create_watc /* bump our own count, corresponding to our entry in dev->watches */ get_inotify_watch(watch); - atomic_inc(&dev->user->inotify_watches); + if (dev->user) + atomic_inc(&dev->user->inotify_watches); atomic_inc(&inotify_watches); return watch; @@ -458,7 +486,8 @@ static void remove_watch_no_event(struct list_del(&watch->i_list); list_del(&watch->d_list); - atomic_dec(&dev->user->inotify_watches); + if (dev->user) + atomic_dec(&dev->user->inotify_watches); atomic_dec(&inotify_watches); idr_remove(&dev->idr, watch->wd); put_inotify_watch(watch); @@ -477,7 +506,10 @@ static void remove_watch_no_event(struct */ static void remove_watch(struct inotify_watch *watch,struct inotify_device *dev) { - inotify_dev_queue_event(dev, watch, IN_IGNORED, 0, NULL); + if (dev->callback) + inotify_callback_event(dev, watch, IN_IGNORED, 0, NULL, NULL); + else + inotify_dev_queue_event(dev, watch, IN_IGNORED, 0, NULL); remove_watch_no_event(watch, dev); } @@ -490,7 +522,190 @@ static inline int inotify_inode_watched( return !list_empty(&inode->inotify_watches); } -/* Kernel API */ +/* Kernel consumer API */ + +/** + * inotify_init - allocates and initializes an inotify device + * @callback: kernel consumer's event callback + */ +struct inotify_device *inotify_init(void (*callback)(struct inotify_event *, + const char *, + struct inode *, void *)) +{ + struct inotify_device *dev; + + dev = kmalloc(sizeof(struct inotify_device), GFP_KERNEL); + if (unlikely(!dev)) + return NULL; + + idr_init(&dev->idr); + INIT_LIST_HEAD(&dev->events); + INIT_LIST_HEAD(&dev->watches); + init_waitqueue_head(&dev->wq); + sema_init(&dev->sem, 1); + dev->event_count = 0; + dev->queue_size = 0; + dev->max_events = inotify_max_queued_events; + dev->user = NULL; /* set in sys_inotify_init */ + dev->last_wd = 0; + dev->callback = callback; + atomic_set(&dev->count, 0); + get_inotify_dev(dev); + + return dev; +} +EXPORT_SYMBOL_GPL(inotify_init); + +/** + * inotify_free - clean up and free an inotify device + * @dev: inotify device to free + */ +int inotify_free(struct inotify_device *dev) +{ + /* + * Destroy all of the watches on this device. Unfortunately, not very + * pretty. We cannot do a simple iteration over the list, because we + * do not know the inode until we iterate to the watch. But we need to + * hold inode->inotify_sem before dev->sem. The following works. + */ + while (1) { + struct inotify_watch *watch; + struct list_head *watches; + struct inode *inode; + + down(&dev->sem); + watches = &dev->watches; + if (list_empty(watches)) { + up(&dev->sem); + break; + } + watch = list_entry(watches->next, struct inotify_watch, d_list); + get_inotify_watch(watch); + up(&dev->sem); + + inode = watch->inode; + down(&inode->inotify_sem); + down(&dev->sem); + remove_watch_no_event(watch, dev); + up(&dev->sem); + up(&inode->inotify_sem); + put_inotify_watch(watch); + } + + /* destroy all of the events on this device */ + down(&dev->sem); + while (!list_empty(&dev->events)) + inotify_dev_event_dequeue(dev); + up(&dev->sem); + + /* free this device: the put matching the get in inotify_init() */ + put_inotify_dev(dev); + + return 0; +} +EXPORT_SYMBOL_GPL(inotify_free); + +/** + * inotify_inotify_add_watch - add a watch to this inotify device + * @dev: inotify device + * @inode: inode to watch for events + * @mask: filesystem event mask + * @callback_arg - ptr to data that kernel consumer associates with this watch + * + * Caller must pin the inode in question, e.g. by calling path_lookup. + */ +s32 inotify_add_watch(struct inotify_device *dev, struct inode *inode, + u32 mask, void *callback_arg) +{ + int mask_add = 0; + struct inotify_watch *watch, *old; + int ret; + + down(&inode->inotify_sem); + down(&dev->sem); + + if (mask & IN_MASK_ADD) + mask_add = 1; + + /* don't let user-space set invalid bits: we don't want flags set */ + mask &= IN_ALL_EVENTS | IN_ONESHOT; + if (unlikely(!mask)) { + ret = -EINVAL; + goto out; + } + + /* + * Handle the case of re-adding a watch on an (inode,dev) pair that we + * are already watching. We just update the mask and callback_arg and + * return its wd. + */ + old = inode_find_dev(inode, dev); + if (unlikely(old)) { + if (mask_add) + old->mask |= mask; + else + old->mask = mask; + old->callback_arg = callback_arg; + ret = old->wd; + goto out; + } + + watch = create_watch(dev, mask, inode, callback_arg); + if (unlikely(IS_ERR(watch))) { + ret = PTR_ERR(watch); + goto out; + } + + /* Add the watch to the device's and the inode's list */ + list_add(&watch->d_list, &dev->watches); + list_add(&watch->i_list, &inode->inotify_watches); + ret = watch->wd; + +out: + up(&dev->sem); + up(&inode->inotify_sem); + return ret; +} +EXPORT_SYMBOL_GPL(inotify_add_watch); + +/** + * inotify_ignore - remove a given wd from this inotify device + * @dev: inotify device + * @wd: watch descriptor to remove + */ +int inotify_ignore(struct inotify_device *dev, s32 wd) +{ + struct inotify_watch *watch; + struct inode *inode; + + down(&dev->sem); + watch = idr_find(&dev->idr, wd); + if (unlikely(!watch)) { + up(&dev->sem); + return -EINVAL; + } + get_inotify_watch(watch); + inode = watch->inode; + up(&dev->sem); + + down(&inode->inotify_sem); + down(&dev->sem); + + /* make sure that we did not race */ + watch = idr_find(&dev->idr, wd); + if (likely(watch)) + remove_watch(watch, dev); + + up(&dev->sem); + up(&inode->inotify_sem); + put_inotify_watch(watch); + + return 0; + +} +EXPORT_SYMBOL_GPL(inotify_ignore); + +/* Kernel producer API */ /** * inotify_inode_queue_event - queue an event to all watches on this inode @@ -498,9 +713,10 @@ static inline int inotify_inode_watched( * @mask: event mask describing this event * @cookie: cookie for synchronization, or zero * @name: filename, if any + * @cinode: child inode, used for events on directories */ void inotify_inode_queue_event(struct inode *inode, u32 mask, u32 cookie, - const char *name) + const char *name, struct inode *cinode) { struct inotify_watch *watch, *next; @@ -514,7 +730,12 @@ void inotify_inode_queue_event(struct in struct inotify_device *dev = watch->dev; get_inotify_watch(watch); down(&dev->sem); - inotify_dev_queue_event(dev, watch, mask, cookie, name); + if (dev->callback) + inotify_callback_event(dev, watch, mask, + cookie, name, cinode); + else + inotify_dev_queue_event(dev, watch, mask, + cookie, name); if (watch_mask & IN_ONESHOT) remove_watch_no_event(watch, dev); up(&dev->sem); @@ -548,7 +769,8 @@ void inotify_dentry_parent_queue_event(s if (inotify_inode_watched(inode)) { dget(parent); spin_unlock(&dentry->d_lock); - inotify_inode_queue_event(inode, mask, cookie, name); + inotify_inode_queue_event(inode, mask, cookie, name, + dentry->d_inode); dput(parent); } else spin_unlock(&dentry->d_lock); @@ -631,7 +853,12 @@ void inotify_unmount_inodes(struct list_ list_for_each_entry_safe(watch, next_w, watches, i_list) { struct inotify_device *dev = watch->dev; down(&dev->sem); - inotify_dev_queue_event(dev, watch, IN_UNMOUNT,0,NULL); + if (dev->callback) + inotify_callback_event(dev, watch, IN_UNMOUNT, + 0, NULL, NULL); + else + inotify_dev_queue_event(dev, watch, IN_UNMOUNT, + 0, NULL); remove_watch(watch, dev); up(&dev->sem); } @@ -757,83 +984,7 @@ static ssize_t inotify_read(struct file static int inotify_release(struct inode *ignored, struct file *file) { - struct inotify_device *dev = file->private_data; - - /* - * Destroy all of the watches on this device. Unfortunately, not very - * pretty. We cannot do a simple iteration over the list, because we - * do not know the inode until we iterate to the watch. But we need to - * hold inode->inotify_sem before dev->sem. The following works. - */ - while (1) { - struct inotify_watch *watch; - struct list_head *watches; - struct inode *inode; - - down(&dev->sem); - watches = &dev->watches; - if (list_empty(watches)) { - up(&dev->sem); - break; - } - watch = list_entry(watches->next, struct inotify_watch, d_list); - get_inotify_watch(watch); - up(&dev->sem); - - inode = watch->inode; - down(&inode->inotify_sem); - down(&dev->sem); - remove_watch_no_event(watch, dev); - up(&dev->sem); - up(&inode->inotify_sem); - put_inotify_watch(watch); - } - - /* destroy all of the events on this device */ - down(&dev->sem); - while (!list_empty(&dev->events)) - inotify_dev_event_dequeue(dev); - up(&dev->sem); - - /* free this device: the put matching the get in inotify_init() */ - put_inotify_dev(dev); - - return 0; -} - -/* - * inotify_ignore - remove a given wd from this inotify instance. - * - * Can sleep. - */ -static int inotify_ignore(struct inotify_device *dev, s32 wd) -{ - struct inotify_watch *watch; - struct inode *inode; - - down(&dev->sem); - watch = idr_find(&dev->idr, wd); - if (unlikely(!watch)) { - up(&dev->sem); - return -EINVAL; - } - get_inotify_watch(watch); - inode = watch->inode; - up(&dev->sem); - - down(&inode->inotify_sem); - down(&dev->sem); - - /* make sure that we did not race */ - watch = idr_find(&dev->idr, wd); - if (likely(watch)) - remove_watch(watch, dev); - - up(&dev->sem); - up(&inode->inotify_sem); - put_inotify_watch(watch); - - return 0; + return inotify_free(file->private_data); } static long inotify_ioctl(struct file *file, unsigned int cmd, @@ -887,12 +1038,15 @@ asmlinkage long sys_inotify_init(void) goto out_free_uid; } - dev = kmalloc(sizeof(struct inotify_device), GFP_KERNEL); + dev = inotify_init(NULL); if (unlikely(!dev)) { ret = -ENOMEM; goto out_free_uid; } + dev->user = user; + atomic_inc(&user->inotify_devs); + filp->f_op = &inotify_fops; filp->f_vfsmnt = mntget(inotify_mnt); filp->f_dentry = dget(inotify_mnt->mnt_root); @@ -901,20 +1055,6 @@ asmlinkage long sys_inotify_init(void) filp->f_flags = O_RDONLY; filp->private_data = dev; - idr_init(&dev->idr); - INIT_LIST_HEAD(&dev->events); - INIT_LIST_HEAD(&dev->watches); - init_waitqueue_head(&dev->wq); - sema_init(&dev->sem, 1); - dev->event_count = 0; - dev->queue_size = 0; - dev->max_events = inotify_max_queued_events; - dev->user = user; - dev->last_wd = 0; - atomic_set(&dev->count, 0); - - get_inotify_dev(dev); - atomic_inc(&user->inotify_devs); fd_install(fd, filp); return fd; @@ -928,13 +1068,11 @@ out_put_fd: asmlinkage long sys_inotify_add_watch(int fd, const char __user *path, u32 mask) { - struct inotify_watch *watch, *old; struct inode *inode; struct inotify_device *dev; struct nameidata nd; struct file *filp; int ret, fput_needed; - int mask_add = 0; unsigned flags = 0; filp = fget_light(fd, &fput_needed); @@ -960,46 +1098,7 @@ asmlinkage long sys_inotify_add_watch(in inode = nd.dentry->d_inode; dev = filp->private_data; - down(&inode->inotify_sem); - down(&dev->sem); - - if (mask & IN_MASK_ADD) - mask_add = 1; - - /* don't let user-space set invalid bits: we don't want flags set */ - mask &= IN_ALL_EVENTS | IN_ONESHOT; - if (unlikely(!mask)) { - ret = -EINVAL; - goto out; - } - - /* - * Handle the case of re-adding a watch on an (inode,dev) pair that we - * are already watching. We just update the mask and return its wd. - */ - old = inode_find_dev(inode, dev); - if (unlikely(old)) { - if (mask_add) - old->mask |= mask; - else - old->mask = mask; - ret = old->wd; - goto out; - } - - watch = create_watch(dev, mask, inode); - if (unlikely(IS_ERR(watch))) { - ret = PTR_ERR(watch); - goto out; - } - - /* Add the watch to the device's and the inode's list */ - list_add(&watch->d_list, &dev->watches); - list_add(&watch->i_list, &inode->inotify_watches); - ret = watch->wd; -out: - up(&dev->sem); - up(&inode->inotify_sem); + ret = inotify_add_watch(dev, inode, mask, NULL); path_release(&nd); fput_and_out: fput_light(filp, fput_needed); diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 94919c3..606b875 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -35,16 +35,18 @@ static inline void fsnotify_move(struct if (isdir) isdir = IN_ISDIR; - inotify_inode_queue_event(old_dir, IN_MOVED_FROM|isdir,cookie,old_name); - inotify_inode_queue_event(new_dir, IN_MOVED_TO|isdir, cookie, new_name); + inotify_inode_queue_event(old_dir, IN_MOVED_FROM|isdir, cookie, + old_name, NULL); + inotify_inode_queue_event(new_dir, IN_MOVED_TO|isdir, cookie, + new_name, source); if (target) { - inotify_inode_queue_event(target, IN_DELETE_SELF, 0, NULL); + inotify_inode_queue_event(target, IN_DELETE_SELF, 0, NULL,NULL); inotify_inode_is_dead(target); } if (source) { - inotify_inode_queue_event(source, IN_MOVE_SELF, 0, NULL); + inotify_inode_queue_event(source, IN_MOVE_SELF, 0, NULL, NULL); } audit_inode_child(old_name, source, old_dir->i_ino); audit_inode_child(new_name, target, new_dir->i_ino); @@ -66,7 +68,7 @@ static inline void fsnotify_nameremove(s */ static inline void fsnotify_inoderemove(struct inode *inode) { - inotify_inode_queue_event(inode, IN_DELETE_SELF, 0, NULL); + inotify_inode_queue_event(inode, IN_DELETE_SELF, 0, NULL, NULL); inotify_inode_is_dead(inode); } @@ -76,7 +78,8 @@ static inline void fsnotify_inoderemove( static inline void fsnotify_create(struct inode *inode, struct dentry *dentry) { inode_dir_notify(inode, DN_CREATE); - inotify_inode_queue_event(inode, IN_CREATE, 0, dentry->d_name.name); + inotify_inode_queue_event(inode, IN_CREATE, 0, dentry->d_name.name, + dentry->d_inode); audit_inode_child(dentry->d_name.name, dentry->d_inode, inode->i_ino); } @@ -87,7 +90,7 @@ static inline void fsnotify_mkdir(struct { inode_dir_notify(inode, DN_CREATE); inotify_inode_queue_event(inode, IN_CREATE | IN_ISDIR, 0, - dentry->d_name.name); + dentry->d_name.name, dentry->d_inode); audit_inode_child(dentry->d_name.name, dentry->d_inode, inode->i_ino); } @@ -104,7 +107,7 @@ static inline void fsnotify_access(struc dnotify_parent(dentry, DN_ACCESS); inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name); - inotify_inode_queue_event(inode, mask, 0, NULL); + inotify_inode_queue_event(inode, mask, 0, NULL, NULL); } /* @@ -120,7 +123,7 @@ static inline void fsnotify_modify(struc dnotify_parent(dentry, DN_MODIFY); inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name); - inotify_inode_queue_event(inode, mask, 0, NULL); + inotify_inode_queue_event(inode, mask, 0, NULL, NULL); } /* @@ -135,7 +138,7 @@ static inline void fsnotify_open(struct mask |= IN_ISDIR; inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name); - inotify_inode_queue_event(inode, mask, 0, NULL); + inotify_inode_queue_event(inode, mask, 0, NULL, NULL); } /* @@ -153,7 +156,7 @@ static inline void fsnotify_close(struct mask |= IN_ISDIR; inotify_dentry_parent_queue_event(dentry, mask, 0, name); - inotify_inode_queue_event(inode, mask, 0, NULL); + inotify_inode_queue_event(inode, mask, 0, NULL, NULL); } /* @@ -168,7 +171,7 @@ static inline void fsnotify_xattr(struct mask |= IN_ISDIR; inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name); - inotify_inode_queue_event(inode, mask, 0, NULL); + inotify_inode_queue_event(inode, mask, 0, NULL, NULL); } /* @@ -215,7 +218,7 @@ static inline void fsnotify_change(struc if (in_mask) { if (S_ISDIR(inode->i_mode)) in_mask |= IN_ISDIR; - inotify_inode_queue_event(inode, in_mask, 0, NULL); + inotify_inode_queue_event(inode, in_mask, 0, NULL, NULL); inotify_dentry_parent_queue_event(dentry, in_mask, 0, dentry->d_name.name); } diff --git a/include/linux/inotify.h b/include/linux/inotify.h index 267c88b..2a8d1bd 100644 --- a/include/linux/inotify.h +++ b/include/linux/inotify.h @@ -14,6 +14,9 @@ * * When you are watching a directory, you will receive the filename for events * such as IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, ..., relative to the wd. + * + * When using inotify from the kernel, len will always be zero. Instead you + * should check the path for non-NULL in your callback. */ struct inotify_event { __s32 wd; /* watch descriptor */ @@ -71,8 +74,19 @@ struct inotify_event { #ifdef CONFIG_INOTIFY +/* Kernel consumer API */ + +extern struct inotify_device *inotify_init(void (*)(struct inotify_event *, + const char *, + struct inode *, void *)); +extern int inotify_free(struct inotify_device *); +extern __s32 inotify_add_watch(struct inotify_device *, struct inode *, __u32, + void *); +extern int inotify_ignore(struct inotify_device *, __s32); + +/* Kernel producer API */ extern void inotify_inode_queue_event(struct inode *, __u32, __u32, - const char *); + const char *, struct inode *); extern void inotify_dentry_parent_queue_event(struct dentry *, __u32, __u32, const char *); extern void inotify_unmount_inodes(struct list_head *); @@ -81,6 +95,7 @@ extern u32 inotify_get_cookie(void); #else +/* Kernel producer API stubs */ static inline void inotify_inode_queue_event(struct inode *inode, __u32 mask, __u32 cookie, const char *filename)