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

Brian Foster
2019-07-05T14:52:14Z

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__ */
>