Re: [PATCH 05/13] xfs: introduce forced allocation alignment

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

On Fri, Jun 21, 2024 at 10:05:32AM +0000, John Garry wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When forced allocation alignment is specified, the extent will
> be aligned to the extent size hint size rather than stripe
> alignment. If aligned allocation cannot be done, then the allocation
> is failed rather than attempting non-aligned fallbacks.
> 
> Note: none of the per-inode force align configuration is present
> yet, so this just triggers off an "always false" wrapper function
> for the moment.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: John Garry <john.g.garry@oracle.com>

Pretty straightfoward!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_alloc.h |  1 +
>  fs/xfs/libxfs/xfs_bmap.c  | 29 +++++++++++++++++++++++------
>  fs/xfs/xfs_inode.h        |  5 +++++
>  3 files changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index aa2c103d98f0..7de2e6f64882 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -66,6 +66,7 @@ typedef struct xfs_alloc_arg {
>  #define XFS_ALLOC_USERDATA		(1 << 0)/* allocation is for user data*/
>  #define XFS_ALLOC_INITIAL_USER_DATA	(1 << 1)/* special case start of file */
>  #define XFS_ALLOC_NOBUSY		(1 << 2)/* Busy extents not allowed */
> +#define XFS_ALLOC_FORCEALIGN		(1 << 3)/* forced extent alignment */
>  
>  /* freespace limit calculations */
>  unsigned int xfs_alloc_set_aside(struct xfs_mount *mp);
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 528e3cd81ee6..9131ba8113a6 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3401,9 +3401,10 @@ xfs_bmap_alloc_account(
>   * Calculate the extent start alignment and the extent length adjustments that
>   * constrain this allocation.
>   *
> - * Extent start alignment is currently determined by stripe configuration and is
> - * carried in args->alignment, whilst extent length adjustment is determined by
> - * extent size hints and is carried by args->prod and args->mod.
> + * Extent start alignment is currently determined by forced inode alignment or
> + * stripe configuration and is carried in args->alignment, whilst extent length
> + * adjustment is determined by extent size hints and is carried by args->prod
> + * and args->mod.
>   *
>   * Low level allocation code is free to either ignore or override these values
>   * as required.
> @@ -3416,11 +3417,18 @@ xfs_bmap_compute_alignments(
>  	struct xfs_mount	*mp = args->mp;
>  	xfs_extlen_t		align = 0; /* minimum allocation alignment */
>  
> -	/* stripe alignment for allocation is determined by mount parameters */
> -	if (mp->m_swidth && xfs_has_swalloc(mp))
> +	/*
> +	 * Forced inode alignment takes preference over stripe alignment.
> +	 * Stripe alignment for allocation is determined by mount parameters.
> +	 */
> +	if (xfs_inode_has_forcealign(ap->ip)) {
> +		args->alignment = xfs_get_extsz_hint(ap->ip);
> +		args->datatype |= XFS_ALLOC_FORCEALIGN;
> +	} else if (mp->m_swidth && xfs_has_swalloc(mp)) {
>  		args->alignment = mp->m_swidth;
> -	else if (mp->m_dalign)
> +	} else if (mp->m_dalign) {
>  		args->alignment = mp->m_dalign;
> +	}
>  
>  	if (ap->flags & XFS_BMAPI_COWFORK)
>  		align = xfs_get_cowextsz_hint(ap->ip);
> @@ -3607,6 +3615,11 @@ xfs_bmap_btalloc_low_space(
>  {
>  	int			error;
>  
> +	if (args->alignment > 1 && (args->datatype & XFS_ALLOC_FORCEALIGN)) {
> +		args->fsbno = NULLFSBLOCK;
> +		return 0;
> +	}
> +
>  	args->alignment = 1;
>  	if (args->minlen > ap->minlen) {
>  		args->minlen = ap->minlen;
> @@ -3658,6 +3671,8 @@ xfs_bmap_btalloc_filestreams(
>  
>  	/* Attempt non-aligned allocation if we haven't already. */
>  	if (!error && args->fsbno == NULLFSBLOCK && args->alignment > 1)  {
> +		if (args->datatype & XFS_ALLOC_FORCEALIGN)
> +			return error;
>  		args->alignment = 1;
>  		error = xfs_alloc_vextent_near_bno(args, ap->blkno);
>  	}
> @@ -3716,6 +3731,8 @@ xfs_bmap_btalloc_best_length(
>  
>  	/* Attempt non-aligned allocation if we haven't already. */
>  	if (!error && args->fsbno == NULLFSBLOCK && args->alignment > 1)  {
> +		if (args->datatype & XFS_ALLOC_FORCEALIGN)
> +			return error;
>  		args->alignment = 1;
>  		error = xfs_alloc_vextent_start_ag(args, ap->blkno);
>  	}
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 292b90b5f2ac..42f999c1106c 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -311,6 +311,11 @@ static inline bool xfs_inode_has_large_extent_counts(struct xfs_inode *ip)
>  	return ip->i_diflags2 & XFS_DIFLAG2_NREXT64;
>  }
>  
> +static inline bool xfs_inode_has_forcealign(struct xfs_inode *ip)
> +{
> +	return false;
> +}
> +
>  /*
>   * Decide if this file is a realtime file whose data allocation unit is larger
>   * than a single filesystem block.
> -- 
> 2.31.1
> 
>