On Wed 15-03-17 22:08:00, Amir Goldstein wrote:
> On Wed, Mar 15, 2017 at 12:46 PM, Jan Kara <jack@xxxxxxx> wrote:
> > Move locking of a mark list into fsnotify_find_mark(). This reduces code
> > churn in the following patch changing lock protecting the list.
> >
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > ---
> > fs/notify/inode_mark.c | 8 +-------
> > fs/notify/mark.c | 8 ++++++++
> > fs/notify/vfsmount_mark.c | 7 +------
> > 3 files changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
> > index 9b2f4e6eb8eb..f05fc49b8242 100644
> > --- a/fs/notify/inode_mark.c
> > +++ b/fs/notify/inode_mark.c
> > @@ -71,13 +71,7 @@ void fsnotify_clear_inode_marks_by_group(struct fsnotify_group *group)
> > struct fsnotify_mark *fsnotify_find_inode_mark(struct fsnotify_group *group,
> > struct inode *inode)
> > {
> > - struct fsnotify_mark *mark;
> > -
> > - spin_lock(&inode->i_lock);
> > - mark = fsnotify_find_mark(inode->i_fsnotify_marks, group);
> > - spin_unlock(&inode->i_lock);
> > -
> > - return mark;
> > + return fsnotify_find_mark(inode->i_fsnotify_marks, group);
> > }
> >
> > /**
> > diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> > index e7929539203a..1113dd8b2f82 100644
> > --- a/fs/notify/mark.c
> > +++ b/fs/notify/mark.c
> > @@ -478,16 +478,24 @@ struct fsnotify_mark *fsnotify_find_mark(struct fsnotify_mark_connector *conn,
> > struct fsnotify_group *group)
> > {
> > struct fsnotify_mark *mark;
> > + spinlock_t *lock;
> >
> > if (!conn)
> > return NULL;
> >
> > + if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
> > + lock = &conn->inode->i_lock;
> > + else
> > + lock = &conn->mnt->mnt_root->d_lock;
> > + spin_lock(lock);
>
> This is becoming a repeating pattern.
> Perhaps:
>
> lock = fsnotify_object_lock(conn);
Yes, but it will all go away in couple of patches so I don't think it's
worth the churn in the patch series...
Honza
>
> > hlist_for_each_entry(mark, &conn->list, obj_list) {
> > if (mark->group == group) {
> > fsnotify_get_mark(mark);
> > + spin_unlock(lock);
> > return mark;
> > }
> > }
> > + spin_unlock(lock);
> > return NULL;
> > }
> >
> > diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c
> > index ffe0d7098cba..3476ee44b2c5 100644
> > --- a/fs/notify/vfsmount_mark.c
> > +++ b/fs/notify/vfsmount_mark.c
> > @@ -65,11 +65,6 @@ struct fsnotify_mark *fsnotify_find_vfsmount_mark(struct fsnotify_group *group,
> > struct vfsmount *mnt)
> > {
> > struct mount *m = real_mount(mnt);
> > - struct fsnotify_mark *mark;
> >
> > - spin_lock(&mnt->mnt_root->d_lock);
> > - mark = fsnotify_find_mark(m->mnt_fsnotify_marks, group);
> > - spin_unlock(&mnt->mnt_root->d_lock);
> > -
> > - return mark;
> > + return fsnotify_find_mark(m->mnt_fsnotify_marks, group);
> > }
> > --
> > 2.10.2
> >
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
