Re: [PATCH 13/30] xfs: remove the xfs_inode argument to xfs_attr_get_ilocked

From: Chandan Rajendra <chandan@linux.ibm.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org,
	Allison Collins <allison.henderson@oracle.com>,
	"Darrick J . Wong" <darrick.wong@oracle.com>
Subject: Re: [PATCH 13/30] xfs: remove the xfs_inode argument to xfs_attr_get_ilocked
Date: Fri, 07 Feb 2020 18:49:18 +0530	[thread overview]
Message-ID: <4456617.JWiNkdPKL8@localhost.localdomain> (raw)
In-Reply-To: <20200129170310.51370-14-hch@lst.de>

On Wednesday, January 29, 2020 10:32 PM Christoph Hellwig wrote: 
> The inode can easily be derived from the args structure.  Also
> don't bother with else statements after early returns.
>

The newly introduced changes logically match with the code flow that existed
earlier.

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c | 15 +++++++--------
>  fs/xfs/libxfs/xfs_attr.h |  2 +-
>  fs/xfs/scrub/attr.c      |  2 +-
>  3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 288b39e81efd..fd095e3d4a9a 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -77,19 +77,18 @@ xfs_inode_hasattr(
>   */
>  int
>  xfs_attr_get_ilocked(
> -	struct xfs_inode	*ip,
>  	struct xfs_da_args	*args)
>  {
> -	ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
> +	ASSERT(xfs_isilocked(args->dp, XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
> 
> -	if (!xfs_inode_hasattr(ip))
> +	if (!xfs_inode_hasattr(args->dp))
>  		return -ENOATTR;
> -	else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
> +
> +	if (args->dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
>  		return xfs_attr_shortform_getvalue(args);
> -	else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK))
> +	if (xfs_bmap_one_block(args->dp, XFS_ATTR_FORK))
>  		return xfs_attr_leaf_get(args);
> -	else
> -		return xfs_attr_node_get(args);
> +	return xfs_attr_node_get(args);
>  }
> 
>  /*
> @@ -133,7 +132,7 @@ xfs_attr_get(
>  		args->op_flags |= XFS_DA_OP_ALLOCVAL;
> 
>  	lock_mode = xfs_ilock_attr_map_shared(args->dp);
> -	error = xfs_attr_get_ilocked(args->dp, args);
> +	error = xfs_attr_get_ilocked(args);
>  	xfs_iunlock(args->dp, lock_mode);
> 
>  	/* on error, we have to clean up allocated value buffers */
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index be77d13a2902..b8c4ed27f626 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -145,7 +145,7 @@ int xfs_attr_inactive(struct xfs_inode *dp);
>  int xfs_attr_list_int_ilocked(struct xfs_attr_list_context *);
>  int xfs_attr_list_int(struct xfs_attr_list_context *);
>  int xfs_inode_hasattr(struct xfs_inode *ip);
> -int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
> +int xfs_attr_get_ilocked(struct xfs_da_args *args);
>  int xfs_attr_get(struct xfs_da_args *args);
>  int xfs_attr_set(struct xfs_da_args *args);
>  int xfs_attr_set_args(struct xfs_da_args *args);
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index d804558cdbca..f983c2b969e0 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -162,7 +162,7 @@ xchk_xattr_listent(
>  	args.value = xchk_xattr_valuebuf(sx->sc);
>  	args.valuelen = valuelen;
> 
> -	error = xfs_attr_get_ilocked(context->dp, &args);
> +	error = xfs_attr_get_ilocked(&args);
>  	if (!xchk_fblock_process_error(sx->sc, XFS_ATTR_FORK, args.blkno,
>  			&error))
>  		goto fail_xref;
> 

-- 
chandan