On Thu, Mar 16, 2017 at 12:11 PM, Jan Kara <jack@xxxxxxx> wrote:
> On Thu 16-03-17 09:10:56, Jan Kara wrote:
>> > > + 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.
>
> In the end, I've decided to just completely drop this change (as I've
> verified no caller really needs it) and added a comment explaining how
> callers must make sure inode is not going away under us.
>
The less the better.
Reviewed commit 1f9c069 on your pushed branch, so
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
