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

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 <[email protected]>
> > Signed-off-by: Ojaswin Mujoo <[email protected]>
> > ---
> >  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
> > 
> >