From: Disha Goel <disgoel@linux.ibm.com>
To: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
ritesh.list@gmail.com, ojaswin@linux.ibm.com, djwong@kernel.org,
fdmanana@kernel.org, quwenruo.btrfs@gmx.com,
anajain.sg@gmail.com
Subject: Re: [PATCH v3 2/2] btrfs/291: fix state transition logic and add size requirement
Date: Thu, 21 May 2026 18:42:03 +0530 [thread overview]
Message-ID: <cf5450c4-9e5d-4d3e-b459-b2d439f4df57@linux.ibm.com> (raw)
In-Reply-To: <agHogPMQGKqAHkIT@zlang-mailbox>
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 <anajain.sg@gmail.com>
>> Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
>> ---
>> 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 <zlang@kernel.org>
>
> 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
next prev parent reply other threads:[~2026-05-21 13:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-08 14:47 [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper Disha Goel
2026-05-08 14:47 ` [PATCH v3 2/2] btrfs/291: fix state transition logic and add size requirement Disha Goel
2026-05-11 15:20 ` Zorro Lang
2026-05-21 13:12 ` Disha Goel [this message]
2026-05-11 14:31 ` [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper Zorro Lang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cf5450c4-9e5d-4d3e-b459-b2d439f4df57@linux.ibm.com \
--to=disgoel@linux.ibm.com \
--cc=anajain.sg@gmail.com \
--cc=djwong@kernel.org \
--cc=fdmanana@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=ojaswin@linux.ibm.com \
--cc=quwenruo.btrfs@gmx.com \
--cc=ritesh.list@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox