Re: [PATCH 4/6] xfs: refactor attr scrub memory allocation function

[Brian Foster <bfoster@redhat.com>]

On Wed, Jun 26, 2019 at 01:46:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move the code that allocates memory buffers for the extended attribute
> scrub code into a separate function so we can reduce memory allocations
> in the next patch.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/scrub/attr.c |   33 ++++++++++++++++++++++++---------
>  fs/xfs/scrub/attr.h |    2 ++
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index fd16eb3fa003..c20b6da1db84 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -31,26 +31,41 @@
>  #include <linux/posix_acl_xattr.h>
>  #include <linux/xattr.h>
>  
> -/* Set us up to scrub an inode's extended attributes. */
> +/* Allocate enough memory to hold an attr value and attr block bitmaps. */
>  int
> -xchk_setup_xattr(
> +xchk_setup_xattr_buf(
>  	struct xfs_scrub	*sc,
> -	struct xfs_inode	*ip)
> +	size_t			value_size)
>  {
>  	size_t			sz;
>  
>  	/*
> -	 * Allocate the buffer without the inode lock held.  We need enough
> -	 * space to read every xattr value in the file or enough space to
> -	 * hold three copies of the xattr free space bitmap.  (Not both at
> -	 * the same time.)
> +	 * We need enough space to read an xattr value from the file or enough
> +	 * space to hold three copies of the xattr free space bitmap.  We don't
> +	 * need the buffer space for both purposes at the same time.
>  	 */
> -	sz = max_t(size_t, XATTR_SIZE_MAX, 3 * sizeof(long) *
> -			BITS_TO_LONGS(sc->mp->m_attr_geo->blksize));
> +	sz = 3 * sizeof(long) * BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
> +	sz = max_t(size_t, sz, value_size);
> +
>  	sc->buf = kmem_zalloc_large(sz, KM_SLEEP);
>  	if (!sc->buf)
>  		return -ENOMEM;
>  
> +	return 0;
> +}
> +
> +/* Set us up to scrub an inode's extended attributes. */
> +int
> +xchk_setup_xattr(
> +	struct xfs_scrub	*sc,
> +	struct xfs_inode	*ip)
> +{
> +	int			error;
> +
> +	error = xchk_setup_xattr_buf(sc, XATTR_SIZE_MAX);
> +	if (error)
> +		return error;
> +
>  	return xchk_setup_inode_contents(sc, ip, 0);
>  }
>  
> diff --git a/fs/xfs/scrub/attr.h b/fs/xfs/scrub/attr.h
> index 88bb5e29c60c..27e879aeaafc 100644
> --- a/fs/xfs/scrub/attr.h
> +++ b/fs/xfs/scrub/attr.h
> @@ -62,4 +62,6 @@ xchk_xattr_dstmap(
>  			BITS_TO_LONGS(sc->mp->m_attr_geo->blksize);
>  }
>  
> +int xchk_setup_xattr_buf(struct xfs_scrub *sc, size_t value_size);
> +
>  #endif	/* __XFS_SCRUB_ATTR_H__ */
>