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

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 8/20/25 11:15, Qu Wenruo wrote:
>
>
> 在 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.
>
Okay, it makes sense now. Thank you for the detailed explanation.

--NR

> 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.
>>>
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore