[PATCH 1/4] ext4/061,062: Minor fixes and refactoring

[PATCH 1/4] ext4/061,062: Minor fixes and refactoring

[Ojaswin Mujoo]

Fix 2 issues in the tests:

1. Use fs atomic write limits instead of bdev's

The tests use block device's limits instead of limits advertised
by filesystem. This can cause failures because block device might
advertise a higher maximum than the FS.  Fix the tests to use
filesystem's limits instead.

2. Fix the test loop range

Fix the test loop start and end so that our calculations for
blocksize, clustersize and iosize are correct.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 tests/ext4/061 | 28 ++++++++++++++++++++--------
 tests/ext4/062 | 26 +++++++++++++++++++-------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/tests/ext4/061 b/tests/ext4/061
index 1d61c8b0..69ddeaca 100755
--- a/tests/ext4/061
+++ b/tests/ext4/061
@@ -31,10 +31,19 @@ _require_aiodio
 FIO_LOAD=$(($(nproc) * 2 * LOAD_FACTOR))
 SIZE=$((100*1024*1024))
 
-# Calculate fsblocksize as per bdev atomic write units.
-bdev_awu_min=$(_get_atomic_write_unit_min $SCRATCH_DEV)
-bdev_awu_max=$(_get_atomic_write_unit_max $SCRATCH_DEV)
-bs=$(_max 4096 "$bdev_awu_min")
+_scratch_mkfs > /dev/null 2>&1 || \
+	_notrun "mkfs failed"
+_try_scratch_mount || \
+	_notrun "mount failed"
+
+touch $SCRATCH_MNT/f
+
+fs_awu_min=$(_get_atomic_write_unit_min $SCRATCH_MNT/f)
+fs_awu_max=$(_get_atomic_write_unit_max $SCRATCH_MNT/f)
+bs=$(_max 4096 "$fs_awu_min")
+ps=$(_get_page_size)
+
+_scratch_unmount
 
 function create_fio_configs()
 {
@@ -97,10 +106,11 @@ run_test_one() {
 	local cs=$2
 	local iosize=$3
 
+	echo "== Testing: bs=$bs cs=$cs iosize=$iosize ==" >> $seqres.full
+
 	MKFS_OPTIONS="-O bigalloc -b $bs -C $cs"
 	_scratch_mkfs_ext4  >> $seqres.full 2>&1 || return
 	if _try_scratch_mount >> $seqres.full 2>&1; then
-		echo "== Testing: bs=$bs cs=$cs iosize=$iosize ==" >> $seqres.full
 
 		touch $SCRATCH_MNT/f1
 		create_fio_configs $iosize
@@ -127,7 +137,7 @@ run_test() {
 	# cluster sizes above 16 x blocksize are experimental so avoid them
 	# Also, cap cluster size at 128kb to keep it reasonable for large
 	# blocks size
-	max_cs=$(_min $((16 * bs)) "$bdev_awu_max" $((128 * 1024)))
+	max_cs=$(_min $((16 * bs)) "$fs_awu_max" $((128 * 1024)))
 
 	# Fuzz for combinations of blocksize, clustersize and
 	# iosize that cover most of the cases
@@ -145,8 +155,10 @@ fio_out=$tmp.fio.out
 create_fio_configs $bs
 _require_fio $fio_aw_config
 
-for ((bs=$bs; bs <= $(_get_page_size); bs = $bs << 1)); do
-	run_test $bs
+echo "Awu min: $fs_awu_min Awu max: $fs_awu_max" >> $seqres.full
+
+for ((bs=$fs_awu_min; bs <= $(_min ps fs_awu_max); bs = $bs << 1)); do
+	run_test $bs $cs $iosize
 done
 
 # success, all done
diff --git a/tests/ext4/062 b/tests/ext4/062
index 05cce696..56bf080c 100755
--- a/tests/ext4/062
+++ b/tests/ext4/062
@@ -33,10 +33,19 @@ _require_aiodio
 FSSIZE=$((360*1024*1024))
 FIO_LOAD=$(($(nproc) * LOAD_FACTOR))
 
-# Calculate bs as per bdev atomic write units.
-bdev_awu_min=$(_get_atomic_write_unit_min $SCRATCH_DEV)
-bdev_awu_max=$(_get_atomic_write_unit_max $SCRATCH_DEV)
-bs=$(_max 4096 "$bdev_awu_min")
+_scratch_mkfs > /dev/null 2>&1 || \
+	_notrun "mkfs failed"
+_try_scratch_mount || \
+	_notrun "mount failed"
+
+touch $SCRATCH_MNT/f
+
+fs_awu_min=$(_get_atomic_write_unit_min $SCRATCH_MNT/f)
+fs_awu_max=$(_get_atomic_write_unit_max $SCRATCH_MNT/f)
+bs=$(_max 4096 "$fs_awu_min")
+ps=$(_get_page_size)
+
+_scratch_unmount
 
 function create_fio_configs()
 {
@@ -146,10 +155,11 @@ run_test_one() {
 	local cs=$2
 	local iosize=$3
 
+	echo "Testing: bs=$bs cs=$cs iosize=$iosize" >> $seqres.full
+
 	MKFS_OPTIONS="-O bigalloc -b $bs -C $cs"
 	_scratch_mkfs_sized "$FSSIZE" >> $seqres.full 2>&1 || return
 	if _try_scratch_mount >> $seqres.full 2>&1; then
-		echo "Testing: bs=$bs cs=$cs iosize=$iosize" >> $seqres.full
 
 		touch $SCRATCH_MNT/f1
 		create_fio_configs $iosize
@@ -175,7 +185,7 @@ run_test() {
 	# cluster sizes above 16 x blocksize are experimental so avoid them
 	# Also, cap cluster size at 128kb to keep it reasonable for large
 	# blocks size
-	max_cs=$(_min $((16 * bs)) "$bdev_awu_max" $((128 * 1024)))
+	max_cs=$(_min $((16 * bs)) "$fs_awu_max" $((128 * 1024)))
 
 	# Fuzz for combinations of blocksize, clustersize and
 	# iosize that cover most of the cases
@@ -193,7 +203,9 @@ fio_out=$tmp.fio.out
 create_fio_configs $bs
 _require_fio $fio_aw_config
 
-for ((bs=$bs; bs <= $(_get_page_size); bs = $bs << 1)); do
+echo "Awu min: $fs_awu_min Awu max: $fs_awu_max" >> $seqres.full
+
+for ((bs=$fs_awu_min; bs <= $(_min ps fs_awu_max); bs = $bs << 1)); do
 	run_test $bs $cs $iosize
 done
 
-- 
2.53.0

[PATCH 2/4] generic/765: Fix sysfs path for nvme partitions

[Ojaswin Mujoo]

This tests checks atomic write limits reported by statx() are same as
the ones reported by sysfs, however nvme partitions don't have the
/sys/block/nvme0n1p1 style entry and also use the namespace's queue
limits for atomic writes. This causes the test to fail because it
expects /sys/block/nvme0n*p* to be present.

Hence, in this case, just check against the parent namespace.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 tests/generic/765 | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/generic/765 b/tests/generic/765
index 8c4e0bd0..4c768783 100755
--- a/tests/generic/765
+++ b/tests/generic/765
@@ -94,16 +94,31 @@ test_atomic_writes()
     _scratch_unmount
 }
 
-sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
-sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
+# Get the right sysfs dev for the atomic write device
+get_aw_dev() {
+    local dev
+    dev=$(_short_dev "$1")
+
+    # Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
+    # entry and also use the namespace's queue limits for atomic writes, hence
+    # just return the parent namespace.
+    if [[ $dev == nvme* ]]; then
+        echo "${dev%%p[0-9]*}"
+    else
+        echo "$dev"
+    fi
+}
+
+sys_min_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
+sys_max_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
 
 bdev_min_write=$(_get_atomic_write_unit_min $SCRATCH_DEV)
 bdev_max_write=$(_get_atomic_write_unit_max $SCRATCH_DEV)
 
 echo "sysfs awu_min $sys_min_write" >> $seqres.full
-echo "sysfs awu_min $sys_max_write" >> $seqres.full
+echo "sysfs awu_max $sys_max_write" >> $seqres.full
 echo "bdev awu_min $bdev_min_write" >> $seqres.full
-echo "bdev awu_min $bdev_max_write" >> $seqres.full
+echo "bdev awu_max $bdev_max_write" >> $seqres.full
 
 # Test that statx atomic values are the same as sysfs values
 if [ "$sys_min_write" -ne "$bdev_min_write" ]; then
-- 
2.53.0

Re: [PATCH 2/4] generic/765: Fix sysfs path for nvme partitions

[Darrick J. Wong]

On Wed, Apr 01, 2026 at 04:10:48PM +0530, Ojaswin Mujoo wrote:
> This tests checks atomic write limits reported by statx() are same as
> the ones reported by sysfs, however nvme partitions don't have the
> /sys/block/nvme0n1p1 style entry and also use the namespace's queue
> limits for atomic writes. This causes the test to fail because it
> expects /sys/block/nvme0n*p* to be present.
> 
> Hence, in this case, just check against the parent namespace.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
>  tests/generic/765 | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/generic/765 b/tests/generic/765
> index 8c4e0bd0..4c768783 100755
> --- a/tests/generic/765
> +++ b/tests/generic/765
> @@ -94,16 +94,31 @@ test_atomic_writes()
>      _scratch_unmount
>  }
>  
> -sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> -sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")

I see this show up in a few other places:

$ git grep /sys/block
common/dmlogwrites:64:  local sysfs="/sys/block/$(_short_dev $1)"
common/rc:2567: local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
common/rc:2631: if [ -e /sys/block/${sdev}/queue/zoned ]; then
common/rc:2632:         cat /sys/block/${sdev}/queue/zoned
common/scsi_debug:67:   device=`grep -wl scsi_debug /sys/block/sd*/device/model | awk -F / '{print $4}'`
tests/btrfs/076:31:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
tests/btrfs/237:48:zone_size=$(($(cat /sys/block/${sdev}/queue/chunk_sectors) << 9))
tests/btrfs/283:29:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
vests/generic/108:20:   echo running > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
tests/generic/108:81:echo offline > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
tests/generic/730:56:echo 1 > /sys/block/$(_short_dev $SCSI_DEBUG_DEV)/device/delete
tests/generic/731:49:echo 1 > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/delete
tests/generic/765:97:sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
tests/generic/765:98:sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")

So I wonder if the same problem exists there as well?

(Well, maybe not the scsi-debug users)

> +# Get the right sysfs dev for the atomic write device
> +get_aw_dev() {
> +    local dev
> +    dev=$(_short_dev "$1")
> +
> +    # Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> +    # entry and also use the namespace's queue limits for atomic writes, hence
> +    # just return the parent namespace.
> +    if [[ $dev == nvme* ]]; then
> +        echo "${dev%%p[0-9]*}"
> +    else
> +        echo "$dev"
> +    fi
> +}

Perhaps this ought to turn into a common helper to fix them all?

_sys_block_path() {
	local dev=$(_short_dev "$1")

	# Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
	# entry and also use the namespace's queue limits for atomic writes, hence
	# just return the parent namespace.
	if [[ $dev == nvme* ]]; then
		echo "/sys/block/${dev%%p[0-9]*}"
	else
		echo "/sys/block/$dev"
	fi
}

sys_min_write=$(_sys_block_path $SCRATCH_DEV)/queue/<whatever>)

Also, don't some of the other block devices follow this "partition has
pXXX suffix" pattern as well?  Not that I have a clue why scsi doesn't
follow that pattern or even which part of sd.c triggers the sdaX
behavior. :/

--D

> +
> +sys_min_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> +sys_max_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
>  
>  bdev_min_write=$(_get_atomic_write_unit_min $SCRATCH_DEV)
>  bdev_max_write=$(_get_atomic_write_unit_max $SCRATCH_DEV)
>  
>  echo "sysfs awu_min $sys_min_write" >> $seqres.full
> -echo "sysfs awu_min $sys_max_write" >> $seqres.full
> +echo "sysfs awu_max $sys_max_write" >> $seqres.full
>  echo "bdev awu_min $bdev_min_write" >> $seqres.full
> -echo "bdev awu_min $bdev_max_write" >> $seqres.full
> +echo "bdev awu_max $bdev_max_write" >> $seqres.full
>  
>  # Test that statx atomic values are the same as sysfs values
>  if [ "$sys_min_write" -ne "$bdev_min_write" ]; then
> -- 
> 2.53.0
> 
> 

Re: [PATCH 2/4] generic/765: Fix sysfs path for nvme partitions

[Ojaswin Mujoo]

On Wed, Apr 01, 2026 at 07:08:29AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 01, 2026 at 04:10:48PM +0530, Ojaswin Mujoo wrote:
> > This tests checks atomic write limits reported by statx() are same as
> > the ones reported by sysfs, however nvme partitions don't have the
> > /sys/block/nvme0n1p1 style entry and also use the namespace's queue
> > limits for atomic writes. This causes the test to fail because it
> > expects /sys/block/nvme0n*p* to be present.
> > 
> > Hence, in this case, just check against the parent namespace.
> > 
> > Reported-by: Disha Goel <disgoel@linux.ibm.com>
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> >  tests/generic/765 | 23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/generic/765 b/tests/generic/765
> > index 8c4e0bd0..4c768783 100755
> > --- a/tests/generic/765
> > +++ b/tests/generic/765
> > @@ -94,16 +94,31 @@ test_atomic_writes()
> >      _scratch_unmount
> >  }
> >  
> > -sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > -sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> 
> I see this show up in a few other places:
> 
> $ git grep /sys/block
> common/dmlogwrites:64:  local sysfs="/sys/block/$(_short_dev $1)"
> common/rc:2567: local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
> common/rc:2631: if [ -e /sys/block/${sdev}/queue/zoned ]; then
> common/rc:2632:         cat /sys/block/${sdev}/queue/zoned
> common/scsi_debug:67:   device=`grep -wl scsi_debug /sys/block/sd*/device/model | awk -F / '{print $4}'`
> tests/btrfs/076:31:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> tests/btrfs/237:48:zone_size=$(($(cat /sys/block/${sdev}/queue/chunk_sectors) << 9))
> tests/btrfs/283:29:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> vests/generic/108:20:   echo running > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
> tests/generic/108:81:echo offline > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
> tests/generic/730:56:echo 1 > /sys/block/$(_short_dev $SCSI_DEBUG_DEV)/device/delete
> tests/generic/731:49:echo 1 > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/delete
> tests/generic/765:97:sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> tests/generic/765:98:sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> 
> So I wonder if the same problem exists there as well?

Okay so seems like there general rule in the kernel is:

File: block/partitions/core.c:336-339

dname = dev_name(ddev);
if (isdigit(dname[strlen(dname) - 1]))
    dev_set_name(pdev, "%sp%d", dname, partno);
else
    dev_set_name(pdev, "%s%d", dname, partno);

So any device where names end with numbers follow the name<%d>p<%d>
scheme for partitions. 

I was not completely sure if this is something only nvmes follow but
looking into the code a bit (from my limited understanding of the blkdev
code) seems like all  partiions just inherit parent's queues and sysfs
"queue" is only for the parent (struct gendisk) rather than partiions.

So I think a more generic way would be to check if dev is a partitions
and then get the correct path for it:

dev_is_partition() {
    local dev
    dev=$(_short_dev "$1")

    [[ -e /sys/class/block/$dev/partition ]]
}

dev_get_queue_path() {
    local dev parent
    dev=$(_short_dev "$1")

    if dev_is_partition "$dev"; then
        parent=$(basename "$(readlink -f /sys/class/block/$dev/..)")
    else
        parent="$dev"
    fi

    echo "/sys/block/$parent/queue"
}

Does that look okay?

Regards,
ojaswin

> 
> (Well, maybe not the scsi-debug users)
> 
> > +# Get the right sysfs dev for the atomic write device
> > +get_aw_dev() {
> > +    local dev
> > +    dev=$(_short_dev "$1")
> > +
> > +    # Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> > +    # entry and also use the namespace's queue limits for atomic writes, hence
> > +    # just return the parent namespace.
> > +    if [[ $dev == nvme* ]]; then
> > +        echo "${dev%%p[0-9]*}"
> > +    else
> > +        echo "$dev"
> > +    fi
> > +}
> 
> Perhaps this ought to turn into a common helper to fix them all?
> 
> _sys_block_path() {
> 	local dev=$(_short_dev "$1")
> 
> 	# Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> 	# entry and also use the namespace's queue limits for atomic writes, hence
> 	# just return the parent namespace.
> 	if [[ $dev == nvme* ]]; then
> 		echo "/sys/block/${dev%%p[0-9]*}"
> 	else
> 		echo "/sys/block/$dev"
> 	fi
> }
> 
> sys_min_write=$(_sys_block_path $SCRATCH_DEV)/queue/<whatever>)
> 
> Also, don't some of the other block devices follow this "partition has
> pXXX suffix" pattern as well?  Not that I have a clue why scsi doesn't
> follow that pattern or even which part of sd.c triggers the sdaX
> behavior. :/
> 
> --D
> 
> > +
> > +sys_min_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > +sys_max_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> >  
> >  bdev_min_write=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> >  bdev_max_write=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> >  
> >  echo "sysfs awu_min $sys_min_write" >> $seqres.full
> > -echo "sysfs awu_min $sys_max_write" >> $seqres.full
> > +echo "sysfs awu_max $sys_max_write" >> $seqres.full
> >  echo "bdev awu_min $bdev_min_write" >> $seqres.full
> > -echo "bdev awu_min $bdev_max_write" >> $seqres.full
> > +echo "bdev awu_max $bdev_max_write" >> $seqres.full
> >  
> >  # Test that statx atomic values are the same as sysfs values
> >  if [ "$sys_min_write" -ne "$bdev_min_write" ]; then
> > -- 
> > 2.53.0
> > 
> > 

Re: [PATCH 2/4] generic/765: Fix sysfs path for nvme partitions

[Darrick J. Wong]

On Sun, Apr 05, 2026 at 08:37:12PM +0530, Ojaswin Mujoo wrote:
> On Wed, Apr 01, 2026 at 07:08:29AM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 01, 2026 at 04:10:48PM +0530, Ojaswin Mujoo wrote:
> > > This tests checks atomic write limits reported by statx() are same as
> > > the ones reported by sysfs, however nvme partitions don't have the
> > > /sys/block/nvme0n1p1 style entry and also use the namespace's queue
> > > limits for atomic writes. This causes the test to fail because it
> > > expects /sys/block/nvme0n*p* to be present.
> > > 
> > > Hence, in this case, just check against the parent namespace.
> > > 
> > > Reported-by: Disha Goel <disgoel@linux.ibm.com>
> > > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > ---
> > >  tests/generic/765 | 23 +++++++++++++++++++----
> > >  1 file changed, 19 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tests/generic/765 b/tests/generic/765
> > > index 8c4e0bd0..4c768783 100755
> > > --- a/tests/generic/765
> > > +++ b/tests/generic/765
> > > @@ -94,16 +94,31 @@ test_atomic_writes()
> > >      _scratch_unmount
> > >  }
> > >  
> > > -sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > > -sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> > 
> > I see this show up in a few other places:
> > 
> > $ git grep /sys/block
> > common/dmlogwrites:64:  local sysfs="/sys/block/$(_short_dev $1)"
> > common/rc:2567: local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
> > common/rc:2631: if [ -e /sys/block/${sdev}/queue/zoned ]; then
> > common/rc:2632:         cat /sys/block/${sdev}/queue/zoned
> > common/scsi_debug:67:   device=`grep -wl scsi_debug /sys/block/sd*/device/model | awk -F / '{print $4}'`
> > tests/btrfs/076:31:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> > tests/btrfs/237:48:zone_size=$(($(cat /sys/block/${sdev}/queue/chunk_sectors) << 9))
> > tests/btrfs/283:29:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> > vests/generic/108:20:   echo running > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
> > tests/generic/108:81:echo offline > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
> > tests/generic/730:56:echo 1 > /sys/block/$(_short_dev $SCSI_DEBUG_DEV)/device/delete
> > tests/generic/731:49:echo 1 > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/delete
> > tests/generic/765:97:sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > tests/generic/765:98:sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> > 
> > So I wonder if the same problem exists there as well?
> 
> Okay so seems like there general rule in the kernel is:
> 
> File: block/partitions/core.c:336-339
> 
> dname = dev_name(ddev);
> if (isdigit(dname[strlen(dname) - 1]))
>     dev_set_name(pdev, "%sp%d", dname, partno);
> else
>     dev_set_name(pdev, "%s%d", dname, partno);
> 
> So any device where names end with numbers follow the name<%d>p<%d>
> scheme for partitions.

Eeeeeee thanks for digging that out :)

> I was not completely sure if this is something only nvmes follow but
> looking into the code a bit (from my limited understanding of the blkdev
> code) seems like all  partiions just inherit parent's queues and sysfs
> "queue" is only for the parent (struct gendisk) rather than partiions.

Yes, partitions just reuse the parent bdev's queues.

> So I think a more generic way would be to check if dev is a partitions
> and then get the correct path for it:
> 
> dev_is_partition() {
>     local dev
>     dev=$(_short_dev "$1")
> 
>     [[ -e /sys/class/block/$dev/partition ]]
> }
> 
> dev_get_queue_path() {
>     local dev parent
>     dev=$(_short_dev "$1")
> 
>     if dev_is_partition "$dev"; then
>         parent=$(basename "$(readlink -f /sys/class/block/$dev/..)")

Heh, that '..' is delectably subtle.

>     else
>         parent="$dev"
>     fi
> 
>     echo "/sys/block/$parent/queue"

Might want to check that the path actually exists before printing it,
but yes that looks ok.

--D

> }
> 
> Does that look okay?
> 
> Regards,
> ojaswin
> 
> > 
> > (Well, maybe not the scsi-debug users)
> > 
> > > +# Get the right sysfs dev for the atomic write device
> > > +get_aw_dev() {
> > > +    local dev
> > > +    dev=$(_short_dev "$1")
> > > +
> > > +    # Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> > > +    # entry and also use the namespace's queue limits for atomic writes, hence
> > > +    # just return the parent namespace.
> > > +    if [[ $dev == nvme* ]]; then
> > > +        echo "${dev%%p[0-9]*}"
> > > +    else
> > > +        echo "$dev"
> > > +    fi
> > > +}
> > 
> > Perhaps this ought to turn into a common helper to fix them all?
> > 
> > _sys_block_path() {
> > 	local dev=$(_short_dev "$1")
> > 
> > 	# Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> > 	# entry and also use the namespace's queue limits for atomic writes, hence
> > 	# just return the parent namespace.
> > 	if [[ $dev == nvme* ]]; then
> > 		echo "/sys/block/${dev%%p[0-9]*}"
> > 	else
> > 		echo "/sys/block/$dev"
> > 	fi
> > }
> > 
> > sys_min_write=$(_sys_block_path $SCRATCH_DEV)/queue/<whatever>)
> > 
> > Also, don't some of the other block devices follow this "partition has
> > pXXX suffix" pattern as well?  Not that I have a clue why scsi doesn't
> > follow that pattern or even which part of sd.c triggers the sdaX
> > behavior. :/
> > 
> > --D
> > 
> > > +
> > > +sys_min_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > > +sys_max_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> > >  
> > >  bdev_min_write=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> > >  bdev_max_write=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> > >  
> > >  echo "sysfs awu_min $sys_min_write" >> $seqres.full
> > > -echo "sysfs awu_min $sys_max_write" >> $seqres.full
> > > +echo "sysfs awu_max $sys_max_write" >> $seqres.full
> > >  echo "bdev awu_min $bdev_min_write" >> $seqres.full
> > > -echo "bdev awu_min $bdev_max_write" >> $seqres.full
> > > +echo "bdev awu_max $bdev_max_write" >> $seqres.full
> > >  
> > >  # Test that statx atomic values are the same as sysfs values
> > >  if [ "$sys_min_write" -ne "$bdev_min_write" ]; then
> > > -- 
> > > 2.53.0
> > > 
> > > 
> 

Re: [PATCH 2/4] generic/765: Fix sysfs path for nvme partitions

[Ojaswin Mujoo]

On Mon, Apr 06, 2026 at 08:43:58AM -0700, Darrick J. Wong wrote:
> On Sun, Apr 05, 2026 at 08:37:12PM +0530, Ojaswin Mujoo wrote:
> > On Wed, Apr 01, 2026 at 07:08:29AM -0700, Darrick J. Wong wrote:
> > > On Wed, Apr 01, 2026 at 04:10:48PM +0530, Ojaswin Mujoo wrote:
> > > > This tests checks atomic write limits reported by statx() are same as
> > > > the ones reported by sysfs, however nvme partitions don't have the
> > > > /sys/block/nvme0n1p1 style entry and also use the namespace's queue
> > > > limits for atomic writes. This causes the test to fail because it
> > > > expects /sys/block/nvme0n*p* to be present.
> > > > 
> > > > Hence, in this case, just check against the parent namespace.
> > > > 
> > > > Reported-by: Disha Goel <disgoel@linux.ibm.com>
> > > > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > > ---
> > > >  tests/generic/765 | 23 +++++++++++++++++++----
> > > >  1 file changed, 19 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/tests/generic/765 b/tests/generic/765
> > > > index 8c4e0bd0..4c768783 100755
> > > > --- a/tests/generic/765
> > > > +++ b/tests/generic/765
> > > > @@ -94,16 +94,31 @@ test_atomic_writes()
> > > >      _scratch_unmount
> > > >  }
> > > >  
> > > > -sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > > > -sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> > > 
> > > I see this show up in a few other places:
> > > 
> > > $ git grep /sys/block
> > > common/dmlogwrites:64:  local sysfs="/sys/block/$(_short_dev $1)"
> > > common/rc:2567: local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
> > > common/rc:2631: if [ -e /sys/block/${sdev}/queue/zoned ]; then
> > > common/rc:2632:         cat /sys/block/${sdev}/queue/zoned
> > > common/scsi_debug:67:   device=`grep -wl scsi_debug /sys/block/sd*/device/model | awk -F / '{print $4}'`
> > > tests/btrfs/076:31:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> > > tests/btrfs/237:48:zone_size=$(($(cat /sys/block/${sdev}/queue/chunk_sectors) << 9))
> > > tests/btrfs/283:29:     zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> > > vests/generic/108:20:   echo running > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
> > > tests/generic/108:81:echo offline > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/state
> > > tests/generic/730:56:echo 1 > /sys/block/$(_short_dev $SCSI_DEBUG_DEV)/device/delete
> > > tests/generic/731:49:echo 1 > /sys/block/`_short_dev $SCSI_DEBUG_DEV`/device/delete
> > > tests/generic/765:97:sys_min_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > > tests/generic/765:98:sys_max_write=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> > > 
> > > So I wonder if the same problem exists there as well?
> > 
> > Okay so seems like there general rule in the kernel is:
> > 
> > File: block/partitions/core.c:336-339
> > 
> > dname = dev_name(ddev);
> > if (isdigit(dname[strlen(dname) - 1]))
> >     dev_set_name(pdev, "%sp%d", dname, partno);
> > else
> >     dev_set_name(pdev, "%s%d", dname, partno);
> > 
> > So any device where names end with numbers follow the name<%d>p<%d>
> > scheme for partitions.
> 
> Eeeeeee thanks for digging that out :)
> 
> > I was not completely sure if this is something only nvmes follow but
> > looking into the code a bit (from my limited understanding of the blkdev
> > code) seems like all  partiions just inherit parent's queues and sysfs
> > "queue" is only for the parent (struct gendisk) rather than partiions.
> 
> Yes, partitions just reuse the parent bdev's queues.
> 
> > So I think a more generic way would be to check if dev is a partitions
> > and then get the correct path for it:
> > 
> > dev_is_partition() {
> >     local dev
> >     dev=$(_short_dev "$1")
> > 
> >     [[ -e /sys/class/block/$dev/partition ]]
> > }
> > 
> > dev_get_queue_path() {
> >     local dev parent
> >     dev=$(_short_dev "$1")
> > 
> >     if dev_is_partition "$dev"; then
> >         parent=$(basename "$(readlink -f /sys/class/block/$dev/..)")
> 
> Heh, that '..' is delectably subtle.

Yes that's true. I think I'll just break it into 2 steps so its more
clear to read.

> 
> >     else
> >         parent="$dev"
> >     fi
> > 
> >     echo "/sys/block/$parent/queue"
> 
> Might want to check that the path actually exists before printing it,
> but yes that looks ok.

Makes sense.

Thanks for the review!
ojaswin
> 
> --D
> 
> > }
> > 
> > Does that look okay?
> > 
> > Regards,
> > ojaswin
> > 
> > > 
> > > (Well, maybe not the scsi-debug users)
> > > 
> > > > +# Get the right sysfs dev for the atomic write device
> > > > +get_aw_dev() {
> > > > +    local dev
> > > > +    dev=$(_short_dev "$1")
> > > > +
> > > > +    # Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> > > > +    # entry and also use the namespace's queue limits for atomic writes, hence
> > > > +    # just return the parent namespace.
> > > > +    if [[ $dev == nvme* ]]; then
> > > > +        echo "${dev%%p[0-9]*}"
> > > > +    else
> > > > +        echo "$dev"
> > > > +    fi
> > > > +}
> > > 
> > > Perhaps this ought to turn into a common helper to fix them all?
> > > 
> > > _sys_block_path() {
> > > 	local dev=$(_short_dev "$1")
> > > 
> > > 	# Special case: nvme partitions don't have the /sys/block/nvme0n1p1 style
> > > 	# entry and also use the namespace's queue limits for atomic writes, hence
> > > 	# just return the parent namespace.
> > > 	if [[ $dev == nvme* ]]; then
> > > 		echo "/sys/block/${dev%%p[0-9]*}"
> > > 	else
> > > 		echo "/sys/block/$dev"
> > > 	fi
> > > }
> > > 
> > > sys_min_write=$(_sys_block_path $SCRATCH_DEV)/queue/<whatever>)
> > > 
> > > Also, don't some of the other block devices follow this "partition has
> > > pXXX suffix" pattern as well?  Not that I have a clue why scsi doesn't
> > > follow that pattern or even which part of sd.c triggers the sdaX
> > > behavior. :/
> > > 
> > > --D
> > > 
> > > > +
> > > > +sys_min_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_min_bytes")
> > > > +sys_max_write=$(cat "/sys/block/$(get_aw_dev $SCRATCH_DEV)/queue/atomic_write_unit_max_bytes")
> > > >  
> > > >  bdev_min_write=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> > > >  bdev_max_write=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> > > >  
> > > >  echo "sysfs awu_min $sys_min_write" >> $seqres.full
> > > > -echo "sysfs awu_min $sys_max_write" >> $seqres.full
> > > > +echo "sysfs awu_max $sys_max_write" >> $seqres.full
> > > >  echo "bdev awu_min $bdev_min_write" >> $seqres.full
> > > > -echo "bdev awu_min $bdev_max_write" >> $seqres.full
> > > > +echo "bdev awu_max $bdev_max_write" >> $seqres.full
> > > >  
> > > >  # Test that statx atomic values are the same as sysfs values
> > > >  if [ "$sys_min_write" -ne "$bdev_min_write" ]; then
> > > > -- 
> > > > 2.53.0
> > > > 
> > > > 
> > 

[PATCH 3/4] generic/765: Ignore mkfs warning

[Ojaswin Mujoo]

From: Super User <root@ltcrain119-lp6.ltc.tadn.ibm.com>

This test validates atomic writes for all possible block sizes. In ext4, for
smaller block sizes with configurations like:

export MKFS_OPTIONS="-O bigalloc,quota -b 65536 -C 131072"

The output can get corrupted with warnings like below because clustersize
more than 16xbs is experimental:

+ 16 times the block size is considered experimental

Hence pipe these to seqres.full to avoid false negatives.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 tests/generic/765 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/765 b/tests/generic/765
index 4c768783..6794eb72 100755
--- a/tests/generic/765
+++ b/tests/generic/765
@@ -64,7 +64,7 @@ test_atomic_writes()
     local bsize=$1
 
     get_mkfs_opts $bsize
-    _scratch_mkfs $mkfs_opts >> $seqres.full
+    _scratch_mkfs $mkfs_opts >> $seqres.full 2>&1
     _scratch_mount
 
     test "$FSTYP" = "xfs" && _xfs_force_bdev data $SCRATCH_MNT
-- 
2.53.0

Re: [PATCH 3/4] generic/765: Ignore mkfs warning

[Darrick J. Wong]

On Wed, Apr 01, 2026 at 04:10:49PM +0530, Ojaswin Mujoo wrote:
> From: Super User <root@ltcrain119-lp6.ltc.tadn.ibm.com>

Who?

> This test validates atomic writes for all possible block sizes. In ext4, for
> smaller block sizes with configurations like:
> 
> export MKFS_OPTIONS="-O bigalloc,quota -b 65536 -C 131072"
> 
> The output can get corrupted with warnings like below because clustersize
> more than 16xbs is experimental:
> 
> + 16 times the block size is considered experimental

Er... a message that doesn't fail the mkfs is printed to stderr?
Maybe mke2fs should fix that...

> Hence pipe these to seqres.full to avoid false negatives.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
>  tests/generic/765 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/765 b/tests/generic/765
> index 4c768783..6794eb72 100755
> --- a/tests/generic/765
> +++ b/tests/generic/765
> @@ -64,7 +64,7 @@ test_atomic_writes()
>      local bsize=$1
>  
>      get_mkfs_opts $bsize
> -    _scratch_mkfs $mkfs_opts >> $seqres.full
> +    _scratch_mkfs $mkfs_opts >> $seqres.full 2>&1

I think we still want to check that the format succeeded as a
precondition for the test round, right?

	_scratch_mkfs $mkfs_opts &>> $seqres.full || \
		echo "mkfs $mkfs_opts" failed"

--D

>      _scratch_mount
>  
>      test "$FSTYP" = "xfs" && _xfs_force_bdev data $SCRATCH_MNT
> -- 
> 2.53.0
> 
> 

Re: [PATCH 3/4] generic/765: Ignore mkfs warning

[Ojaswin Mujoo]

On Wed, Apr 01, 2026 at 07:30:18AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 01, 2026 at 04:10:49PM +0530, Ojaswin Mujoo wrote:
> > From: Super User <root@ltcrain119-lp6.ltc.tadn.ibm.com>
> 
> Who?

Ahh dang I'll fix this sorry about that.
> 
> > This test validates atomic writes for all possible block sizes. In ext4, for
> > smaller block sizes with configurations like:
> > 
> > export MKFS_OPTIONS="-O bigalloc,quota -b 65536 -C 131072"
> > 
> > The output can get corrupted with warnings like below because clustersize
> > more than 16xbs is experimental:
> > 
> > + 16 times the block size is considered experimental
> 
> Er... a message that doesn't fail the mkfs is printed to stderr?
> Maybe mke2fs should fix that...

Hmm yes makes sense, I'll try to make that change in e2fsprogs.
Hopefully noone is depending on such a message showing up in stderr.

> 
> > Hence pipe these to seqres.full to avoid false negatives.
> > 
> > Reported-by: Disha Goel <disgoel@linux.ibm.com>
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> >  tests/generic/765 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/generic/765 b/tests/generic/765
> > index 4c768783..6794eb72 100755
> > --- a/tests/generic/765
> > +++ b/tests/generic/765
> > @@ -64,7 +64,7 @@ test_atomic_writes()
> >      local bsize=$1
> >  
> >      get_mkfs_opts $bsize
> > -    _scratch_mkfs $mkfs_opts >> $seqres.full
> > +    _scratch_mkfs $mkfs_opts >> $seqres.full 2>&1
> 
> I think we still want to check that the format succeeded as a
> precondition for the test round, right?
> 
> 	_scratch_mkfs $mkfs_opts &>> $seqres.full || \
> 		echo "mkfs $mkfs_opts" failed"
> 
> --D

Yes makes sense, I'll do that. 

Thanks for the review,
Ojaswin.
> 
> >      _scratch_mount
> >  
> >      test "$FSTYP" = "xfs" && _xfs_force_bdev data $SCRATCH_MNT
> > -- 
> > 2.53.0
> > 
> > 

[PATCH 4/4] generic/775: Fix an infinite loop due to variable name clash

[Ojaswin Mujoo]

We use i as the iteration variable in the main test loop as well as some
internal loops. Due to this clash, $i variable of main test loops was
getting modified by the following loop in prep_mixed_mapping().

	for ((i=0; i<num_blocks; i++)); do

If num_blocks is less than 10 (example ext4 with blocksize 4k and
cluster size 8k) i would always be set as 3 and the main loop would
never exit. Take the simplest approach of just renaming the variable.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 tests/generic/775 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/775 b/tests/generic/775
index 2a4287bb..19ae95e2 100755
--- a/tests/generic/775
+++ b/tests/generic/775
@@ -41,7 +41,7 @@ prep_mixed_mapping() {
 
 	local operations=("W" "H" "U")
 	local num_blocks=$((awu_max / blksz))
-	for ((i=0; i<num_blocks; i++)); do
+	for ((j=0; j<num_blocks; j++)); do
 		local index=$((RANDOM % ${#operations[@]}))
 		local map="${operations[$index]}"
 		local mapping="${mapping}${map}"
-- 
2.53.0

Re: [PATCH 4/4] generic/775: Fix an infinite loop due to variable name clash

[Darrick J. Wong]

On Wed, Apr 01, 2026 at 04:10:50PM +0530, Ojaswin Mujoo wrote:
> We use i as the iteration variable in the main test loop as well as some
> internal loops. Due to this clash, $i variable of main test loops was
> getting modified by the following loop in prep_mixed_mapping().
> 
> 	for ((i=0; i<num_blocks; i++)); do
> 
> If num_blocks is less than 10 (example ext4 with blocksize 4k and
> cluster size 8k) i would always be set as 3 and the main loop would
> never exit. Take the simplest approach of just renaming the variable.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
>  tests/generic/775 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/775 b/tests/generic/775
> index 2a4287bb..19ae95e2 100755
> --- a/tests/generic/775
> +++ b/tests/generic/775
> @@ -41,7 +41,7 @@ prep_mixed_mapping() {
>  
>  	local operations=("W" "H" "U")
>  	local num_blocks=$((awu_max / blksz))
> -	for ((i=0; i<num_blocks; i++)); do
> +	for ((j=0; j<num_blocks; j++)); do

You're right that the single-letter variables are a really bad idea in
languages like bash/python/etc where functions can see variables in
caller's scope if they're not explicitly marked local.

But a) why not use "local i" to prevent that; and (b) if you're going
to rename it, why not "blkno"?

--D

>  		local index=$((RANDOM % ${#operations[@]}))
>  		local map="${operations[$index]}"
>  		local mapping="${mapping}${map}"
> -- 
> 2.53.0
> 
> 

Re: [PATCH 4/4] generic/775: Fix an infinite loop due to variable name clash

[Ojaswin Mujoo]

On Wed, Apr 01, 2026 at 07:32:18AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 01, 2026 at 04:10:50PM +0530, Ojaswin Mujoo wrote:
> > We use i as the iteration variable in the main test loop as well as some
> > internal loops. Due to this clash, $i variable of main test loops was
> > getting modified by the following loop in prep_mixed_mapping().
> > 
> > 	for ((i=0; i<num_blocks; i++)); do
> > 
> > If num_blocks is less than 10 (example ext4 with blocksize 4k and
> > cluster size 8k) i would always be set as 3 and the main loop would
> > never exit. Take the simplest approach of just renaming the variable.
> > 
> > Reported-by: Disha Goel <disgoel@linux.ibm.com>
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> >  tests/generic/775 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/generic/775 b/tests/generic/775
> > index 2a4287bb..19ae95e2 100755
> > --- a/tests/generic/775
> > +++ b/tests/generic/775
> > @@ -41,7 +41,7 @@ prep_mixed_mapping() {
> >  
> >  	local operations=("W" "H" "U")
> >  	local num_blocks=$((awu_max / blksz))
> > -	for ((i=0; i<num_blocks; i++)); do
> > +	for ((j=0; j<num_blocks; j++)); do
> 
> You're right that the single-letter variables are a really bad idea in
> languages like bash/python/etc where functions can see variables in
> caller's scope if they're not explicitly marked local.
> 
> But a) why not use "local i" to prevent that; and (b) if you're going
> to rename it, why not "blkno"?

Hey Darrick thanks for the review.

Why not both a) and b) :).
I probably shouldve done that in the first place. I'll fix in v2

Thanks,
ojaswin

> 
> --D
> 
> >  		local index=$((RANDOM % ${#operations[@]}))
> >  		local map="${operations[$index]}"
> >  		local mapping="${mapping}${map}"
> > -- 
> > 2.53.0
> > 
> > 

Re: [PATCH 4/4] generic/775: Fix an infinite loop due to variable name clash

[Darrick J. Wong]

On Sun, Apr 05, 2026 at 07:37:08PM +0530, Ojaswin Mujoo wrote:
> On Wed, Apr 01, 2026 at 07:32:18AM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 01, 2026 at 04:10:50PM +0530, Ojaswin Mujoo wrote:
> > > We use i as the iteration variable in the main test loop as well as some
> > > internal loops. Due to this clash, $i variable of main test loops was
> > > getting modified by the following loop in prep_mixed_mapping().
> > > 
> > > 	for ((i=0; i<num_blocks; i++)); do
> > > 
> > > If num_blocks is less than 10 (example ext4 with blocksize 4k and
> > > cluster size 8k) i would always be set as 3 and the main loop would
> > > never exit. Take the simplest approach of just renaming the variable.
> > > 
> > > Reported-by: Disha Goel <disgoel@linux.ibm.com>
> > > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > ---
> > >  tests/generic/775 | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/generic/775 b/tests/generic/775
> > > index 2a4287bb..19ae95e2 100755
> > > --- a/tests/generic/775
> > > +++ b/tests/generic/775
> > > @@ -41,7 +41,7 @@ prep_mixed_mapping() {
> > >  
> > >  	local operations=("W" "H" "U")
> > >  	local num_blocks=$((awu_max / blksz))
> > > -	for ((i=0; i<num_blocks; i++)); do
> > > +	for ((j=0; j<num_blocks; j++)); do
> > 
> > You're right that the single-letter variables are a really bad idea in
> > languages like bash/python/etc where functions can see variables in
> > caller's scope if they're not explicitly marked local.
> > 
> > But a) why not use "local i" to prevent that; and (b) if you're going
> > to rename it, why not "blkno"?
> 
> Hey Darrick thanks for the review.
> 
> Why not both a) and b) :).
> I probably shouldve done that in the first place. I'll fix in v2

Yeah, that works! :)

Thanks for making those fixes.

--D

> Thanks,
> ojaswin
> 
> > 
> > --D
> > 
> > >  		local index=$((RANDOM % ${#operations[@]}))
> > >  		local map="${operations[$index]}"
> > >  		local mapping="${mapping}${map}"
> > > -- 
> > > 2.53.0
> > > 
> > > 
> 

Re: [PATCH 1/4] ext4/061,062: Minor fixes and refactoring

[Darrick J. Wong]

On Wed, Apr 01, 2026 at 04:10:47PM +0530, Ojaswin Mujoo wrote:
> Fix 2 issues in the tests:
> 
> 1. Use fs atomic write limits instead of bdev's
> 
> The tests use block device's limits instead of limits advertised
> by filesystem. This can cause failures because block device might
> advertise a higher maximum than the FS.  Fix the tests to use
> filesystem's limits instead.
> 
> 2. Fix the test loop range
> 
> Fix the test loop start and end so that our calculations for
> blocksize, clustersize and iosize are correct.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Looks fine to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  tests/ext4/061 | 28 ++++++++++++++++++++--------
>  tests/ext4/062 | 26 +++++++++++++++++++-------
>  2 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/ext4/061 b/tests/ext4/061
> index 1d61c8b0..69ddeaca 100755
> --- a/tests/ext4/061
> +++ b/tests/ext4/061
> @@ -31,10 +31,19 @@ _require_aiodio
>  FIO_LOAD=$(($(nproc) * 2 * LOAD_FACTOR))
>  SIZE=$((100*1024*1024))
>  
> -# Calculate fsblocksize as per bdev atomic write units.
> -bdev_awu_min=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> -bdev_awu_max=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> -bs=$(_max 4096 "$bdev_awu_min")
> +_scratch_mkfs > /dev/null 2>&1 || \
> +	_notrun "mkfs failed"
> +_try_scratch_mount || \
> +	_notrun "mount failed"
> +
> +touch $SCRATCH_MNT/f
> +
> +fs_awu_min=$(_get_atomic_write_unit_min $SCRATCH_MNT/f)
> +fs_awu_max=$(_get_atomic_write_unit_max $SCRATCH_MNT/f)
> +bs=$(_max 4096 "$fs_awu_min")
> +ps=$(_get_page_size)
> +
> +_scratch_unmount
>  
>  function create_fio_configs()
>  {
> @@ -97,10 +106,11 @@ run_test_one() {
>  	local cs=$2
>  	local iosize=$3
>  
> +	echo "== Testing: bs=$bs cs=$cs iosize=$iosize ==" >> $seqres.full
> +
>  	MKFS_OPTIONS="-O bigalloc -b $bs -C $cs"
>  	_scratch_mkfs_ext4  >> $seqres.full 2>&1 || return
>  	if _try_scratch_mount >> $seqres.full 2>&1; then
> -		echo "== Testing: bs=$bs cs=$cs iosize=$iosize ==" >> $seqres.full
>  
>  		touch $SCRATCH_MNT/f1
>  		create_fio_configs $iosize
> @@ -127,7 +137,7 @@ run_test() {
>  	# cluster sizes above 16 x blocksize are experimental so avoid them
>  	# Also, cap cluster size at 128kb to keep it reasonable for large
>  	# blocks size
> -	max_cs=$(_min $((16 * bs)) "$bdev_awu_max" $((128 * 1024)))
> +	max_cs=$(_min $((16 * bs)) "$fs_awu_max" $((128 * 1024)))
>  
>  	# Fuzz for combinations of blocksize, clustersize and
>  	# iosize that cover most of the cases
> @@ -145,8 +155,10 @@ fio_out=$tmp.fio.out
>  create_fio_configs $bs
>  _require_fio $fio_aw_config
>  
> -for ((bs=$bs; bs <= $(_get_page_size); bs = $bs << 1)); do
> -	run_test $bs
> +echo "Awu min: $fs_awu_min Awu max: $fs_awu_max" >> $seqres.full
> +
> +for ((bs=$fs_awu_min; bs <= $(_min ps fs_awu_max); bs = $bs << 1)); do
> +	run_test $bs $cs $iosize
>  done
>  
>  # success, all done
> diff --git a/tests/ext4/062 b/tests/ext4/062
> index 05cce696..56bf080c 100755
> --- a/tests/ext4/062
> +++ b/tests/ext4/062
> @@ -33,10 +33,19 @@ _require_aiodio
>  FSSIZE=$((360*1024*1024))
>  FIO_LOAD=$(($(nproc) * LOAD_FACTOR))
>  
> -# Calculate bs as per bdev atomic write units.
> -bdev_awu_min=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> -bdev_awu_max=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> -bs=$(_max 4096 "$bdev_awu_min")
> +_scratch_mkfs > /dev/null 2>&1 || \
> +	_notrun "mkfs failed"
> +_try_scratch_mount || \
> +	_notrun "mount failed"
> +
> +touch $SCRATCH_MNT/f
> +
> +fs_awu_min=$(_get_atomic_write_unit_min $SCRATCH_MNT/f)
> +fs_awu_max=$(_get_atomic_write_unit_max $SCRATCH_MNT/f)
> +bs=$(_max 4096 "$fs_awu_min")
> +ps=$(_get_page_size)
> +
> +_scratch_unmount
>  
>  function create_fio_configs()
>  {
> @@ -146,10 +155,11 @@ run_test_one() {
>  	local cs=$2
>  	local iosize=$3
>  
> +	echo "Testing: bs=$bs cs=$cs iosize=$iosize" >> $seqres.full
> +
>  	MKFS_OPTIONS="-O bigalloc -b $bs -C $cs"
>  	_scratch_mkfs_sized "$FSSIZE" >> $seqres.full 2>&1 || return
>  	if _try_scratch_mount >> $seqres.full 2>&1; then
> -		echo "Testing: bs=$bs cs=$cs iosize=$iosize" >> $seqres.full
>  
>  		touch $SCRATCH_MNT/f1
>  		create_fio_configs $iosize
> @@ -175,7 +185,7 @@ run_test() {
>  	# cluster sizes above 16 x blocksize are experimental so avoid them
>  	# Also, cap cluster size at 128kb to keep it reasonable for large
>  	# blocks size
> -	max_cs=$(_min $((16 * bs)) "$bdev_awu_max" $((128 * 1024)))
> +	max_cs=$(_min $((16 * bs)) "$fs_awu_max" $((128 * 1024)))
>  
>  	# Fuzz for combinations of blocksize, clustersize and
>  	# iosize that cover most of the cases
> @@ -193,7 +203,9 @@ fio_out=$tmp.fio.out
>  create_fio_configs $bs
>  _require_fio $fio_aw_config
>  
> -for ((bs=$bs; bs <= $(_get_page_size); bs = $bs << 1)); do
> +echo "Awu min: $fs_awu_min Awu max: $fs_awu_max" >> $seqres.full
> +
> +for ((bs=$fs_awu_min; bs <= $(_min ps fs_awu_max); bs = $bs << 1)); do
>  	run_test $bs $cs $iosize
>  done
>  
> -- 
> 2.53.0
> 
>