Re: [PATCH 04/16] xfs: remove an extra buffer allocation in xfs_dir2_sf_to_block

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Apr 30, 2024 at 02:49:14PM +0200, Christoph Hellwig wrote:
> Make use of the new ability to call xfs_bmap_local_to_extents_empty on
> a non-empty local fork to reuse the old inode fork data to build the
> directory block to replace the local temporary buffer.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks pretty straightforward,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_dir2_block.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 035a54dbdd7586..20d4e86e14ab08 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -1097,8 +1097,7 @@ xfs_dir2_sf_to_block(
>  	int			newoffset;	/* offset from current entry */
>  	unsigned int		offset = geo->data_entry_offset;
>  	xfs_dir2_sf_entry_t	*sfep;		/* sf entry pointer */
> -	struct xfs_dir2_sf_hdr	*oldsfp = ifp->if_data;
> -	xfs_dir2_sf_hdr_t	*sfp;		/* shortform header  */
> +	struct xfs_dir2_sf_hdr	*sfp = ifp->if_data;
>  	__be16			*tagp;		/* end of data entry */
>  	struct xfs_name		name;
>  
> @@ -1106,17 +1105,10 @@ xfs_dir2_sf_to_block(
>  
>  	ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
>  	ASSERT(ifp->if_bytes == dp->i_disk_size);
> -	ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(oldsfp->i8count));
> +	ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
>  
> -	/*
> -	 * Copy the directory into a temporary buffer.
> -	 * Then pitch the incore inode data so we can make extents.
> -	 */
> -	sfp = kmalloc(ifp->if_bytes, GFP_KERNEL | __GFP_NOFAIL);
> -	memcpy(sfp, oldsfp, ifp->if_bytes);
> +	sfp = xfs_bmap_local_to_extents_empty(tp, dp, XFS_DATA_FORK);
>  
> -	xfs_idata_realloc(dp, -ifp->if_bytes, XFS_DATA_FORK);
> -	xfs_bmap_local_to_extents_empty(tp, dp, XFS_DATA_FORK);
>  	dp->i_disk_size = 0;
>  
>  	/*
> -- 
> 2.39.2
> 
>