From b3d9b7a3c752dc4b6976a4ff7b8298887a5b734d Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 9 Jun 2012 13:51:19 -0400 Subject: vfs: switch i_dentry/d_alias to hlist Signed-off-by: Al Viro --- fs/notify/fsnotify.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/notify') diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index b39c5c161ad..6baadb5a843 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -52,6 +52,7 @@ void __fsnotify_vfsmount_delete(struct vfsmount *mnt) void __fsnotify_update_child_dentry_flags(struct inode *inode) { struct dentry *alias; + struct hlist_node *p; int watched; if (!S_ISDIR(inode->i_mode)) @@ -63,7 +64,7 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode) spin_lock(&inode->i_lock); /* run all of the dentries associated with this inode. Since this is a * directory, there damn well better only be one item on this list */ - list_for_each_entry(alias, &inode->i_dentry, d_alias) { + hlist_for_each_entry(alias, p, &inode->i_dentry, d_alias) { struct dentry *child; /* run all of the children of the original inode and fix their -- cgit v1.2.3 From 765927b2d508712d320c8934db963bbe14c3fcec Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 26 Jun 2012 21:58:53 +0400 Subject: switch dentry_open() to struct path, make it grab references itself Signed-off-by: Al Viro --- fs/notify/fanotify/fanotify_user.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 3568c8a8b13..d4380366973 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -61,8 +61,6 @@ static struct fsnotify_event *get_one_event(struct fsnotify_group *group, static int create_fd(struct fsnotify_group *group, struct fsnotify_event *event) { int client_fd; - struct dentry *dentry; - struct vfsmount *mnt; struct file *new_file; pr_debug("%s: group=%p event=%p\n", __func__, group, event); @@ -81,12 +79,10 @@ static int create_fd(struct fsnotify_group *group, struct fsnotify_event *event) * we need a new file handle for the userspace program so it can read even if it was * originally opened O_WRONLY. */ - dentry = dget(event->path.dentry); - mnt = mntget(event->path.mnt); /* it's possible this event was an overflow event. in that case dentry and mnt * are NULL; That's fine, just don't call dentry open */ - if (dentry && mnt) - new_file = dentry_open(dentry, mnt, + if (event->path.dentry && event->path.mnt) + new_file = dentry_open(&event->path, group->fanotify_data.f_flags | FMODE_NONOTIFY, current_cred()); else -- cgit v1.2.3 From 352e3b249284235e00745f3e71fc348b913e5deb Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 19 Aug 2012 12:30:45 -0400 Subject: fanotify: sanitize failure exits in copy_event_to_user() * do copy_to_user() before prepare_for_access_response(); that kills the need in remove_access_response(). * don't do fd_install() until we are past the last possible failure exit. Don't use sys_close() on cleanup side - just put_unused_fd() and fput(). Less racy that way... Signed-off-by: Al Viro --- fs/notify/fanotify/fanotify_user.c | 59 +++++++++++++------------------------- 1 file changed, 20 insertions(+), 39 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index d4380366973..ea48693940f 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -58,7 +58,9 @@ static struct fsnotify_event *get_one_event(struct fsnotify_group *group, return fsnotify_remove_notify_event(group); } -static int create_fd(struct fsnotify_group *group, struct fsnotify_event *event) +static int create_fd(struct fsnotify_group *group, + struct fsnotify_event *event, + struct file **file) { int client_fd; struct file *new_file; @@ -98,7 +100,7 @@ static int create_fd(struct fsnotify_group *group, struct fsnotify_event *event) put_unused_fd(client_fd); client_fd = PTR_ERR(new_file); } else { - fd_install(client_fd, new_file); + *file = new_file; } return client_fd; @@ -106,13 +108,15 @@ static int create_fd(struct fsnotify_group *group, struct fsnotify_event *event) static int fill_event_metadata(struct fsnotify_group *group, struct fanotify_event_metadata *metadata, - struct fsnotify_event *event) + struct fsnotify_event *event, + struct file **file) { int ret = 0; pr_debug("%s: group=%p metadata=%p event=%p\n", __func__, group, metadata, event); + *file = NULL; metadata->event_len = FAN_EVENT_METADATA_LEN; metadata->metadata_len = FAN_EVENT_METADATA_LEN; metadata->vers = FANOTIFY_METADATA_VERSION; @@ -121,7 +125,7 @@ static int fill_event_metadata(struct fsnotify_group *group, if (unlikely(event->mask & FAN_Q_OVERFLOW)) metadata->fd = FAN_NOFD; else { - metadata->fd = create_fd(group, event); + metadata->fd = create_fd(group, event, file); if (metadata->fd < 0) ret = metadata->fd; } @@ -220,25 +224,6 @@ static int prepare_for_access_response(struct fsnotify_group *group, return 0; } -static void remove_access_response(struct fsnotify_group *group, - struct fsnotify_event *event, - __s32 fd) -{ - struct fanotify_response_event *re; - - if (!(event->mask & FAN_ALL_PERM_EVENTS)) - return; - - re = dequeue_re(group, fd); - if (!re) - return; - - BUG_ON(re->event != event); - - kmem_cache_free(fanotify_response_event_cache, re); - - return; -} #else static int prepare_for_access_response(struct fsnotify_group *group, struct fsnotify_event *event, @@ -247,12 +232,6 @@ static int prepare_for_access_response(struct fsnotify_group *group, return 0; } -static void remove_access_response(struct fsnotify_group *group, - struct fsnotify_event *event, - __s32 fd) -{ - return; -} #endif static ssize_t copy_event_to_user(struct fsnotify_group *group, @@ -260,31 +239,33 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group, char __user *buf) { struct fanotify_event_metadata fanotify_event_metadata; + struct file *f; int fd, ret; pr_debug("%s: group=%p event=%p\n", __func__, group, event); - ret = fill_event_metadata(group, &fanotify_event_metadata, event); + ret = fill_event_metadata(group, &fanotify_event_metadata, event, &f); if (ret < 0) goto out; fd = fanotify_event_metadata.fd; - ret = prepare_for_access_response(group, event, fd); - if (ret) - goto out_close_fd; - ret = -EFAULT; if (copy_to_user(buf, &fanotify_event_metadata, fanotify_event_metadata.event_len)) - goto out_kill_access_response; + goto out_close_fd; + + ret = prepare_for_access_response(group, event, fd); + if (ret) + goto out_close_fd; + fd_install(fd, f); return fanotify_event_metadata.event_len; -out_kill_access_response: - remove_access_response(group, event, fd); out_close_fd: - if (fd != FAN_NOFD) - sys_close(fd); + if (fd != FAN_NOFD) { + put_unused_fd(fd); + fput(f); + } out: #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS if (event->mask & FAN_ALL_PERM_EVENTS) { -- cgit v1.2.3 From 2903ff019b346ab8d36ebbf54853c3aaf6590608 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 28 Aug 2012 12:52:22 -0400 Subject: switch simple cases of fget_light to fdget Signed-off-by: Al Viro --- fs/notify/fanotify/fanotify_user.c | 28 +++++++++++++--------------- fs/notify/inotify/inotify_user.c | 28 ++++++++++++++-------------- 2 files changed, 27 insertions(+), 29 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index ea48693940f..721d692fa8d 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -451,24 +451,22 @@ static int fanotify_find_path(int dfd, const char __user *filename, dfd, filename, flags); if (filename == NULL) { - struct file *file; - int fput_needed; + struct fd f = fdget(dfd); ret = -EBADF; - file = fget_light(dfd, &fput_needed); - if (!file) + if (!f.file) goto out; ret = -ENOTDIR; if ((flags & FAN_MARK_ONLYDIR) && - !(S_ISDIR(file->f_path.dentry->d_inode->i_mode))) { - fput_light(file, fput_needed); + !(S_ISDIR(f.file->f_path.dentry->d_inode->i_mode))) { + fdput(f); goto out; } - *path = file->f_path; + *path = f.file->f_path; path_get(path); - fput_light(file, fput_needed); + fdput(f); } else { unsigned int lookup_flags = 0; @@ -748,9 +746,9 @@ SYSCALL_DEFINE(fanotify_mark)(int fanotify_fd, unsigned int flags, struct inode *inode = NULL; struct vfsmount *mnt = NULL; struct fsnotify_group *group; - struct file *filp; + struct fd f; struct path path; - int ret, fput_needed; + int ret; pr_debug("%s: fanotify_fd=%d flags=%x dfd=%d pathname=%p mask=%llx\n", __func__, fanotify_fd, flags, dfd, pathname, mask); @@ -784,15 +782,15 @@ SYSCALL_DEFINE(fanotify_mark)(int fanotify_fd, unsigned int flags, #endif return -EINVAL; - filp = fget_light(fanotify_fd, &fput_needed); - if (unlikely(!filp)) + f = fdget(fanotify_fd); + if (unlikely(!f.file)) return -EBADF; /* verify that this is indeed an fanotify instance */ ret = -EINVAL; - if (unlikely(filp->f_op != &fanotify_fops)) + if (unlikely(f.file->f_op != &fanotify_fops)) goto fput_and_out; - group = filp->private_data; + group = f.file->private_data; /* * group->priority == FS_PRIO_0 == FAN_CLASS_NOTIF. These are not @@ -839,7 +837,7 @@ SYSCALL_DEFINE(fanotify_mark)(int fanotify_fd, unsigned int flags, path_put(&path); fput_and_out: - fput_light(filp, fput_needed); + fdput(f); return ret; } diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 8445fbc8985..c311dda054a 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -757,16 +757,16 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname, struct fsnotify_group *group; struct inode *inode; struct path path; - struct file *filp; - int ret, fput_needed; + struct fd f; + int ret; unsigned flags = 0; - filp = fget_light(fd, &fput_needed); - if (unlikely(!filp)) + f = fdget(fd); + if (unlikely(!f.file)) return -EBADF; /* verify that this is indeed an inotify instance */ - if (unlikely(filp->f_op != &inotify_fops)) { + if (unlikely(f.file->f_op != &inotify_fops)) { ret = -EINVAL; goto fput_and_out; } @@ -782,13 +782,13 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname, /* inode held in place by reference to path; group by fget on fd */ inode = path.dentry->d_inode; - group = filp->private_data; + group = f.file->private_data; /* create/update an inode mark */ ret = inotify_update_watch(group, inode, mask); path_put(&path); fput_and_out: - fput_light(filp, fput_needed); + fdput(f); return ret; } @@ -796,19 +796,19 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd) { struct fsnotify_group *group; struct inotify_inode_mark *i_mark; - struct file *filp; - int ret = 0, fput_needed; + struct fd f; + int ret = 0; - filp = fget_light(fd, &fput_needed); - if (unlikely(!filp)) + f = fdget(fd); + if (unlikely(!f.file)) return -EBADF; /* verify that this is indeed an inotify instance */ ret = -EINVAL; - if (unlikely(filp->f_op != &inotify_fops)) + if (unlikely(f.file->f_op != &inotify_fops)) goto out; - group = filp->private_data; + group = f.file->private_data; ret = -EINVAL; i_mark = inotify_idr_find(group, wd); @@ -823,7 +823,7 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd) fsnotify_put_mark(&i_mark->fsn_mark); out: - fput_light(filp, fput_needed); + fdput(f); return ret; } -- cgit v1.2.3 From 848561d368751a1c0f679b9f045a02944506a801 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Thu, 8 Nov 2012 15:53:37 -0800 Subject: fanotify: fix missing break Anders Blomdell noted in 2010 that Fanotify lost events and provided a test case. Eric Paris confirmed it was a bug and posted a fix to the list https://groups.google.com/forum/?fromgroups=#!topic/linux.kernel/RrJfTfyW2BE but never applied it. Repeated attempts over time to actually get him to apply it have never had a reply from anyone who has raised it So apply it anyway Signed-off-by: Alan Cox Reported-by: Anders Blomdell Cc: Eric Paris Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/fanotify/fanotify.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index f35794b97e8..a5063602536 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -21,6 +21,7 @@ static bool should_merge(struct fsnotify_event *old, struct fsnotify_event *new) if ((old->path.mnt == new->path.mnt) && (old->path.dentry == new->path.dentry)) return true; + break; case (FSNOTIFY_EVENT_NONE): return true; default: -- cgit v1.2.3 From 3587b1b097d70c2eb9fee95ea7995d13c05f66e5 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 18 Nov 2012 19:19:00 +0000 Subject: fanotify: fix FAN_Q_OVERFLOW case of fanotify_read() If the FAN_Q_OVERFLOW bit set in event->mask, the fanotify event metadata will not contain a valid file descriptor, but copy_event_to_user() didn't check for that, and unconditionally does a fd_install() on the file descriptor. Which in turn will cause a BUG_ON() in __fd_install(). Introduced by commit 352e3b249284 ("fanotify: sanitize failure exits in copy_event_to_user()") Mea culpa - missed that path ;-/ Reported-by: Alex Shi Signed-off-by: Al Viro Signed-off-by: Linus Torvalds --- fs/notify/fanotify/fanotify_user.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 721d692fa8d..6fcaeb8c902 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -258,7 +258,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group, if (ret) goto out_close_fd; - fd_install(fd, f); + if (fd != FAN_NOFD) + fd_install(fd, f); return fanotify_event_metadata.event_len; out_close_fd: -- cgit v1.2.3 From 02582e9bcc36ed503ffede46e104a885dea222fb Mon Sep 17 00:00:00 2001 From: Masanari Iida Date: Wed, 22 Aug 2012 19:11:26 +0900 Subject: treewide: fix typo of "suport" in various comments and Kconfig Signed-off-by: Masanari Iida Signed-off-by: Jiri Kosina --- fs/notify/fanotify/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/Kconfig b/fs/notify/fanotify/Kconfig index 7dceff005a6..e5f911bd80d 100644 --- a/fs/notify/fanotify/Kconfig +++ b/fs/notify/fanotify/Kconfig @@ -4,7 +4,7 @@ config FANOTIFY select ANON_INODES default n ---help--- - Say Y here to enable fanotify suport. fanotify is a file access + Say Y here to enable fanotify support. fanotify is a file access notification system which differs from inotify in that it sends an open file descriptor to the userspace listener along with the event. -- cgit v1.2.3 From b3834be5c42a5d2fd85ff4b819fa38983b1450e6 Mon Sep 17 00:00:00 2001 From: Adam Buchbinder Date: Wed, 19 Sep 2012 21:48:02 -0400 Subject: various: Fix spelling of "asynchronous" in comments. "Asynchronous" is misspelled in some comments. No code changes. Signed-off-by: Adam Buchbinder Signed-off-by: Jiri Kosina --- fs/notify/notification.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/notify') diff --git a/fs/notify/notification.c b/fs/notify/notification.c index c887b1378f7..48cb994e492 100644 --- a/fs/notify/notification.c +++ b/fs/notify/notification.c @@ -18,7 +18,7 @@ /* * Basic idea behind the notification queue: An fsnotify group (like inotify) - * sends the userspace notification about events asyncronously some time after + * sends the userspace notification about events asynchronously some time after * the event happened. When inotify gets an event it will need to add that * event to the group notify queue. Since a single event might need to be on * multiple group's notification queues we can't add the event directly to each -- cgit v1.2.3 From d8153d4d8b7b6141770e1416c4a338161205ed1b Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:45 +0200 Subject: inotify, fanotify: replace fsnotify_put_group() with fsnotify_destroy_group() Currently in fsnotify_put_group() the ref count of a group is decremented and if it becomes 0 fsnotify_destroy_group() is called. Since a groups ref count is only at group creation set to 1 and never increased after that a call to fsnotify_put_group() always results in a call to fsnotify_destroy_group(). With this patch fsnotify_destroy_group() is called directly. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/fanotify/fanotify_user.c | 14 +++++++------- fs/notify/group.c | 2 +- fs/notify/inotify/inotify_user.c | 8 +++----- 3 files changed, 11 insertions(+), 13 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index d4380366973..82ae6d783c1 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -415,7 +415,7 @@ static int fanotify_release(struct inode *ignored, struct file *file) wake_up(&group->fanotify_data.access_waitq); #endif /* matches the fanotify_init->fsnotify_alloc_group */ - fsnotify_put_group(group); + fsnotify_destroy_group(group); return 0; } @@ -728,13 +728,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) break; default: fd = -EINVAL; - goto out_put_group; + goto out_destroy_group; } if (flags & FAN_UNLIMITED_QUEUE) { fd = -EPERM; if (!capable(CAP_SYS_ADMIN)) - goto out_put_group; + goto out_destroy_group; group->max_events = UINT_MAX; } else { group->max_events = FANOTIFY_DEFAULT_MAX_EVENTS; @@ -743,7 +743,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) if (flags & FAN_UNLIMITED_MARKS) { fd = -EPERM; if (!capable(CAP_SYS_ADMIN)) - goto out_put_group; + goto out_destroy_group; group->fanotify_data.max_marks = UINT_MAX; } else { group->fanotify_data.max_marks = FANOTIFY_DEFAULT_MAX_MARKS; @@ -751,12 +751,12 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) fd = anon_inode_getfd("[fanotify]", &fanotify_fops, group, f_flags); if (fd < 0) - goto out_put_group; + goto out_destroy_group; return fd; -out_put_group: - fsnotify_put_group(group); +out_destroy_group: + fsnotify_destroy_group(group); return fd; } diff --git a/fs/notify/group.c b/fs/notify/group.c index 63fc294a469..cfda328c3d1 100644 --- a/fs/notify/group.c +++ b/fs/notify/group.c @@ -50,7 +50,7 @@ void fsnotify_final_destroy_group(struct fsnotify_group *group) * situtation, the fsnotify_final_destroy_group will get called when that final * mark is freed. */ -static void fsnotify_destroy_group(struct fsnotify_group *group) +void fsnotify_destroy_group(struct fsnotify_group *group) { /* clear all inode marks for this group */ fsnotify_clear_marks_by_group(group); diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 8445fbc8985..dbafbfc8cec 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -293,10 +293,8 @@ static int inotify_release(struct inode *ignored, struct file *file) pr_debug("%s: group=%p\n", __func__, group); - fsnotify_clear_marks_by_group(group); - /* free this group, matching get was inotify_init->fsnotify_obtain_group */ - fsnotify_put_group(group); + fsnotify_destroy_group(group); return 0; } @@ -712,7 +710,7 @@ static struct fsnotify_group *inotify_new_group(unsigned int max_events) if (atomic_inc_return(&group->inotify_data.user->inotify_devs) > inotify_max_user_instances) { - fsnotify_put_group(group); + fsnotify_destroy_group(group); return ERR_PTR(-EMFILE); } @@ -741,7 +739,7 @@ SYSCALL_DEFINE1(inotify_init1, int, flags) ret = anon_inode_getfd("inotify", &inotify_fops, group, O_RDONLY | flags); if (ret < 0) - fsnotify_put_group(group); + fsnotify_destroy_group(group); return ret; } -- cgit v1.2.3 From 986129520479d689962a42c31acdeaf854ac91f5 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:46 +0200 Subject: fsnotify: introduce fsnotify_get_group() Introduce fsnotify_get_group() which increments the reference counter of a group. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/group.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'fs/notify') diff --git a/fs/notify/group.c b/fs/notify/group.c index cfda328c3d1..1d57c35f104 100644 --- a/fs/notify/group.c +++ b/fs/notify/group.c @@ -62,6 +62,14 @@ void fsnotify_destroy_group(struct fsnotify_group *group) fsnotify_final_destroy_group(group); } +/* + * Get reference to a group. + */ +void fsnotify_get_group(struct fsnotify_group *group) +{ + atomic_inc(&group->refcnt); +} + /* * Drop a reference to a group. Free it if it's through. */ -- cgit v1.2.3 From 23e964c284ca0a767b80a30482bd53b059d30391 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:47 +0200 Subject: fsnotify: use reference counting for groups Get a group ref for each mark that is added to the groups list and release that ref when the mark is freed in fsnotify_put_mark(). We also use get a group reference for duplicated marks and for private event data. Now we dont free a group any more when the number of marks becomes 0 but when the groups ref count does. Since this will only happen when all marks are removed from a groups mark list, we dont have to set the groups number of marks to 1 at group creation. Beside clearing all marks in fsnotify_destroy_group() we do also flush the groups event queue. This is since events may hold references to groups (due to private event data) and we have to put those references first before we get a chance to put the final ref, which will result in a call to fsnotify_final_destroy_group(). Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/group.c | 28 ++++++++++------------------ fs/notify/inotify/inotify_fsnotify.c | 2 ++ fs/notify/inotify/inotify_user.c | 1 + fs/notify/mark.c | 24 ++++++++++++++---------- 4 files changed, 27 insertions(+), 28 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/group.c b/fs/notify/group.c index 1d57c35f104..354044c47e2 100644 --- a/fs/notify/group.c +++ b/fs/notify/group.c @@ -33,9 +33,6 @@ */ void fsnotify_final_destroy_group(struct fsnotify_group *group) { - /* clear the notification queue of all events */ - fsnotify_flush_notify(group); - if (group->ops->free_group_priv) group->ops->free_group_priv(group); @@ -43,12 +40,10 @@ void fsnotify_final_destroy_group(struct fsnotify_group *group) } /* - * Trying to get rid of a group. We need to first get rid of any outstanding - * allocations and then free the group. Remember that fsnotify_clear_marks_by_group - * could miss marks that are being freed by inode and those marks could still - * hold a reference to this group (via group->num_marks) If we get into that - * situtation, the fsnotify_final_destroy_group will get called when that final - * mark is freed. + * Trying to get rid of a group. Remove all marks, flush all events and release + * the group reference. + * Note that another thread calling fsnotify_clear_marks_by_group() may still + * hold a ref to the group. */ void fsnotify_destroy_group(struct fsnotify_group *group) { @@ -57,9 +52,10 @@ void fsnotify_destroy_group(struct fsnotify_group *group) synchronize_srcu(&fsnotify_mark_srcu); - /* past the point of no return, matches the initial value of 1 */ - if (atomic_dec_and_test(&group->num_marks)) - fsnotify_final_destroy_group(group); + /* clear the notification queue of all events */ + fsnotify_flush_notify(group); + + fsnotify_put_group(group); } /* @@ -76,7 +72,7 @@ void fsnotify_get_group(struct fsnotify_group *group) void fsnotify_put_group(struct fsnotify_group *group) { if (atomic_dec_and_test(&group->refcnt)) - fsnotify_destroy_group(group); + fsnotify_final_destroy_group(group); } /* @@ -92,11 +88,7 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops) /* set to 0 when there a no external references to this group */ atomic_set(&group->refcnt, 1); - /* - * hits 0 when there are no external references AND no marks for - * this group - */ - atomic_set(&group->num_marks, 1); + atomic_set(&group->num_marks, 0); mutex_init(&group->notification_mutex); INIT_LIST_HEAD(&group->notification_list); diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c index e3cbd746f64..74977fbf5aa 100644 --- a/fs/notify/inotify/inotify_fsnotify.c +++ b/fs/notify/inotify/inotify_fsnotify.c @@ -118,6 +118,7 @@ static int inotify_handle_event(struct fsnotify_group *group, fsn_event_priv = &event_priv->fsnotify_event_priv_data; + fsnotify_get_group(group); fsn_event_priv->group = group; event_priv->wd = wd; @@ -210,6 +211,7 @@ void inotify_free_event_priv(struct fsnotify_event_private_data *fsn_event_priv) event_priv = container_of(fsn_event_priv, struct inotify_event_private_data, fsnotify_event_priv_data); + fsnotify_put_group(fsn_event_priv->group); kmem_cache_free(event_priv_cachep, event_priv); } diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index dbafbfc8cec..246250f1db7 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -531,6 +531,7 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark, fsn_event_priv = &event_priv->fsnotify_event_priv_data; + fsnotify_get_group(group); fsn_event_priv->group = group; event_priv->wd = i_mark->wd; diff --git a/fs/notify/mark.c b/fs/notify/mark.c index f104d565b68..3c7a1699df3 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -109,8 +109,11 @@ void fsnotify_get_mark(struct fsnotify_mark *mark) void fsnotify_put_mark(struct fsnotify_mark *mark) { - if (atomic_dec_and_test(&mark->refcnt)) + if (atomic_dec_and_test(&mark->refcnt)) { + if (mark->group) + fsnotify_put_group(mark->group); mark->free_mark(mark); + } } /* @@ -125,12 +128,13 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) spin_lock(&mark->lock); + fsnotify_get_group(mark->group); group = mark->group; /* something else already called this function on this mark */ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { spin_unlock(&mark->lock); - return; + goto put_group; } mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; @@ -177,19 +181,15 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED)) iput(inode); - /* * We don't necessarily have a ref on mark from caller so the above iput * may have already destroyed it. Don't touch from now on. */ - /* - * it's possible that this group tried to destroy itself, but this - * this mark was simultaneously being freed by inode. If that's the - * case, we finish freeing the group here. - */ - if (unlikely(atomic_dec_and_test(&group->num_marks))) - fsnotify_final_destroy_group(group); + atomic_dec(&group->num_marks); + +put_group: + fsnotify_put_group(group); } void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask) @@ -234,6 +234,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE; + fsnotify_get_group(group); mark->group = group; list_add(&mark->g_list, &group->marks_list); atomic_inc(&group->num_marks); @@ -265,6 +266,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, err: mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; list_del_init(&mark->g_list); + fsnotify_put_group(group); mark->group = NULL; atomic_dec(&group->num_marks); @@ -317,6 +319,8 @@ void fsnotify_duplicate_mark(struct fsnotify_mark *new, struct fsnotify_mark *ol assert_spin_locked(&old->lock); new->i.inode = old->i.inode; new->m.mnt = old->m.mnt; + if (old->group) + fsnotify_get_group(old->group); new->group = old->group; new->mask = old->mask; new->free_mark = old->free_mark; -- cgit v1.2.3 From 104d06f08ea59247cb0e7e548c5a5d22d21dcfd5 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:48 +0200 Subject: fsnotify: take groups mark_lock before mark lock Race-free addition and removal of a mark to a groups mark list would be easier if we could lock the mark list of group before we lock the specific mark. This patch changes the order used to add/remove marks to/from mark lists from 1. mark->lock 2. group->mark_lock 3. inode->i_lock to 1. group->mark_lock 2. mark->lock 3. inode->i_lock Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/mark.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 3c7a1699df3..32447dc06c0 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -127,20 +127,27 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) struct inode *inode = NULL; spin_lock(&mark->lock); - + /* dont get the group from a mark that is not alive yet */ + if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { + spin_unlock(&mark->lock); + return; + } fsnotify_get_group(mark->group); group = mark->group; + spin_unlock(&mark->lock); + + spin_lock(&group->mark_lock); + spin_lock(&mark->lock); /* something else already called this function on this mark */ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { spin_unlock(&mark->lock); + spin_unlock(&group->mark_lock); goto put_group; } mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; - spin_lock(&group->mark_lock); - if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) { inode = mark->i.inode; fsnotify_destroy_inode_mark(mark); @@ -151,8 +158,8 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) list_del_init(&mark->g_list); - spin_unlock(&group->mark_lock); spin_unlock(&mark->lock); + spin_unlock(&group->mark_lock); spin_lock(&destroy_lock); list_add(&mark->destroy_list, &destroy_list); @@ -225,13 +232,13 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, /* * LOCKING ORDER!!!! - * mark->lock * group->mark_lock + * mark->lock * inode->i_lock */ - spin_lock(&mark->lock); spin_lock(&group->mark_lock); + spin_lock(&mark->lock); mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE; fsnotify_get_group(group); @@ -252,13 +259,12 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, BUG(); } - spin_unlock(&group->mark_lock); - /* this will pin the object if appropriate */ fsnotify_set_mark_mask_locked(mark, mark->mask); - spin_unlock(&mark->lock); + spin_unlock(&group->mark_lock); + if (inode) __fsnotify_update_child_dentry_flags(inode); @@ -270,8 +276,8 @@ err: mark->group = NULL; atomic_dec(&group->num_marks); - spin_unlock(&group->mark_lock); spin_unlock(&mark->lock); + spin_unlock(&group->mark_lock); spin_lock(&destroy_lock); list_add(&mark->destroy_list, &destroy_list); -- cgit v1.2.3 From 6dfbd149946c22c2e2886d6b560def78630c8387 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:49 +0200 Subject: fanotify: add an extra flag to mark_remove_from_mask that indicates wheather a mark should be destroyed This patch adds an extra flag to mark_remove_from_mask() to inform the caller if the mark should be destroyed. With this we dont destroy the mark implicitly in the function itself any more but let the caller handle it. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/fanotify/fanotify_user.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 82ae6d783c1..599a01952c7 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -511,7 +511,8 @@ out: static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, __u32 mask, - unsigned int flags) + unsigned int flags, + int *destroy) { __u32 oldmask; @@ -525,8 +526,7 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, } spin_unlock(&fsn_mark->lock); - if (!(oldmask & ~mask)) - fsnotify_destroy_mark(fsn_mark); + *destroy = !(oldmask & ~mask); return mask & oldmask; } @@ -537,12 +537,17 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group, { struct fsnotify_mark *fsn_mark = NULL; __u32 removed; + int destroy_mark; fsn_mark = fsnotify_find_vfsmount_mark(group, mnt); if (!fsn_mark) return -ENOENT; - removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags); + removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags, + &destroy_mark); + if (destroy_mark) + fsnotify_destroy_mark(fsn_mark); + fsnotify_put_mark(fsn_mark); if (removed & real_mount(mnt)->mnt_fsnotify_mask) fsnotify_recalc_vfsmount_mask(mnt); @@ -556,12 +561,16 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group, { struct fsnotify_mark *fsn_mark = NULL; __u32 removed; + int destroy_mark; fsn_mark = fsnotify_find_inode_mark(group, inode); if (!fsn_mark) return -ENOENT; - removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags); + removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags, + &destroy_mark); + if (destroy_mark) + fsnotify_destroy_mark(fsn_mark); /* matches the fsnotify_find_inode_mark() */ fsnotify_put_mark(fsn_mark); if (removed & inode->i_fsnotify_mask) -- cgit v1.2.3 From 986ab09807ca9454c3f54aae4db7e1bb00daeed3 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:50 +0200 Subject: fsnotify: use a mutex instead of a spinlock to protect a groups mark list Replaces the groups mark_lock spinlock with a mutex. Using a mutex instead of a spinlock results in more flexibility (i.e it allows to sleep while the lock is held). Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/group.c | 2 +- fs/notify/inode_mark.c | 4 ++-- fs/notify/mark.c | 18 +++++++++--------- fs/notify/vfsmount_mark.c | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/group.c b/fs/notify/group.c index 354044c47e2..1f7305711fc 100644 --- a/fs/notify/group.c +++ b/fs/notify/group.c @@ -95,7 +95,7 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops) init_waitqueue_head(&group->notification_waitq); group->max_events = UINT_MAX; - spin_lock_init(&group->mark_lock); + mutex_init(&group->mark_mutex); INIT_LIST_HEAD(&group->marks_list); group->ops = ops; diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index b13c00ac48e..4e9071e37d5 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -63,8 +63,8 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark) { struct inode *inode = mark->i.inode; + BUG_ON(!mutex_is_locked(&mark->group->mark_mutex)); assert_spin_locked(&mark->lock); - assert_spin_locked(&mark->group->mark_lock); spin_lock(&inode->i_lock); @@ -191,8 +191,8 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark, mark->flags |= FSNOTIFY_MARK_FLAG_INODE; + BUG_ON(!mutex_is_locked(&group->mark_mutex)); assert_spin_locked(&mark->lock); - assert_spin_locked(&group->mark_lock); spin_lock(&inode->i_lock); diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 32447dc06c0..ab25b810b14 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -136,13 +136,13 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) group = mark->group; spin_unlock(&mark->lock); - spin_lock(&group->mark_lock); + mutex_lock(&group->mark_mutex); spin_lock(&mark->lock); /* something else already called this function on this mark */ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { spin_unlock(&mark->lock); - spin_unlock(&group->mark_lock); + mutex_unlock(&group->mark_mutex); goto put_group; } @@ -159,7 +159,7 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) list_del_init(&mark->g_list); spin_unlock(&mark->lock); - spin_unlock(&group->mark_lock); + mutex_unlock(&group->mark_mutex); spin_lock(&destroy_lock); list_add(&mark->destroy_list, &destroy_list); @@ -232,11 +232,11 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, /* * LOCKING ORDER!!!! - * group->mark_lock + * group->mark_mutex * mark->lock * inode->i_lock */ - spin_lock(&group->mark_lock); + mutex_lock(&group->mark_mutex); spin_lock(&mark->lock); mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE; @@ -263,7 +263,7 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, fsnotify_set_mark_mask_locked(mark, mark->mask); spin_unlock(&mark->lock); - spin_unlock(&group->mark_lock); + mutex_unlock(&group->mark_mutex); if (inode) __fsnotify_update_child_dentry_flags(inode); @@ -277,7 +277,7 @@ err: atomic_dec(&group->num_marks); spin_unlock(&mark->lock); - spin_unlock(&group->mark_lock); + mutex_unlock(&group->mark_mutex); spin_lock(&destroy_lock); list_add(&mark->destroy_list, &destroy_list); @@ -296,7 +296,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, struct fsnotify_mark *lmark, *mark; LIST_HEAD(free_list); - spin_lock(&group->mark_lock); + mutex_lock(&group->mark_mutex); list_for_each_entry_safe(mark, lmark, &group->marks_list, g_list) { if (mark->flags & flags) { list_add(&mark->free_g_list, &free_list); @@ -304,7 +304,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, fsnotify_get_mark(mark); } } - spin_unlock(&group->mark_lock); + mutex_unlock(&group->mark_mutex); list_for_each_entry_safe(mark, lmark, &free_list, free_g_list) { fsnotify_destroy_mark(mark); diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c index b7b4b0e8554..f26a348827f 100644 --- a/fs/notify/vfsmount_mark.c +++ b/fs/notify/vfsmount_mark.c @@ -88,8 +88,8 @@ void fsnotify_destroy_vfsmount_mark(struct fsnotify_mark *mark) { struct vfsmount *mnt = mark->m.mnt; + BUG_ON(!mutex_is_locked(&mark->group->mark_mutex)); assert_spin_locked(&mark->lock); - assert_spin_locked(&mark->group->mark_lock); spin_lock(&mnt->mnt_root->d_lock); @@ -151,8 +151,8 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark, mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT; + BUG_ON(!mutex_is_locked(&group->mark_mutex)); assert_spin_locked(&mark->lock); - assert_spin_locked(&group->mark_lock); spin_lock(&mnt->mnt_root->d_lock); -- cgit v1.2.3 From e2a29943e9a2ee2aa737a77f550f46ba72269db4 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:51 +0200 Subject: fsnotify: pass group to fsnotify_destroy_mark() In fsnotify_destroy_mark() dont get the group from the passed mark anymore, but pass the group itself as an additional parameter to the function. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/dnotify/dnotify.c | 4 ++-- fs/notify/fanotify/fanotify_user.c | 4 ++-- fs/notify/inode_mark.c | 10 +++++++++- fs/notify/inotify/inotify_fsnotify.c | 2 +- fs/notify/inotify/inotify_user.c | 2 +- fs/notify/mark.c | 21 ++++----------------- fs/notify/vfsmount_mark.c | 10 +++++++++- 7 files changed, 28 insertions(+), 25 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index 3344bdd5506..08b886f119c 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -201,7 +201,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id) /* nothing else could have found us thanks to the dnotify_mark_mutex */ if (dn_mark->dn == NULL) - fsnotify_destroy_mark(fsn_mark); + fsnotify_destroy_mark(fsn_mark, dnotify_group); mutex_unlock(&dnotify_mark_mutex); @@ -385,7 +385,7 @@ out: spin_unlock(&fsn_mark->lock); if (destroy) - fsnotify_destroy_mark(fsn_mark); + fsnotify_destroy_mark(fsn_mark, dnotify_group); mutex_unlock(&dnotify_mark_mutex); fsnotify_put_mark(fsn_mark); diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 599a01952c7..1218d10424d 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -546,7 +546,7 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group, removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags, &destroy_mark); if (destroy_mark) - fsnotify_destroy_mark(fsn_mark); + fsnotify_destroy_mark(fsn_mark, group); fsnotify_put_mark(fsn_mark); if (removed & real_mount(mnt)->mnt_fsnotify_mask) @@ -570,7 +570,7 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group, removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags, &destroy_mark); if (destroy_mark) - fsnotify_destroy_mark(fsn_mark); + fsnotify_destroy_mark(fsn_mark, group); /* matches the fsnotify_find_inode_mark() */ fsnotify_put_mark(fsn_mark); if (removed & inode->i_fsnotify_mask) diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index 4e9071e37d5..21230209c95 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -99,8 +99,16 @@ void fsnotify_clear_marks_by_inode(struct inode *inode) spin_unlock(&inode->i_lock); list_for_each_entry_safe(mark, lmark, &free_list, i.free_i_list) { - fsnotify_destroy_mark(mark); + struct fsnotify_group *group; + + spin_lock(&mark->lock); + fsnotify_get_group(mark->group); + group = mark->group; + spin_unlock(&mark->lock); + + fsnotify_destroy_mark(mark, group); fsnotify_put_mark(mark); + fsnotify_put_group(group); } } diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c index 74977fbf5aa..871569c7d60 100644 --- a/fs/notify/inotify/inotify_fsnotify.c +++ b/fs/notify/inotify/inotify_fsnotify.c @@ -132,7 +132,7 @@ static int inotify_handle_event(struct fsnotify_group *group, } if (inode_mark->mask & IN_ONESHOT) - fsnotify_destroy_mark(inode_mark); + fsnotify_destroy_mark(inode_mark, group); return ret; } diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 246250f1db7..00ff82ff7c9 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -816,7 +816,7 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd) ret = 0; - fsnotify_destroy_mark(&i_mark->fsn_mark); + fsnotify_destroy_mark(&i_mark->fsn_mark, group); /* match ref taken by inotify_idr_find */ fsnotify_put_mark(&i_mark->fsn_mark); diff --git a/fs/notify/mark.c b/fs/notify/mark.c index ab25b810b14..b77c833c8d0 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -121,21 +121,11 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) * The caller had better be holding a reference to this mark so we don't actually * do the final put under the mark->lock */ -void fsnotify_destroy_mark(struct fsnotify_mark *mark) +void fsnotify_destroy_mark(struct fsnotify_mark *mark, + struct fsnotify_group *group) { - struct fsnotify_group *group; struct inode *inode = NULL; - spin_lock(&mark->lock); - /* dont get the group from a mark that is not alive yet */ - if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { - spin_unlock(&mark->lock); - return; - } - fsnotify_get_group(mark->group); - group = mark->group; - spin_unlock(&mark->lock); - mutex_lock(&group->mark_mutex); spin_lock(&mark->lock); @@ -143,7 +133,7 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { spin_unlock(&mark->lock); mutex_unlock(&group->mark_mutex); - goto put_group; + return; } mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; @@ -194,9 +184,6 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark) */ atomic_dec(&group->num_marks); - -put_group: - fsnotify_put_group(group); } void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask) @@ -307,7 +294,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, mutex_unlock(&group->mark_mutex); list_for_each_entry_safe(mark, lmark, &free_list, free_g_list) { - fsnotify_destroy_mark(mark); + fsnotify_destroy_mark(mark, group); fsnotify_put_mark(mark); } } diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c index f26a348827f..4df58b8ea64 100644 --- a/fs/notify/vfsmount_mark.c +++ b/fs/notify/vfsmount_mark.c @@ -46,8 +46,16 @@ void fsnotify_clear_marks_by_mount(struct vfsmount *mnt) spin_unlock(&mnt->mnt_root->d_lock); list_for_each_entry_safe(mark, lmark, &free_list, m.free_m_list) { - fsnotify_destroy_mark(mark); + struct fsnotify_group *group; + + spin_lock(&mark->lock); + fsnotify_get_group(mark->group); + group = mark->group; + spin_unlock(&mark->lock); + + fsnotify_destroy_mark(mark, group); fsnotify_put_mark(mark); + fsnotify_put_group(group); } } -- cgit v1.2.3 From d5a335b845792d2a69ed1e244c0b233117b7db3c Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:52 +0200 Subject: fsnotify: introduce locked versions of fsnotify_add_mark() and fsnotify_remove_mark() This patch introduces fsnotify_add_mark_locked() and fsnotify_remove_mark_locked() which are essentially the same as fsnotify_add_mark() and fsnotify_remove_mark() but assume that the caller has already taken the groups mark mutex. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/mark.c | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/mark.c b/fs/notify/mark.c index b77c833c8d0..f9dda0304a1 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -121,18 +121,18 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) * The caller had better be holding a reference to this mark so we don't actually * do the final put under the mark->lock */ -void fsnotify_destroy_mark(struct fsnotify_mark *mark, - struct fsnotify_group *group) +void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, + struct fsnotify_group *group) { struct inode *inode = NULL; - mutex_lock(&group->mark_mutex); + BUG_ON(!mutex_is_locked(&group->mark_mutex)); + spin_lock(&mark->lock); /* something else already called this function on this mark */ if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { spin_unlock(&mark->lock); - mutex_unlock(&group->mark_mutex); return; } @@ -149,6 +149,8 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark, list_del_init(&mark->g_list); spin_unlock(&mark->lock); + + /* release lock temporarily */ mutex_unlock(&group->mark_mutex); spin_lock(&destroy_lock); @@ -184,6 +186,16 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark, */ atomic_dec(&group->num_marks); + + mutex_lock(&group->mark_mutex); +} + +void fsnotify_destroy_mark(struct fsnotify_mark *mark, + struct fsnotify_group *group) +{ + mutex_lock(&group->mark_mutex); + fsnotify_destroy_mark_locked(mark, group); + mutex_unlock(&group->mark_mutex); } void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask) @@ -208,14 +220,15 @@ void fsnotify_set_mark_ignored_mask_locked(struct fsnotify_mark *mark, __u32 mas * These marks may be used for the fsnotify backend to determine which * event types should be delivered to which group. */ -int fsnotify_add_mark(struct fsnotify_mark *mark, - struct fsnotify_group *group, struct inode *inode, - struct vfsmount *mnt, int allow_dups) +int fsnotify_add_mark_locked(struct fsnotify_mark *mark, + struct fsnotify_group *group, struct inode *inode, + struct vfsmount *mnt, int allow_dups) { int ret = 0; BUG_ON(inode && mnt); BUG_ON(!inode && !mnt); + BUG_ON(!mutex_is_locked(&group->mark_mutex)); /* * LOCKING ORDER!!!! @@ -223,8 +236,6 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, * mark->lock * inode->i_lock */ - mutex_lock(&group->mark_mutex); - spin_lock(&mark->lock); mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE; @@ -250,8 +261,6 @@ int fsnotify_add_mark(struct fsnotify_mark *mark, fsnotify_set_mark_mask_locked(mark, mark->mask); spin_unlock(&mark->lock); - mutex_unlock(&group->mark_mutex); - if (inode) __fsnotify_update_child_dentry_flags(inode); @@ -264,7 +273,6 @@ err: atomic_dec(&group->num_marks); spin_unlock(&mark->lock); - mutex_unlock(&group->mark_mutex); spin_lock(&destroy_lock); list_add(&mark->destroy_list, &destroy_list); @@ -274,6 +282,16 @@ err: return ret; } +int fsnotify_add_mark(struct fsnotify_mark *mark, struct fsnotify_group *group, + struct inode *inode, struct vfsmount *mnt, int allow_dups) +{ + int ret; + mutex_lock(&group->mark_mutex); + ret = fsnotify_add_mark_locked(mark, group, inode, mnt, allow_dups); + mutex_unlock(&group->mark_mutex); + return ret; +} + /* * clear any marks in a group in which mark->flags & flags is true */ -- cgit v1.2.3 From 64c20d2a20fce295c260ea6cb3b468edfa2fb07b Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Tue, 14 Jun 2011 17:29:53 +0200 Subject: fsnotify: dont put marks on temporary list when clearing marks by group In clear_marks_by_group_flags() the mark list of a group is iterated and the marks are put on a temporary list. Since we introduced fsnotify_destroy_mark_locked() we dont need the temp list any more and are able to remove the marks while the mark list is iterated and the mark list mutex is held. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/mark.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/mark.c b/fs/notify/mark.c index f9dda0304a1..0e93d90bb75 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -299,22 +299,16 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, unsigned int flags) { struct fsnotify_mark *lmark, *mark; - LIST_HEAD(free_list); mutex_lock(&group->mark_mutex); list_for_each_entry_safe(mark, lmark, &group->marks_list, g_list) { if (mark->flags & flags) { - list_add(&mark->free_g_list, &free_list); - list_del_init(&mark->g_list); fsnotify_get_mark(mark); + fsnotify_destroy_mark_locked(mark, group); + fsnotify_put_mark(mark); } } mutex_unlock(&group->mark_mutex); - - list_for_each_entry_safe(mark, lmark, &free_list, free_g_list) { - fsnotify_destroy_mark(mark, group); - fsnotify_put_mark(mark); - } } /* -- cgit v1.2.3 From 6960b0d909cde5bdff49e4e5c1250edd10be7ebd Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Fri, 12 Aug 2011 01:13:31 +0200 Subject: fsnotify: change locking order On Mon, Aug 01, 2011 at 04:38:22PM -0400, Eric Paris wrote: > > I finally built and tested a v3.0 kernel with these patches (I know I'm > SOOOOOO far behind). Not what I hoped for: > > > [ 150.937798] VFS: Busy inodes after unmount of tmpfs. Self-destruct in 5 seconds. Have a nice day... > > [ 150.945290] BUG: unable to handle kernel NULL pointer dereference at 0000000000000070 > > [ 150.946012] IP: [] shmem_free_inode+0x18/0x50 > > [ 150.946012] PGD 2bf9e067 PUD 2bf9f067 PMD 0 > > [ 150.946012] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC > > [ 150.946012] CPU 0 > > [ 150.946012] Modules linked in: nfs lockd fscache auth_rpcgss nfs_acl sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables ext4 jbd2 crc16 joydev ata_piix i2c_piix4 pcspkr uinput ipv6 autofs4 usbhid [last unloaded: scsi_wait_scan] > > [ 150.946012] > > [ 150.946012] Pid: 2764, comm: syscall_thrash Not tainted 3.0.0+ #1 Red Hat KVM > > [ 150.946012] RIP: 0010:[] [] shmem_free_inode+0x18/0x50 > > [ 150.946012] RSP: 0018:ffff88002c2e5df8 EFLAGS: 00010282 > > [ 150.946012] RAX: 000000004e370d9f RBX: 0000000000000000 RCX: ffff88003a029438 > > [ 150.946012] RDX: 0000000033630a5f RSI: 0000000000000000 RDI: ffff88003491c240 > > [ 150.946012] RBP: ffff88002c2e5e08 R08: 0000000000000000 R09: 0000000000000000 > > [ 150.946012] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88003a029428 > > [ 150.946012] R13: ffff88003a029428 R14: ffff88003a029428 R15: ffff88003499a610 > > [ 150.946012] FS: 00007f5a05420700(0000) GS:ffff88003f600000(0000) knlGS:0000000000000000 > > [ 150.946012] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > > [ 150.946012] CR2: 0000000000000070 CR3: 000000002a662000 CR4: 00000000000006f0 > > [ 150.946012] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > [ 150.946012] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > > [ 150.946012] Process syscall_thrash (pid: 2764, threadinfo ffff88002c2e4000, task ffff88002bfbc760) > > [ 150.946012] Stack: > > [ 150.946012] ffff88003a029438 ffff88003a029428 ffff88002c2e5e38 ffffffff81102f76 > > [ 150.946012] ffff88003a029438 ffff88003a029598 ffffffff8160f9c0 ffff88002c221250 > > [ 150.946012] ffff88002c2e5e68 ffffffff8115e9be ffff88002c2e5e68 ffff88003a029438 > > [ 150.946012] Call Trace: > > [ 150.946012] [] shmem_evict_inode+0x76/0x130 > > [ 150.946012] [] evict+0x7e/0x170 > > [ 150.946012] [] iput_final+0xd0/0x190 > > [ 150.946012] [] iput+0x33/0x40 > > [ 150.946012] [] fsnotify_destroy_mark_locked+0x145/0x160 > > [ 150.946012] [] fsnotify_destroy_mark+0x36/0x50 > > [ 150.946012] [] sys_inotify_rm_watch+0x77/0xd0 > > [ 150.946012] [] system_call_fastpath+0x16/0x1b > > [ 150.946012] Code: 67 4a 00 b8 e4 ff ff ff eb aa 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 48 83 ec 10 48 89 1c 24 4c 89 64 24 08 48 8b 9f 40 05 00 00 > > [ 150.946012] 83 7b 70 00 74 1c 4c 8d a3 80 00 00 00 4c 89 e7 e8 d2 5d 4a > > [ 150.946012] RIP [] shmem_free_inode+0x18/0x50 > > [ 150.946012] RSP > > [ 150.946012] CR2: 0000000000000070 > > Looks at aweful lot like the problem from: > http://www.spinics.net/lists/linux-fsdevel/msg46101.html > I tried to reproduce this bug with your test program, but without success. However, if I understand correctly, this occurs since we dont hold any locks when we call iput() in mark_destroy(), right? With the patches you tested, iput() is also not called within any lock, since the groups mark_mutex is released temporarily before iput() is called. This is, since the original codes behaviour is similar. However since we now have a mutex as the biggest lock, we can do what you suggested (http://www.spinics.net/lists/linux-fsdevel/msg46107.html) and call iput() with the mutex held to avoid the race. The patch below implements this. It uses nested locking to avoid deadlock in case we do the final iput() on an inode which still holds marks and thus would take the mutex again when calling fsnotify_inode_delete() in destroy_inode(). Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/mark.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 0e93d90bb75..fc6b49bf736 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -150,6 +150,8 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, spin_unlock(&mark->lock); + if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED)) + iput(inode); /* release lock temporarily */ mutex_unlock(&group->mark_mutex); @@ -157,6 +159,11 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, list_add(&mark->destroy_list, &destroy_list); spin_unlock(&destroy_lock); wake_up(&destroy_waitq); + /* + * We don't necessarily have a ref on mark from caller so the above destroy + * may have actually freed it, unless this group provides a 'freeing_mark' + * function which must be holding a reference. + */ /* * Some groups like to know that marks are being freed. This is a @@ -178,22 +185,15 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, * is just a lazy update (and could be a perf win...) */ - if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED)) - iput(inode); - /* - * We don't necessarily have a ref on mark from caller so the above iput - * may have already destroyed it. Don't touch from now on. - */ - atomic_dec(&group->num_marks); - mutex_lock(&group->mark_mutex); + mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING); } void fsnotify_destroy_mark(struct fsnotify_mark *mark, struct fsnotify_group *group) { - mutex_lock(&group->mark_mutex); + mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING); fsnotify_destroy_mark_locked(mark, group); mutex_unlock(&group->mark_mutex); } @@ -300,7 +300,7 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, { struct fsnotify_mark *lmark, *mark; - mutex_lock(&group->mark_mutex); + mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING); list_for_each_entry_safe(mark, lmark, &group->marks_list, g_list) { if (mark->flags & flags) { fsnotify_get_mark(mark); -- cgit v1.2.3 From 0a6b6bd5919a65030b557ec8fe81f6fb3e93744a Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 14 Oct 2011 17:43:39 -0400 Subject: fsnotify: make fasync generic for both inotify and fanotify inotify is supposed to support async signal notification when information is available on the inotify fd. This patch moves that support to generic fsnotify functions so it can be used by all notification mechanisms. Signed-off-by: Eric Paris --- fs/notify/fanotify/fanotify_user.c | 4 ++++ fs/notify/group.c | 7 +++++++ fs/notify/inotify/inotify_user.c | 13 ++++--------- fs/notify/notification.c | 1 + 4 files changed, 16 insertions(+), 9 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 1218d10424d..f0e7a57bc89 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -414,6 +414,10 @@ static int fanotify_release(struct inode *ignored, struct file *file) wake_up(&group->fanotify_data.access_waitq); #endif + + if (file->f_flags & FASYNC) + fsnotify_fasync(-1, file, 0); + /* matches the fanotify_init->fsnotify_alloc_group */ fsnotify_destroy_group(group); diff --git a/fs/notify/group.c b/fs/notify/group.c index 1f7305711fc..bd2625bd88b 100644 --- a/fs/notify/group.c +++ b/fs/notify/group.c @@ -102,3 +102,10 @@ struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops) return group; } + +int fsnotify_fasync(int fd, struct file *file, int on) +{ + struct fsnotify_group *group = file->private_data; + + return fasync_helper(fd, file, on, &group->fsn_fa) >= 0 ? 0 : -EIO; +} diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 00ff82ff7c9..68f7bec1e66 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -280,19 +280,15 @@ static ssize_t inotify_read(struct file *file, char __user *buf, return ret; } -static int inotify_fasync(int fd, struct file *file, int on) -{ - struct fsnotify_group *group = file->private_data; - - return fasync_helper(fd, file, on, &group->inotify_data.fa) >= 0 ? 0 : -EIO; -} - static int inotify_release(struct inode *ignored, struct file *file) { struct fsnotify_group *group = file->private_data; pr_debug("%s: group=%p\n", __func__, group); + if (file->f_flags & FASYNC) + fsnotify_fasync(-1, file, 0); + /* free this group, matching get was inotify_init->fsnotify_obtain_group */ fsnotify_destroy_group(group); @@ -335,7 +331,7 @@ static long inotify_ioctl(struct file *file, unsigned int cmd, static const struct file_operations inotify_fops = { .poll = inotify_poll, .read = inotify_read, - .fasync = inotify_fasync, + .fasync = fsnotify_fasync, .release = inotify_release, .unlocked_ioctl = inotify_ioctl, .compat_ioctl = inotify_ioctl, @@ -706,7 +702,6 @@ static struct fsnotify_group *inotify_new_group(unsigned int max_events) spin_lock_init(&group->inotify_data.idr_lock); idr_init(&group->inotify_data.idr); group->inotify_data.last_wd = 0; - group->inotify_data.fa = NULL; group->inotify_data.user = get_current_user(); if (atomic_inc_return(&group->inotify_data.user->inotify_devs) > diff --git a/fs/notify/notification.c b/fs/notify/notification.c index c887b1378f7..b3963d8c998 100644 --- a/fs/notify/notification.c +++ b/fs/notify/notification.c @@ -225,6 +225,7 @@ alloc_holder: mutex_unlock(&group->notification_mutex); wake_up(&group->notification_waitq); + kill_fasync(&group->fsn_fa, SIGIO, POLL_IN); return return_event; } -- cgit v1.2.3 From 03a1cec1f17ac1a6041996b3e40f96b5a2f90e1b Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Fri, 23 Mar 2012 02:42:23 +0100 Subject: fanotify: dont merge permission events Boyd Yang reported a problem for the case that multiple threads of the same thread group are waiting for a reponse for a permission event. In this case it is possible that some of the threads are never woken up, even if the response for the event has been received (see http://marc.info/?l=linux-kernel&m=131822913806350&w=2). The reason is that we are currently merging permission events if they belong to the same thread group. But we are not prepared to wake up more than one waiter for each event. We do wait_event(group->fanotify_data.access_waitq, event->response || atomic_read(&group->fanotify_data.bypass_perm)); and after that event->response = 0; which is the reason that even if we woke up all waiters for the same event some of them may see event->response being already set 0 again, then go back to sleep and block forever. With this patch we avoid that more than one thread is waiting for a response by not merging permission events for the same thread group any more. Reported-by: Boyd Yang Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/fanotify/fanotify.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index f35794b97e8..aeb5b5abbd4 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -18,6 +18,12 @@ static bool should_merge(struct fsnotify_event *old, struct fsnotify_event *new) old->tgid == new->tgid) { switch (old->data_type) { case (FSNOTIFY_EVENT_PATH): +#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS + /* dont merge two permission events */ + if ((old->mask & FAN_ALL_PERM_EVENTS) && + (new->mask & FAN_ALL_PERM_EVENTS)) + return false; +#endif if ((old->path.mnt == new->path.mnt) && (old->path.dentry == new->path.dentry)) return true; -- cgit v1.2.3 From 8b99c3ccf735a2294c7842d236caa42e543e2c95 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Sat, 24 Mar 2012 23:44:19 +0100 Subject: inotify: dont skip removal of watch descriptor if creation of ignored event failed In inotify_ignored_and_remove_idr() the removal of a watch descriptor is skipped if the allocation of an ignored event failed and we are leaking memory (the watch descriptor and the mark linked to it). This patch ensures that the watch descriptor is removed regardless of whether event creation failed or not. Signed-off-by: Lino Sanfilippo Signed-off-by: Eric Paris --- fs/notify/inotify/inotify_user.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 68f7bec1e66..a6879d16924 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -513,13 +513,13 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark, struct fsnotify_event_private_data *fsn_event_priv; int ret; + i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark); + ignored_event = fsnotify_create_event(NULL, FS_IN_IGNORED, NULL, FSNOTIFY_EVENT_NONE, NULL, 0, GFP_NOFS); if (!ignored_event) - return; - - i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark); + goto skip_send_ignore; event_priv = kmem_cache_alloc(event_priv_cachep, GFP_NOFS); if (unlikely(!event_priv)) @@ -541,9 +541,9 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark, } skip_send_ignore: - /* matches the reference taken when the event was created */ - fsnotify_put_event(ignored_event); + if (ignored_event) + fsnotify_put_event(ignored_event); /* remove this mark from the idr */ inotify_remove_from_idr(group, i_mark); -- cgit v1.2.3 From 1ca39ab9d21ac93f94b9e3eb364ea9a5cf2aba06 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 26 Mar 2012 13:07:59 -0400 Subject: inotify: automatically restart syscalls We were mistakenly returning EINTR when we found an outstanding signal. Instead we should returen ERESTARTSYS and allow the kernel to handle things the right way. Patch-from: Oleg Nesterov Signed-off-by: Eric Paris --- fs/notify/inotify/inotify_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/notify') diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index a6879d16924..463e828f1f3 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -264,7 +264,7 @@ static ssize_t inotify_read(struct file *file, char __user *buf, ret = -EAGAIN; if (file->f_flags & O_NONBLOCK) break; - ret = -EINTR; + ret = -ERESTARTSYS; if (signal_pending(current)) break; -- cgit v1.2.3 From ac5f121b8f2cad52b7671f9af872f8761b0ea1d4 Mon Sep 17 00:00:00 2001 From: Tushar Behera Date: Mon, 17 Dec 2012 15:59:40 -0800 Subject: fs/notify/inode_mark.c: make fsnotify_find_inode_mark_locked() static Fixes following sparse warning: fs/notify/inode_mark.c:127:22: warning: symbol 'fsnotify_find_inode_mark_locked' was not declared. Should it be static? Signed-off-by: Tushar Behera Cc: Eric Paris Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/inode_mark.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index b13c00ac48e..f3035691f52 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -116,8 +116,9 @@ void fsnotify_clear_inode_marks_by_group(struct fsnotify_group *group) * given a group and inode, find the mark associated with that combination. * if found take a reference to that mark and return it, else return NULL */ -struct fsnotify_mark *fsnotify_find_inode_mark_locked(struct fsnotify_group *group, - struct inode *inode) +static struct fsnotify_mark *fsnotify_find_inode_mark_locked( + struct fsnotify_group *group, + struct inode *inode) { struct fsnotify_mark *mark; struct hlist_node *pos; -- cgit v1.2.3 From be77196b809cdce8603a5aadd5e3cfabd3cbef96 Mon Sep 17 00:00:00 2001 From: Cyrill Gorcunov Date: Mon, 17 Dec 2012 16:05:12 -0800 Subject: fs, notify: add procfs fdinfo helper This allow us to print out fsnotify details such as watchee inode, device, mask and optionally a file handle. For inotify objects if kernel compiled with exportfs support the output will be | pos: 0 | flags: 02000000 | inotify wd:3 ino:9e7e sdev:800013 mask:800afce ignored_mask:0 fhandle-bytes:8 fhandle-type:1 f_handle:7e9e0000640d1b6d | inotify wd:2 ino:a111 sdev:800013 mask:800afce ignored_mask:0 fhandle-bytes:8 fhandle-type:1 f_handle:11a1000020542153 | inotify wd:1 ino:6b149 sdev:800013 mask:800afce ignored_mask:0 fhandle-bytes:8 fhandle-type:1 f_handle:49b1060023552153 If kernel compiled without exportfs support, the file handle won't be provided but inode and device only. | pos: 0 | flags: 02000000 | inotify wd:3 ino:9e7e sdev:800013 mask:800afce ignored_mask:0 | inotify wd:2 ino:a111 sdev:800013 mask:800afce ignored_mask:0 | inotify wd:1 ino:6b149 sdev:800013 mask:800afce ignored_mask:0 For fanotify the output is like | pos: 0 | flags: 04002 | fanotify flags:10 event-flags:0 | fanotify mnt_id:12 mask:3b ignored_mask:0 | fanotify ino:50205 sdev:800013 mask:3b ignored_mask:40000000 fhandle-bytes:8 fhandle-type:1 f_handle:05020500fb1d47e7 To minimize impact on general fsnotify code the new functionality is gathered in fs/notify/fdinfo.c file. Signed-off-by: Cyrill Gorcunov Acked-by: Pavel Emelyanov Cc: Oleg Nesterov Cc: Andrey Vagin Cc: Al Viro Cc: Alexey Dobriyan Cc: James Bottomley Cc: "Aneesh Kumar K.V" Cc: Alexey Dobriyan Cc: Matthew Helsley Cc: "J. Bruce Fields" Cc: "Aneesh Kumar K.V" Cc: Tvrtko Ursulin Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/Makefile | 2 +- fs/notify/fanotify/fanotify_user.c | 2 + fs/notify/fdinfo.c | 175 +++++++++++++++++++++++++++++++++++++ fs/notify/fdinfo.h | 27 ++++++ fs/notify/inotify/inotify_user.c | 2 + 5 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 fs/notify/fdinfo.c create mode 100644 fs/notify/fdinfo.h (limited to 'fs/notify') diff --git a/fs/notify/Makefile b/fs/notify/Makefile index ae5f33a6d86..96d3420d024 100644 --- a/fs/notify/Makefile +++ b/fs/notify/Makefile @@ -1,5 +1,5 @@ obj-$(CONFIG_FSNOTIFY) += fsnotify.o notification.o group.o inode_mark.o \ - mark.o vfsmount_mark.o + mark.o vfsmount_mark.o fdinfo.o obj-y += dnotify/ obj-y += inotify/ diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 6fcaeb8c902..a5cd9bba022 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -17,6 +17,7 @@ #include #include "../../mount.h" +#include "../fdinfo.h" #define FANOTIFY_DEFAULT_MAX_EVENTS 16384 #define FANOTIFY_DEFAULT_MAX_MARKS 8192 @@ -428,6 +429,7 @@ static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long ar } static const struct file_operations fanotify_fops = { + .show_fdinfo = fanotify_show_fdinfo, .poll = fanotify_poll, .read = fanotify_read, .write = fanotify_write, diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c new file mode 100644 index 00000000000..cb996179abf --- /dev/null +++ b/fs/notify/fdinfo.c @@ -0,0 +1,175 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "inotify/inotify.h" +#include "../fs/mount.h" + +#if defined(CONFIG_PROC_FS) + +#if defined(CONFIG_INOTIFY_USER) || defined(CONFIG_FANOTIFY) + +static int show_fdinfo(struct seq_file *m, struct file *f, + int (*show)(struct seq_file *m, struct fsnotify_mark *mark)) +{ + struct fsnotify_group *group = f->private_data; + struct fsnotify_mark *mark; + int ret = 0; + + spin_lock(&group->mark_lock); + list_for_each_entry(mark, &group->marks_list, g_list) { + ret = show(m, mark); + if (ret) + break; + } + spin_unlock(&group->mark_lock); + return ret; +} + +#if defined(CONFIG_EXPORTFS) +static int show_mark_fhandle(struct seq_file *m, struct inode *inode) +{ + struct { + struct file_handle handle; + u8 pad[64]; + } f; + int size, ret, i; + + f.handle.handle_bytes = sizeof(f.pad); + size = f.handle.handle_bytes >> 2; + + ret = exportfs_encode_inode_fh(inode, (struct fid *)f.handle.f_handle, &size, 0); + if ((ret == 255) || (ret == -ENOSPC)) { + WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret); + return 0; + } + + f.handle.handle_type = ret; + f.handle.handle_bytes = size * sizeof(u32); + + ret = seq_printf(m, "fhandle-bytes:%x fhandle-type:%x f_handle:", + f.handle.handle_bytes, f.handle.handle_type); + + for (i = 0; i < f.handle.handle_bytes; i++) + ret |= seq_printf(m, "%02x", (int)f.handle.f_handle[i]); + + return ret; +} +#else +static int show_mark_fhandle(struct seq_file *m, struct inode *inode) +{ + return 0; +} +#endif + +#ifdef CONFIG_INOTIFY_USER + +static int inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) +{ + struct inotify_inode_mark *inode_mark; + struct inode *inode; + int ret = 0; + + if (!(mark->flags & (FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_INODE))) + return 0; + + inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark); + inode = igrab(mark->i.inode); + if (inode) { + ret = seq_printf(m, "inotify wd:%x ino:%lx sdev:%x " + "mask:%x ignored_mask:%x ", + inode_mark->wd, inode->i_ino, + inode->i_sb->s_dev, + mark->mask, mark->ignored_mask); + ret |= show_mark_fhandle(m, inode); + ret |= seq_putc(m, '\n'); + iput(inode); + } + + return ret; +} + +int inotify_show_fdinfo(struct seq_file *m, struct file *f) +{ + return show_fdinfo(m, f, inotify_fdinfo); +} + +#endif /* CONFIG_INOTIFY_USER */ + +#ifdef CONFIG_FANOTIFY + +static int fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) +{ + struct inode *inode; + int ret = 0; + + if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) + return 0; + + if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) { + inode = igrab(mark->i.inode); + if (!inode) + goto out; + ret = seq_printf(m, "fanotify ino:%lx sdev:%x " + "mask:%x ignored_mask:%x ", + inode->i_ino, inode->i_sb->s_dev, + mark->mask, mark->ignored_mask); + ret |= show_mark_fhandle(m, inode); + ret |= seq_putc(m, '\n'); + iput(inode); + } else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT) { + struct mount *mnt = real_mount(mark->m.mnt); + + ret = seq_printf(m, "fanotify mnt_id:%x mask:%x " + "ignored_mask:%x\n", + mnt->mnt_id, mark->mask, mark->ignored_mask); + } +out: + return ret; +} + +int fanotify_show_fdinfo(struct seq_file *m, struct file *f) +{ + struct fsnotify_group *group = f->private_data; + unsigned int flags = 0; + + switch (group->priority) { + case FS_PRIO_0: + flags |= FAN_CLASS_NOTIF; + break; + case FS_PRIO_1: + flags |= FAN_CLASS_CONTENT; + break; + case FS_PRIO_2: + flags |= FAN_CLASS_PRE_CONTENT; + break; + } + + if (group->max_events == UINT_MAX) + flags |= FAN_UNLIMITED_QUEUE; + + if (group->fanotify_data.max_marks == UINT_MAX) + flags |= FAN_UNLIMITED_MARKS; + + seq_printf(m, "fanotify flags:%x event-flags:%x\n", + flags, group->fanotify_data.f_flags); + + return show_fdinfo(m, f, fanotify_fdinfo); +} + +#endif /* CONFIG_FANOTIFY */ + +#endif /* CONFIG_INOTIFY_USER || CONFIG_FANOTIFY */ + +#endif /* CONFIG_PROC_FS */ diff --git a/fs/notify/fdinfo.h b/fs/notify/fdinfo.h new file mode 100644 index 00000000000..556afda990e --- /dev/null +++ b/fs/notify/fdinfo.h @@ -0,0 +1,27 @@ +#ifndef __FSNOTIFY_FDINFO_H__ +#define __FSNOTIFY_FDINFO_H__ + +#include +#include + +struct seq_file; +struct file; + +#ifdef CONFIG_PROC_FS + +#ifdef CONFIG_INOTIFY_USER +extern int inotify_show_fdinfo(struct seq_file *m, struct file *f); +#endif + +#ifdef CONFIG_FANOTIFY +extern int fanotify_show_fdinfo(struct seq_file *m, struct file *f); +#endif + +#else /* CONFIG_PROC_FS */ + +#define inotify_show_fdinfo NULL +#define fanotify_show_fdinfo NULL + +#endif /* CONFIG_PROC_FS */ + +#endif /* __FSNOTIFY_FDINFO_H__ */ diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index c311dda054a..36cb013c7c1 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -40,6 +40,7 @@ #include #include "inotify.h" +#include "../fdinfo.h" #include @@ -335,6 +336,7 @@ static long inotify_ioctl(struct file *file, unsigned int cmd, } static const struct file_operations inotify_fops = { + .show_fdinfo = inotify_show_fdinfo, .poll = inotify_poll, .read = inotify_read, .fasync = inotify_fasync, -- cgit v1.2.3 From e6dbcafb744ab94a94142a6e721e16330397fad8 Mon Sep 17 00:00:00 2001 From: Cyrill Gorcunov Date: Mon, 17 Dec 2012 16:05:16 -0800 Subject: fs, fanotify: add @mflags field to fanotify output The kernel keeps FAN_MARK_IGNORED_SURV_MODIFY bit separately from fsnotify_mark::mask|ignored_mask thus put it in @mflags (mark flags) field so the user-space reader will be able to detect if such bit were used on mark creation procedure. | pos: 0 | flags: 04002 | fanotify flags:10 event-flags:0 | fanotify mnt_id:12 mflags:40 mask:38 ignored_mask:40000003 | fanotify ino:4f969 sdev:800013 mflags:0 mask:3b ignored_mask:40000000 fhandle-bytes:8 fhandle-type:1 f_handle:69f90400c275b5b4 Signed-off-by: Cyrill Gorcunov Cc: Pavel Emelyanov Cc: Oleg Nesterov Cc: Andrey Vagin Cc: Al Viro Cc: Alexey Dobriyan Cc: James Bottomley Cc: "Aneesh Kumar K.V" Cc: Matthew Helsley Cc: "J. Bruce Fields" Cc: Tvrtko Ursulin Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/fdinfo.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c index cb996179abf..514c4b81483 100644 --- a/fs/notify/fdinfo.c +++ b/fs/notify/fdinfo.c @@ -111,29 +111,33 @@ int inotify_show_fdinfo(struct seq_file *m, struct file *f) static int fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) { + unsigned int mflags = 0; struct inode *inode; int ret = 0; if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) return 0; + if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY) + mflags |= FAN_MARK_IGNORED_SURV_MODIFY; + if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) { inode = igrab(mark->i.inode); if (!inode) goto out; ret = seq_printf(m, "fanotify ino:%lx sdev:%x " - "mask:%x ignored_mask:%x ", + "mflags:%x mask:%x ignored_mask:%x ", inode->i_ino, inode->i_sb->s_dev, - mark->mask, mark->ignored_mask); + mflags, mark->mask, mark->ignored_mask); ret |= show_mark_fhandle(m, inode); ret |= seq_putc(m, '\n'); iput(inode); } else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT) { struct mount *mnt = real_mount(mark->m.mnt); - ret = seq_printf(m, "fanotify mnt_id:%x mask:%x " - "ignored_mask:%x\n", - mnt->mnt_id, mark->mask, mark->ignored_mask); + ret = seq_printf(m, "fanotify mnt_id:%x mflags:%x mask:%x " + "ignored_mask:%x\n", mnt->mnt_id, mflags, + mark->mask, mark->ignored_mask); } out: return ret; -- cgit v1.2.3