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