Re: [PATCH 1/5] xfs: only allow minlen allocations when near ENOSPC

[John Garry <john.g.garry@oracle.com>]

>>> -	if (available < (int)args->maxlen && !(flags & XFS_ALLOC_FLAG_CHECK)) {
>>> -		args->maxlen = available;
>>> +	alloc_len = args->maxlen + (args->alignment - 1) + args->minalignslop;
>>
>> I added some kernel logs to assist debugging, and if I am reading them
>> correctly, for ext #2 allocation we had at this point:
>>
>> longest = 46, alloc_len = 47, args->minlen=30, maxlen=32, alignslop=0
>> available=392; longest < alloc_len, so we set args->maxlen = args->minlen (=
>> 30)
> 
> Why was args->minlen set to 30? Where did that value come from? i.e.
> That is not correctly sized/aligned for force alignment - it should
> be rounded down to 16 fsbs, right?

I could not recreate this exact scenario, but another similar one 
occurred which gave:

/root/mnt2/file_2425:
  EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL
    0: [0..431]:        489600..490031    2 (131200..131631)   432
    1: [432..511]:      529408..529487    2 (171008..171087)    80
#

For allocating EXT #0, in xfs_bmap_select_minlen() we had initially 
args->minlen=0, maxlen=64 blen=70

then blen -= alignment (=16), an finally condition blen < args->maxlen 
passed and have minlen = 54


Then in xfs_alloc_space_available(), for check (longest < alloc_len) 
near the end, at that point we have:
longest=70, alloc_len=79, args->minlen=54, maxlen=64, alignslop=0 
available=155, and so we set maxlen = minlen = 54

Then in xfs_alloc_fix_len(), initially args->mod=0, prod=16, minlen=54, 
maxlen=54, len=54 rlen=54

And we end up with the 54 FSB extent.

> 
> I'm guessing that "30" is (longest - alignment) = 46 - 16 = 30? And
> then it wasn't rounded down from there?
> 
>> For ext3:
>> longest = 32, alloc_len = 17, args->minlen=2, args->maxlen=2, alignslop=0,
>> available=362; longest > alloc_len, so do nothing
> 
> Same issue here - for a forced align allocation, minlen needs to be
> bound to alignment constraints. If minlen is set like this, then the
> allocation will always return a 2 block extent if they exist.
> 
> IOWs, the allocation constraints are not correctly aligned, but the
> allocation is doing exactly what the constraints say it is allowed
> to do. Therefore the issue is in the constraint setup (args->minlen)
> for forced alignment and not the allocation code that selects a
> an args->minlen extent that is not correctly sized near ENOSPC.
> 
> I'm guessing that we need something like the (untested) patch below
> to ensure args->minlen is properly aligned....
> 
> -Dave.

For some reason that diff got excluded or mangled when I tried to reply. 
Anyway, it seems to work ok, but that's based only on a quite limited 
amount of testing.

Thanks,
John