| Message ID | 20250318072835.3508696-3-yi.zhang@huaweicloud.com |
|---|---|
| State | New |
| Headers | show |
| Series | blktest: add unmap write zeroes tests | expand |
On Mar 18, 2025 / 15:28, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@huawei.com> > > Test block device unmap write zeroes sysfs interface with device-mapper > stacked devices. The /sys/block/<disk>/queue/write_zeroes_unmap > interface should return 1 if the underlying devices support the unmap > write zeroes command, and it should return 0 otherwise. > > Signed-off-by: Zhang Yi <yi.zhang@huawei.com> > --- > common/rc | 16 ++++++++++++++ > tests/dm/003 | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ > tests/dm/003.out | 2 ++ > 3 files changed, 75 insertions(+) > create mode 100755 tests/dm/003 > create mode 100644 tests/dm/003.out > > diff --git a/common/rc b/common/rc > index bc6c2e4..60c21f2 100644 > --- a/common/rc > +++ b/common/rc > @@ -615,3 +615,19 @@ _io_uring_restore() > echo "$IO_URING_DISABLED" > /proc/sys/kernel/io_uring_disabled > fi > } > + > +# get real device path name by following link > +_real_dev() > +{ > + local dev=$1 > + if [ -b "$dev" ] && [ -L "$dev" ]; then > + dev=`readlink -f "$dev"` > + fi > + echo $dev > +} This helper function looks useful, and it looks reasonable to add it. > + > +# basename of a device > +_short_dev() > +{ > + echo `basename $(_real_dev $1)` > +} But I'm not sure about this one. The name "_short_dev" is not super clear for me. > diff --git a/tests/dm/003 b/tests/dm/003 > new file mode 100755 > index 0000000..1013eb5 > --- /dev/null > +++ b/tests/dm/003 > @@ -0,0 +1,57 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-3.0+ > +# Copyright (C) 2025 Huawei. > +# > +# Test block device unmap write zeroes sysfs interface with device-mapper > +# stacked devices. > + > +. tests/dm/rc > +. common/scsi_debug > + > +DESCRIPTION="test unmap write zeroes sysfs interface with dm devices" > +QUICK=1 > + > +requires() { > + _have_scsi_debug > +} > + > +device_requries() { > + _require_test_dev_sysfs queue/write_zeroes_unmap > +} Same comment as the 1st patch: device_requries() does not work here. > + > +setup_test_device() { > + if ! _configure_scsi_debug "$@"; then > + return 1 > + fi In same manner as the 1st patch, I suggest to check /queue/write_zeroes_unmap here. if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then _exit_scsi_debug SKIP_REASONS+=("kernel does not support unmap write zeroes sysfs interface") return 1 fi The caller will need to check setup_test_device() return value. > + > + local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" > + local blk_sz="$(blockdev --getsz "$dev")" > + dmsetup create test --table "0 $blk_sz linear $dev 0" I suggest to call _real_dev() here, and echo back the device name. dpath=$(_real_dev /dev/mapper/test) echo ${dpath##*/} The bash parameter expansion ${xxx##*/} works in same manner as the basename command. The caller can receive the device name in a local variable. This will avoid a bit of code duplication, and allow to avoid _short_dev(). > +} > + > +cleanup_test_device() { > + dmsetup remove test > + _exit_scsi_debug > +} > + > +test() { > + echo "Running ${TEST_NAME}" > + > + # disable WRITE SAME with unmap > + setup_test_device lbprz=0 > + umap="$(cat "/sys/block/$(_short_dev /dev/mapper/test)/queue/write_zeroes_unmap")" I suggest to modify the two lines above as follows, to match with the other suggested changes: local dname umap if ! dname=$(setup_test_device lbprz=0); then return 1 fi umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")" (Please note that the suggested changes are untested)
diff --git a/common/rc b/common/rc index bc6c2e4..60c21f2 100644 --- a/common/rc +++ b/common/rc @@ -615,3 +615,19 @@ _io_uring_restore() echo "$IO_URING_DISABLED" > /proc/sys/kernel/io_uring_disabled fi } + +# get real device path name by following link +_real_dev() +{ + local dev=$1 + if [ -b "$dev" ] && [ -L "$dev" ]; then + dev=`readlink -f "$dev"` + fi + echo $dev +} + +# basename of a device +_short_dev() +{ + echo `basename $(_real_dev $1)` +} diff --git a/tests/dm/003 b/tests/dm/003 new file mode 100755 index 0000000..1013eb5 --- /dev/null +++ b/tests/dm/003 @@ -0,0 +1,57 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2025 Huawei. +# +# Test block device unmap write zeroes sysfs interface with device-mapper +# stacked devices. + +. tests/dm/rc +. common/scsi_debug + +DESCRIPTION="test unmap write zeroes sysfs interface with dm devices" +QUICK=1 + +requires() { + _have_scsi_debug +} + +device_requries() { + _require_test_dev_sysfs queue/write_zeroes_unmap +} + +setup_test_device() { + if ! _configure_scsi_debug "$@"; then + return 1 + fi + + local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" + local blk_sz="$(blockdev --getsz "$dev")" + dmsetup create test --table "0 $blk_sz linear $dev 0" +} + +cleanup_test_device() { + dmsetup remove test + _exit_scsi_debug +} + +test() { + echo "Running ${TEST_NAME}" + + # disable WRITE SAME with unmap + setup_test_device lbprz=0 + umap="$(cat "/sys/block/$(_short_dev /dev/mapper/test)/queue/write_zeroes_unmap")" + if [[ $umap -ne 0 ]]; then + echo "Test disable WRITE SAME with unmap failed." + fi + cleanup_test_device + + # enable WRITE SAME with unmap + setup_test_device lbprz=1 lbpws=1 + umap="$(cat "/sys/block/$(_short_dev /dev/mapper/test)/queue/write_zeroes_unmap")" + if [[ $umap -ne 1 ]]; then + echo "Test enable WRITE SAME with unmap failed." + fi + cleanup_test_device + + echo "Test complete" +} diff --git a/tests/dm/003.out b/tests/dm/003.out new file mode 100644 index 0000000..51a5405 --- /dev/null +++ b/tests/dm/003.out @@ -0,0 +1,2 @@ +Running dm/003 +Test complete