Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper
@ 2026-05-08 14:47 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 14:31 ` [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper Zorro Lang
  0 siblings, 2 replies; 4+ messages in thread
From: Disha Goel @ 2026-05-08 14:47 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, fdmanana,
	quwenruo.btrfs, zlang, anajain.sg, Disha Goel

Add a new helper function _require_log_writes_sized() to check if the
LOGWRITES_DEV meets a minimum size requirement.

This is useful for tests that use dm-log-writes with additional space
requirements, such as creating LVM snapshots during log replay or tests
that generate large amounts of logged I/O operations.

The function takes a size parameter in KB, calls _require_log_writes()
internally, and skips the test if LOGWRITES_DEV is smaller than the
required size.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Suggested-by: Anand Jain <anajain.sg@gmail.com>
Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
v2 -> v3:
- Fixed variable declaration order - declare all local variables at the
  beginning of the function
- Use $size instead of $1 for clarity in the comparison

 common/dmlogwrites | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/common/dmlogwrites b/common/dmlogwrites
index a27e1966..b457ef7f 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -14,6 +14,22 @@ _require_log_writes()
 	_require_test_program "log-writes/replay-log"
 }
 
+# Require a log writes device of a minimum size
+# $1: minimum size in KB
+_require_log_writes_sized()
+{
+	local size=$1
+	local devsize
+
+	[ $# -eq 1 ] || _fail "_require_log_writes_sized: expected size param"
+
+	_require_log_writes
+
+	devsize=$(_get_device_size $LOGWRITES_DEV)
+	[ $devsize -lt $size ] && \
+		_notrun "LOGWRITES_DEV too small, ${devsize}KB < ${size}KB"
+}
+
 # Starting from v4.15-rc1, DAX support was added to dm-log-writes, but note
 # that it doesn't track the data that we write via the mmap(), so we can't do
 # any data integrity checking. We can only verify that the metadata writes for
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] btrfs/291: fix state transition logic and add size requirement
  2026-05-08 14:47 [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper Disha Goel
@ 2026-05-08 14:47 ` Disha Goel
  2026-05-11 15:20   ` Zorro Lang
  2026-05-11 14:31 ` [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper Zorro Lang
  1 sibling, 1 reply; 4+ messages in thread
From: Disha Goel @ 2026-05-08 14:47 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, fdmanana,
	quwenruo.btrfs, zlang, anajain.sg, Disha Goel

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
 _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
+			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 ] || \
+				_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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper
  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 14:31 ` Zorro Lang
  1 sibling, 0 replies; 4+ messages in thread
From: Zorro Lang @ 2026-05-11 14:31 UTC (permalink / raw)
  To: Disha Goel
  Cc: fstests, linux-btrfs, ritesh.list, ojaswin, djwong, fdmanana,
	quwenruo.btrfs, anajain.sg

On Fri, May 08, 2026 at 08:17:00PM +0530, Disha Goel wrote:
> Add a new helper function _require_log_writes_sized() to check if the
> LOGWRITES_DEV meets a minimum size requirement.
> 
> This is useful for tests that use dm-log-writes with additional space
> requirements, such as creating LVM snapshots during log replay or tests
> that generate large amounts of logged I/O operations.
> 
> The function takes a size parameter in KB, calls _require_log_writes()
> internally, and skips the test if LOGWRITES_DEV is smaller than the
> required size.
> 
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Suggested-by: Anand Jain <anajain.sg@gmail.com>
> Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
> ---
> v2 -> v3:
> - Fixed variable declaration order - declare all local variables at the
>   beginning of the function
> - Use $size instead of $1 for clarity in the comparison
> 
>  common/dmlogwrites | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index a27e1966..b457ef7f 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -14,6 +14,22 @@ _require_log_writes()
>  	_require_test_program "log-writes/replay-log"
>  }
>  
> +# Require a log writes device of a minimum size
> +# $1: minimum size in KB
> +_require_log_writes_sized()

Makes sense to me. Although I think we can support 'size checking' in original
_require_log_writes helper too, but I don't have objection on this change. Thanks

Reviewed-by: Zorro Lang <zlang@kernel.org>

> +{
> +	local size=$1
> +	local devsize
> +
> +	[ $# -eq 1 ] || _fail "_require_log_writes_sized: expected size param"
> +
> +	_require_log_writes
> +
> +	devsize=$(_get_device_size $LOGWRITES_DEV)
> +	[ $devsize -lt $size ] && \
> +		_notrun "LOGWRITES_DEV too small, ${devsize}KB < ${size}KB"
> +}
> +
>  # Starting from v4.15-rc1, DAX support was added to dm-log-writes, but note
>  # that it doesn't track the data that we write via the mmap(), so we can't do
>  # any data integrity checking. We can only verify that the metadata writes for
> -- 
> 2.45.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 2/2] btrfs/291: fix state transition logic and add size requirement
  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
  0 siblings, 0 replies; 4+ messages in thread
From: Zorro Lang @ 2026-05-11 15:20 UTC (permalink / raw)
  To: Disha Goel
  Cc: fstests, linux-btrfs, ritesh.list, ojaswin, djwong, fdmanana,
	quwenruo.btrfs, anajain.sg

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?

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-11 15:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-11 14:31 ` [PATCH v3 1/2] common/dmlogwrites: add _require_log_writes_sized helper Zorro Lang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox