Re: [PATCH v3 11/16] fsnotify: allow adding an inode mark without pinning inode

[Jan Kara <jack@suse.cz>]

On Wed 13-04-22 12:09:30, Amir Goldstein wrote:
> fsnotify_add_mark() and variants implicitly take a reference on inode
> when attaching a mark to an inode.
> 
> Make that behavior opt-out with the mark flag FSNOTIFY_MARK_FLAG_NO_IREF.
> 
> Instead of taking the inode reference when attaching connector to inode
> and dropping the inode reference when detaching connector from inode,
> take the inode reference on attach of the first mark that wants to hold
> an inode reference and drop the inode reference on detach of the last
> mark that wants to hold an inode reference.
> 
> Backends can "upgrade" an existing mark to take an inode reference, but
> cannot "downgrade" a mark with inode reference to release the refernce.
> 
> This leaves the choice to the backend whether or not to pin the inode
> when adding an inode mark.
> 
> This is intended to be used when adding a mark with ignored mask that is
> used for optimization in cases where group can afford getting unneeded
> events and reinstate the mark with ignored mask when inode is accessed
> again after being evicted.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Just two nits below.

> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index 7120918d8251..e38cb241536f 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -116,20 +116,67 @@ __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn)
>  	return *fsnotify_conn_mask_p(conn);
>  }
>  
> -static void __fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
> +/*
> + * Update the proxy refcount on inode maintainted by connector.
> + *
> + * When it's time to drop the proxy refcount, clear the HAS_IREF flag
> + * and return the inode object.  fsnotify_drop_object() will be resonsible
> + * for doing iput() outside of spinlocks when last mark that wanted iref
> + * is detached.
> + *
> + * Note that the proxy refcount is NOT dropped if backend only sets the
> + * NO_IREF mark flag and does detach the mark!
> + */

This comment seems outdated - still speaking about proxy refcount which
does not exist anymore...

> +static void fsnotify_get_inode_ref(struct inode *inode)
> +{
> +	ihold(inode);
> +	atomic_long_inc(&inode->i_sb->s_fsnotify_connectors);
> +}
> +
> @@ -505,6 +551,7 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
>  		return -ENOMEM;
>  	spin_lock_init(&conn->lock);
>  	INIT_HLIST_HEAD(&conn->list);
> +	conn->flags = 0;

Why this? We init conn->flags just a bit later...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR