On Wed 15-03-17 22:03:05, Amir Goldstein wrote:
> On Wed, Mar 15, 2017 at 12:46 PM, Jan Kara <jack@xxxxxxx> wrote:
> > Move locking of locks protecting a list of marks into
> > fsnotify_recalc_mask(). This reduces code churn in the following patch
> > which changes the lock protecting the list of marks.
> >
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > ---
>
> [...]
>
> > diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> > index e8c707d07f9f..e7929539203a 100644
> > --- a/fs/notify/mark.c
> > +++ b/fs/notify/mark.c
> > @@ -105,18 +105,45 @@ void fsnotify_put_mark(struct fsnotify_mark *mark)
> > }
> > }
> >
> > -/* Calculate mask of events for a list of marks */
> > -u32 fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
> > +static void __fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
> > {
> > u32 new_mask = 0;
> > struct fsnotify_mark *mark;
> >
> > + hlist_for_each_entry(mark, &conn->list, obj_list) {
> > + if (mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)
>
> How is this added flag check related to this patch?
> Leftover from future patch?
Yeah, that flag check will make sense only in later patch. At this point of
the series non-attached marks cannot be see on conn->list. I'll move it.
> > + new_mask |= mark->mask;
> > + }
> > + if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
> > + conn->inode->i_fsnotify_mask = new_mask;
> > + else if (conn->flags & FSNOTIFY_OBJ_TYPE_VFSMOUNT)
> > + real_mount(conn->mnt)->mnt_fsnotify_mask = new_mask;
> > +}
> > +
> > +/*
> > + * Calculate mask of events for a list of marks. The caller must make sure list
> > + * cannot disappear under us (usually by holding a mark->lock or
> > + * mark->group->mark_mutex for a mark on this list).
> > + */
> > +void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
> > +{
> > if (!conn)
> > - return 0;
> > + return;
> > +
> > + if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE)
> > + spin_lock(&conn->inode->i_lock);
> > + else
> > + spin_lock(&conn->mnt->mnt_root->d_lock);
> > + __fsnotify_recalc_mask(conn);
> > + if (conn->flags & FSNOTIFY_OBJ_TYPE_INODE) {
> > + struct inode *inode = igrab(conn->inode);
> >
>
> igrab change here related to this patch or belongs to future patch?
> If belongs here, better drop a word about it in commit message.
It is making fsnotify_recalc_mask() more self-contained - with igrab()
it does not have to assume the caller has somehow pinned the inode. As such
it is not directly related to this locking change so I'll move it to a
separate commit with appropriate message.
Honza
> > - hlist_for_each_entry(mark, &conn->list, obj_list)
> > - new_mask |= mark->mask;
> > - return new_mask;
> > + spin_unlock(&inode->i_lock);
> > + __fsnotify_update_child_dentry_flags(inode);
> > + iput(inode);
> > + } else {
> > + spin_unlock(&conn->mnt->mnt_root->d_lock);
> > + }
> > }
> >
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
