Re: [PATCH v3 2/2] btrfs/291: fix state transition logic and add size requirement

On 11/05/26 8:50 pm, Zorro Lang wrote:
> On Fri, May 08, 2026 at 08:17:01PM +0530, Disha Goel wrote:
>> This patch fixes two issues in btrfs/291:
>>
>> 1. Add dynamic LOGWRITES_DEV size requirement based on SCRATCH_DEV
>>     The test creates LVM snapshots at each FUA point during replay,
>>     requiring significant space. Calculate the required size as 120%
>>     of SCRATCH_DEV size (adding 20% overhead for LVM snapshots and
>>     metadata) to ensure the test works regardless of SCRATCH_DEV size.
>>
>> 2. Fix state transition logic for verity enablement
>>     The original test assumed orphan items would always be created
>>     during verity enablement (state 0->1 transition). However, in
>>     some cases verity completes without creating orphan items,
>>     causing the test to fail with "expected to reach verity done state".
>>
>>     Fix by transitioning to state 1 when either orphan items exist
>>     OR merkle items appear, handling both verity enablement paths.
>>     Also improve state 1 validation to only check for cleared merkle
>>     items when measurement actually fails.
>>
>> The test now correctly handles verity enablement with or without
>> orphan items while maintaining crash consistency validation, and
>> works with any SCRATCH_DEV size.
>>
>> Suggested-by: Anand Jain <[email protected]>
>> Signed-off-by: Disha Goel <[email protected]>
>> ---
>> v2 -> v3:
>> - Calculate LOGWRITES_DEV size requirement dynamically based on SCRATCH_DEV
>>    size (120% of SCRATCH_DEV) instead of fixed 9GB
>> - This fixes test failures when SCRATCH_DEV > 9GB, as reported by Anand
>>
>>   tests/btrfs/291 | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/btrfs/291 b/tests/btrfs/291
>> index 122aeaa5..e5ea4b50 100755
>> --- a/tests/btrfs/291
>> +++ b/tests/btrfs/291
>> @@ -36,7 +36,9 @@ _cleanup()
>>   _require_scratch
>>   _require_test
>>   _require_loop
>> -_require_log_writes
>> +scratch_size=$(_get_device_size $SCRATCH_DEV)
>> +required_log_size=$((scratch_size * 120 / 100))
>> +_require_log_writes_sized $required_log_size
> 
> OK, more 20% space makes sense.
> 
>>   _require_dm_target snapshot
>>   _require_command $LVM_PROG lvm
>>   _require_scratch_verity
>> @@ -129,9 +131,14 @@ do
>>   	_udev_wait /dev/mapper/$vgname-$snapname
>>   
>>   	orphan=$(count_item $snap_dev ORPHAN)
>> -	[ $state -eq 0 ] && [ $orphan -gt 0 ] && state=1
>> -
>>   	pre_mount=$(count_merkle_items $snap_dev)
>> +
>> +	if [ $state -eq 0 ]; then
>> +		if [ $orphan -gt 0 ] || [ $pre_mount -gt 0 ]; then
> 
> Hm, this's this exception you metioned in commit log, so if we find
> merkle items, no matter where's it from, we need to deal with them
> later, so set state to 1.
> 
>> +			state=1
>> +		fi
>> +	fi
>> +
>>   	_mount $snap_dev $SCRATCH_MNT || _fail "mount failed at entry $cur"
>>   	fsverity measure $SCRATCH_MNT/fsv >>$seqres.full 2>&1
>>   	measured=$?
>> @@ -143,8 +150,10 @@ do
>>   	echo "entry: $cur, state: $state, orphan: $orphan, pre_mount: $pre_mount, post_mount: $post_mount" >> $seqres.full
>>   
>>   	if [ $state -eq 1 ]; then
>> -		[ $post_mount -eq 0 ] || \
>> -			_fail "mount failed to clear under-construction merkle items pre: $pre_mount, post: $post_mount at entry $cur";
>> +		if [ $measured -ne 0 ]; then
>> +			[ $post_mount -eq 0 ] || \
> 
> OK, fix same situation you metioned.
> 
>  From the code logic, this change is good to me.
> 
> Reviewed-by: Zorro Lang <[email protected]>
> 
> I just have one more question, this case is written in 2021, now it's 2026. It
> still fails on latest test with kernel 7.1.0-0.rc2 and btrfs-progs-6.19.1-1.fc45,
> no matter with or without this patch.
> 
> Is that expected by btrfs? Is there a known kernel commit uncovered by this case?
> 

Hi Zorro,

Thank you for reviewing and accepting the patch!

Regarding the test failure on your setup - I can confirm the test passes
cleanly on my environment with both 4K and 64K block sizes:

SECTION       -- btrfs_4k
RECREATING    -- btrfs on /dev/loop0
FSTYP         -- btrfs
PLATFORM      -- Linux/ppc64le localhost 7.1.0-rc4-00100-g8bc67e4db64a 
#6 SMP PREEMPT Thu May 21 09:02:51 CEST 2026
MKFS_OPTIONS  -- -f -s 4096 /dev/loop1
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/loop1 
/mnt/scratch

btrfs/291    2s ...  2s
Ran: btrfs/291
Passed all 1 tests

My test environment:
Kernel: 7.1.0-rc4
btrfs-progs: v7.0
Architecture: powerpc/ppc64le

> Thanks,
> Zorro
> 
> # ./check btrfs/291
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 localhost 7.1.0-0.rc2.260505ga293ec25d59dd.17.fc45.x86_64 #1 SMP PREEMPT_DYNAMIC Tue May  5 17:39:29 UTC 2026
> MKFS_OPTIONS  -- /dev/vdd
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/vdd /mnt/scratch
> 
> btrfs/291          _check_btrfs_filesystem: filesystem on /dev/vdd is inconsistent
> (see /root/xfstests/results//btrfs/291.full for details)
> 
> Ran: btrfs/291
> Failures: btrfs/291
> Failed 1 of 1 tests
> 
> Thanks,
> Zorro
> 
>> +				_fail "mount failed to clear under-construction merkle items pre: $pre_mount, post: $post_mount at entry $cur";
>> +		fi
>>   	fi
>>   	if [ $state -eq 2 ]; then
>>   		[ $pre_mount -gt 0 ] || \
>> -- 
>> 2.45.1
>>

-- 
Regards,
Disha