Re: [PATCH 1/3] xfs: simplify extent allocation alignment

John Garry
2024-03-13T11:03:38Z

On 06/03/2024 05:20, Dave Chinner wrote:
> return false;
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 0b956f8b9d5a..aa2c103d98f0 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -46,7 +46,7 @@ typedef struct xfs_alloc_arg {
> xfs_extlen_t minleft; /* min blocks must be left after us */
> xfs_extlen_t total; /* total blocks needed in xaction */
> xfs_extlen_t alignment; /* align answer to multiple of this */
> - xfs_extlen_t minalignslop; /* slop for minlen+alignment calcs */
> + xfs_extlen_t alignslop; /* slop for alignment calcs */
> xfs_agblock_t min_agbno; /* set an agbno range for NEAR allocs */
> xfs_agblock_t max_agbno; /* ... */
> xfs_extlen_t len; /* output: actual size of extent */
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 656c95a22f2e..d56c82c07505 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3295,6 +3295,10 @@ xfs_bmap_select_minlen(
> xfs_extlen_t blen)

Hi Dave,

> {
>
> + /* Adjust best length for extent start alignment. */
> + if (blen > args->alignment)
> + blen -= args->alignment;
> +

This change seems to be causing or exposing some issue, in that I find
that I am being allocated an extent which is aligned to but not a
multiple of args->alignment.

For my test, I have forcealign=16KB and initially I write 1756 * 4096 =
7192576B to the file, so I have this:

EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL
0: [0..14079]: 42432..56511 0 (42432..56511) 14080

That is 1760 FSBs for extent #0.

Then I write 340992B from offset 7195648, and I find this:
EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL
0: [0..14079]: 42432..56511 0 (42432..56511) 14080
1: [14080..14711]: 177344..177975 0 (177344..177975) 632
2: [14712..14719]: 350720..350727 1 (171520..171527) 8

extent #1 is 79 FSBs, which is not a multiple of 16KB.

In this case, in xfs_bmap_select_minlen() I find initially blen=80
args->alignment=4, ->minlen=0, ->maxlen=80. Subsequently blen is reduced
to 76 and args->minlen is set to 76, and then
xfs_bmap_btalloc_best_length() -> xfs_alloc_vextent_start_ag() happens
to find an extent of length 79.

Removing the specific change to modify blen seems to make things ok again.

I will also note something strange which could be the issue, that being
that xfs_alloc_fix_len() does not fix this up - I thought that was its
job. Firstly, in this same scenario, in xfs_alloc_space_available() we
calculate alloc_len = args->minlen + (args->alignment - 1) +
args->alignslop = 76 + (4 - 1) + 0 = 79, and then args->maxlen = 79.
Then xfs_alloc_fix_len() allows this as args->len == args->maxlen (=79),
even though args->prod, mod = 4, 0. To me, that (args->alignment - 1)
component in calculating alloc_len is odd. I assume it is done as
default args->alignment == 1.

Anyway, let me know what you think.

Cheers,
John

> /*
> * Since we used XFS_ALLOC_FLAG_TRYLOCK in _longest_free_extent(), it is
> * possible that there is enough contiguous free space for this request.
> @@ -3310,6 +3314,7 @@ xfs_bmap_select_minlen(
> if (blen < args->maxlen)
> return blen;
> return args->maxlen;
> +