| Message ID | 1334599360-15346-9-git-send-email-apw@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 16.04.2012 20:02, Andy Whitcroft wrote: > From: Lino Sanfilippo <LinoSanfilippo@gmx.de> > > 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 <LinoSanfilippo@gmx.de> > Signed-off-by: Eric Paris <eparis@redhat.com> > > (cherry-picked from commit fb634e66bbe4119fe5074ecf89630cdb361041d6 git://git.infradead.org/users/eparis/notify.git) > BugLink: http://bugs.launchpad.net/bugs/922906 > Signed-off-by: Andy Whitcroft <apw@canonical.com> > --- > fs/notify/mark.c | 42 +++++++++++++++++++++++++++----------- > include/linux/fsnotify_backend.h | 4 ++++ > 2 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/fs/notify/mark.c b/fs/notify/mark.c > index 716b7e2..37962c7 100644 > --- a/fs/notify/mark.c > +++ b/fs/notify/mark.c > @@ -122,18 +122,18 @@ EXPORT_SYMBOL(fsnotify_put_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; > } > > @@ -150,6 +150,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); Hm, not necessarily means anything, but here the lock is temporarily dropped while the next patch sort of seems to assume iteration over a list is save because the lock is held over the iteration... > @@ -185,6 +187,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); > } > EXPORT_SYMBOL(fsnotify_destroy_mark); > > @@ -210,14 +222,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!!!! > @@ -225,8 +238,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; > > @@ -252,8 +263,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); > > @@ -266,7 +275,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); > @@ -277,6 +285,16 @@ err: > } > EXPORT_SYMBOL(fsnotify_add_mark); > > +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 > */ > diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h > index 5b33a85..4f205fa 100644 > --- a/include/linux/fsnotify_backend.h > +++ b/include/linux/fsnotify_backend.h > @@ -408,9 +408,13 @@ extern void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask > /* attach the mark to both the group and the inode */ > extern int fsnotify_add_mark(struct fsnotify_mark *mark, struct fsnotify_group *group, > struct inode *inode, struct vfsmount *mnt, int allow_dups); > +extern int fsnotify_add_mark_locked(struct fsnotify_mark *mark, struct fsnotify_group *group, > + struct inode *inode, struct vfsmount *mnt, int allow_dups); > /* given a group and a mark, flag mark to be freed when all references are dropped */ > extern void fsnotify_destroy_mark(struct fsnotify_mark *mark, > struct fsnotify_group *group); > +extern void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, > + struct fsnotify_group *group); > /* run all the marks in a group, and clear all of the vfsmount marks */ > extern void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group); > /* run all the marks in a group, and clear all of the inode marks */
diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 716b7e2..37962c7 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -122,18 +122,18 @@ EXPORT_SYMBOL(fsnotify_put_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; } @@ -150,6 +150,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); @@ -185,6 +187,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); } EXPORT_SYMBOL(fsnotify_destroy_mark); @@ -210,14 +222,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!!!! @@ -225,8 +238,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; @@ -252,8 +263,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); @@ -266,7 +275,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); @@ -277,6 +285,16 @@ err: } EXPORT_SYMBOL(fsnotify_add_mark); +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 */ diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index 5b33a85..4f205fa 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -408,9 +408,13 @@ extern void fsnotify_set_mark_mask_locked(struct fsnotify_mark *mark, __u32 mask /* attach the mark to both the group and the inode */ extern int fsnotify_add_mark(struct fsnotify_mark *mark, struct fsnotify_group *group, struct inode *inode, struct vfsmount *mnt, int allow_dups); +extern int fsnotify_add_mark_locked(struct fsnotify_mark *mark, struct fsnotify_group *group, + struct inode *inode, struct vfsmount *mnt, int allow_dups); /* given a group and a mark, flag mark to be freed when all references are dropped */ extern void fsnotify_destroy_mark(struct fsnotify_mark *mark, struct fsnotify_group *group); +extern void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, + struct fsnotify_group *group); /* run all the marks in a group, and clear all of the vfsmount marks */ extern void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group); /* run all the marks in a group, and clear all of the inode marks */