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@xxxxxxxxx>
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@xxxxxxxx>
SUSE Labs, CR
