Re: [PATCH v3 11/13] fsnotify: add fsnotify_connector_inode() wrapper

[Jan Kara <jack@suse.cz>]

On Fri 20-04-18 16:10:59, Amir Goldstein wrote:
> Soon we will be changing the way to dereference an inode object from
> connector object. Introduce a wrapper to abstract this dereference.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

I don't think this helper helps us much. It just changes (after the
following patch):

fsnotify_obj_inode(conn->obj)

to

fsnotify_connector_inode(conn).

So the result is even longer to type and we are speaking only about 6
places so it's not like this is going to make the following patch that much
easier. Also mounts don't have similar helper. I'll just drop this patch
and fixup the following one for now. If you really want the helper, then we
can talk about that but I find fsnotify_mark_inode() helper more useful -
most places actually want mark -> inode transition and that's also a bit
longer (fsnotify_obj_inode(mark->conn->obj)).

								Honza

> ---
>  fs/notify/fdinfo.c               |  4 ++--
>  fs/notify/mark.c                 |  3 ++-
>  include/linux/fsnotify_backend.h |  6 ++++++
>  kernel/audit_tree.c              | 11 ++++++-----
>  4 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
> index 10aac1942c9f..b3cf03e347a8 100644
> --- a/fs/notify/fdinfo.c
> +++ b/fs/notify/fdinfo.c
> @@ -81,7 +81,7 @@ static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
>  		return;
>  
>  	inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark);
> -	inode = igrab(mark->connector->inode);
> +	inode = igrab(fsnotify_connector_inode(mark->connector));
>  	if (inode) {
>  		/*
>  		 * IN_ALL_EVENTS represents all of the mask bits
> @@ -117,7 +117,7 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
>  		mflags |= FAN_MARK_IGNORED_SURV_MODIFY;
>  
>  	if (mark->connector->type == FSNOTIFY_OBJ_TYPE_INODE) {
> -		inode = igrab(mark->connector->inode);
> +		inode = igrab(fsnotify_connector_inode(mark->connector));
>  		if (!inode)
>  			return;
>  		seq_printf(m, "fanotify ino:%lx sdev:%x mflags:%x mask:%x ignored_mask:%x ",
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index 771178c46eeb..7103a0a442e7 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -140,7 +140,8 @@ void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
>  	__fsnotify_recalc_mask(conn);
>  	spin_unlock(&conn->lock);
>  	if (conn->type == FSNOTIFY_OBJ_TYPE_INODE)
> -		__fsnotify_update_child_dentry_flags(conn->inode);
> +		__fsnotify_update_child_dentry_flags(
> +				fsnotify_connector_inode(conn));
>  }
>  
>  /* Free all connectors queued for freeing once SRCU period ends */
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index b2826dd1e68c..6e9397c16124 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -279,6 +279,12 @@ static inline struct inode *fsnotify_obj_inode(struct fsnotify_obj *obj)
>  	return container_of(obj, struct inode, i_fsnotify);
>  }
>  
> +static inline struct inode *fsnotify_connector_inode(
> +		struct fsnotify_mark_connector *conn)
> +{
> +	return conn->inode;
> +}
> +
>  /*
>   * A mark is simply an object attached to an in core inode which allows an
>   * fsnotify listener to indicate they are either no longer interested in events
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index eadee98f5a12..529e4757f4e1 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -183,7 +183,7 @@ static unsigned long chunk_to_key(struct audit_chunk *chunk)
>  	 */
>  	if (WARN_ON_ONCE(!chunk->mark.connector))
>  		return 0;
> -	return (unsigned long)chunk->mark.connector->inode;
> +	return (unsigned long)fsnotify_connector_inode(chunk->mark.connector);
>  }
>  
>  static inline struct list_head *chunk_hash(unsigned long key)
> @@ -258,7 +258,7 @@ static void untag_chunk(struct node *p)
>  	spin_lock(&entry->lock);
>  	/*
>  	 * mark_mutex protects mark from getting detached and thus also from
> -	 * mark->connector->inode getting NULL.
> +	 * mark->connector->obj getting NULL.
>  	 */
>  	if (chunk->dead || !(entry->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) {
>  		spin_unlock(&entry->lock);
> @@ -288,8 +288,9 @@ static void untag_chunk(struct node *p)
>  	if (!new)
>  		goto Fallback;
>  
> -	if (fsnotify_add_inode_mark_locked(&new->mark, entry->connector->inode,
> -					   1)) {
> +	if (fsnotify_add_inode_mark_locked(&new->mark,
> +				fsnotify_connector_inode(entry->connector),
> +				1)) {
>  		fsnotify_put_mark(&new->mark);
>  		goto Fallback;
>  	}
> @@ -434,7 +435,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
>  	}
>  
>  	if (fsnotify_add_inode_mark_locked(chunk_entry,
> -			     old_entry->connector->inode, 1)) {
> +			fsnotify_connector_inode(old_entry->connector), 1)) {
>  		spin_unlock(&old_entry->lock);
>  		mutex_unlock(&old_entry->group->mark_mutex);
>  		fsnotify_put_mark(chunk_entry);
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR