Re: [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs

Qu Wenruo
2025-08-20T05:45:19Z

在 2025/8/20 14:30, Nirjhar Roy (IBM) 写道:
>
> On 8/20/25 03:59, Qu Wenruo wrote:
>>
>>
>> 在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
>>> When tested with block size/node size 64K on btrfs, then the test fails
>>> with the folllowing error:
>>>       QA output created by 563
>>>       read/write
>>>       read is in range
>>>      -write is in range
>>>      +write has value of 8855552
>>>      +write is NOT in range 7969177.6 .. 8808038.4
>>>       write -> read/write
>>>      ...
>>> The slight increase in the amount of bytes that are written is because
>>> of the increase in the the nodesize(metadata) and hence it exceeds
>>> the tolerance limit slightly. Fix this by increasing the iosize.
>>> Increasing the iosize increases the tolerance range and covers the
>>> tolerance for btrfs higher node sizes.
>>>
>>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>
>> Looks good to me.
>>
>> Just want to add some more analyze for the failure case.
>>
>> For the test it writes 8M data with 5% tolerance (around 408K) with
>> the total writes.
>>
>> With 64K block size (and it implies 64K metadata size) for btrfs, it
>> mean we can have at most 7 tree blocks of writes plus two super blocks
>> updated.
>>
>> Considering the default metadata profile is DUP, doubling the metadata
>> writes, the real limit is only 3 tree blocks.
>>
>> And when doing fsync, btrfs will create at least 2 new tree blocks,
>> one for the log tree root, and one for the log tree of the subvolume.
>>
>> This is still inside the tolerance, thus the test case can still pass
>> for a lot of cases.
>>
>> But if a full transaction commit is triggered, btrfs will need to
>> create at least 3 new tree blocks for root, extent and subvolume tree.
>> Depending on the mkfs config, it will increase to 7 tree blocks (free
>> space tree, block group tree, csum tree and uuid tree created at mount).
>>
>> All are exceeding the tolerance limit.
>>
>> Doubling the io size will make the tolerance to be 8 tree blocks,
>> covering the worst case of 64K metadata sized btrfs, at least for now.
>
> Thank you for the detailed analysis. I will add this analysis in the
> commit message and address the comment for generic/274[1] and add your
> RBs in the next revision.
>
> A couple of questions for the above explanation:
>
> Doubling the iosize to 16M with 5% tolerance is around 819k. So, with
> 64k blocks, it turns out to be 819k/64k = 12. Considering DUP, it should
> be approximately 12/2=6 blocks, but you are saying 8. Can you please
> explain this part a bit?

My bad, wrong calculation.

The original 8M tolerance is only for 6 tree blocks, with DUP it reduced
to 3. Thus a full commit transaction will always fail the tolerance check.

Doubled to 16M iosize, the tolerance is exactly what you said, 12 tree
blocks not 16, and with DUP into consideration it's 6.

With 6 tree blocks tolerance, it's borderline for a full commit transaction.

The recent default mkfs config means 5 tree blocks (root, extent,
subvolume, csum and free space), and with incoming new default bgt free
it will be exactly at the boundary, but should still pass for now.

Thanks,
Qu

>
> [1] https://lore.kernel.org/all/0a10a9b0-a55c-4607-
> be0b-7f7f01c2d729@suse.com/
>
> --NR
>
>>
>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>
>> Thanks,
>> Qu
>>
>>> ---
>>>   tests/generic/563 | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/generic/563 b/tests/generic/563
>>> index 89a71aa4..6cb9ddb0 100755
>>> --- a/tests/generic/563
>>> +++ b/tests/generic/563
>>> @@ -43,7 +43,7 @@ _require_block_device $SCRATCH_DEV
>>>   _require_non_zoned_device ${SCRATCH_DEV}
>>>     cgdir=$CGROUP2_PATH
>>> -iosize=$((1024 * 1024 * 8))
>>> +iosize=$((1024 * 1024 * 16))
>>>     # Check cgroup read/write charges against expected values. Allow
>>> for some
>>>   # tolerance as different filesystems seem to account slightly
>>> differently.
>>