Linux block layer
 help / color / mirror / Atom feed
* [PATCH blktests 0/2] Add atomic write tests for scsi and nvme
@ 2025-01-21 22:25 Alan Adamson
  2025-01-21 22:25 ` [PATCH blktests 1/2] scsi/009: add atomic write tests Alan Adamson
  2025-01-21 22:25 ` [PATCH blktests 2/2] nvme/059: " Alan Adamson
  0 siblings, 2 replies; 5+ messages in thread
From: Alan Adamson @ 2025-01-21 22:25 UTC (permalink / raw)
  To: linux-block; +Cc: linux-scsi, alan.adamson, linux-nvme, shinichiro.kawasaki

Add tests for atomic write support.

Tests will be delivered for scsi (using scsi_debug) and nvme.  NVMe can use the qemu-nvme
emulated device that supports Controller-based Atomic Parameters (QEMU 9.2).

The xfs_io utility delivered with the xfsprogs-devel package (version 6.12) is required by
these tests.

The Linux Kernel 6.11 (and greater) supports Atomic Writes and is required by these tests. 

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

* [PATCH blktests 1/2] scsi/009: add atomic write tests
  2025-01-21 22:25 [PATCH blktests 0/2] Add atomic write tests for scsi and nvme Alan Adamson
@ 2025-01-21 22:25 ` Alan Adamson
  2025-01-22  0:51   ` Chaitanya Kulkarni
  2025-01-21 22:25 ` [PATCH blktests 2/2] nvme/059: " Alan Adamson
  1 sibling, 1 reply; 5+ messages in thread
From: Alan Adamson @ 2025-01-21 22:25 UTC (permalink / raw)
  To: linux-block; +Cc: linux-scsi, alan.adamson, linux-nvme, shinichiro.kawasaki

Uses scsi_debug to test basic atomic write functionality. Testing
areas include:

- Verify sysfs atomic write attributes are consistent with
  atomic write attributes advertised by scsi_debug.
- Verify the atomic write paramters of statx are correct using
  xfs_io.
- Perform a pwritev2() (with and without RWF_ATOMIC flag) using
  xfs_io:
    - maximum byte size (atomic_write_unit_max_bytes)
    - minimum byte size (atomic_write_unit_min_bytes)
    - a write larger than atomic_write_unit_max_bytes
    - a write smaller than atomic_write_unit_min_bytes

Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
---
 common/xfs         |  49 +++++++++++
 tests/scsi/009     | 213 +++++++++++++++++++++++++++++++++++++++++++++
 tests/scsi/009.out |  18 ++++
 3 files changed, 280 insertions(+)
 create mode 100755 tests/scsi/009
 create mode 100644 tests/scsi/009.out

diff --git a/common/xfs b/common/xfs
index 569770fecd53..284c6d7cdc40 100644
--- a/common/xfs
+++ b/common/xfs
@@ -6,6 +6,28 @@
 
 . common/shellcheck
 
+_have_xfs_io() {
+	if ! _have_program xfs_io; then
+		return 1
+	fi
+	return 0
+}
+
+# Check whether the version of xfs_io is greater than or equal to $1.$2.$3
+_have_xfs_io_ver() {
+	local d=$1 e=$2 f=$3
+
+	_have_xfs_io || return $?
+
+	IFS='.' read -r a b c < <(xfs_io -V | sed 's/xfs_io version *//')
+	if [ $((a * 65536 + b * 256 + c)) -lt $((d * 65536 + e * 256 + f)) ];
+	then
+		SKIP_REASONS+=("xfs_io version too old")
+		return 1
+	fi
+	return 0
+}
+
 _have_xfs() {
 	_have_fs xfs && _have_program mkfs.xfs
 }
@@ -52,3 +74,30 @@ _xfs_run_fio_verify_io() {
 
 	return "${rc}"
 }
+
+run_xfs_io_pwritev2() {
+	local dev=$1
+	local bytes_to_write=$2
+	local bytes_written
+
+	bytes_written=$(xfs_io -d -C "pwrite -b ${bytes_to_write} -V 1 -D 0 ${bytes_to_write}" "$dev" | grep "wrote" | sed 's/\// /g' | awk '{ print $2 }')
+	echo "$bytes_written"
+}
+
+run_xfs_io_pwritev2_atomic() {
+	local dev=$1
+	local bytes_to_write=$2
+	local bytes_written
+
+	bytes_written=$(xfs_io -d -C "pwrite -b ${bytes_to_write} -V 1 -A -D 0 ${bytes_to_write}" "$dev" | grep "wrote" | sed 's/\// /g' | awk '{ print $2 }')
+	echo "$bytes_written"
+}
+
+run_xfs_io_xstat() {
+	local dev=$1
+	local field=$2
+	local statx_output
+
+	statx_output=$(xfs_io -c "statx -r -m 0x00010000" "$dev" | grep "$field" | awk '{ print $3 }')
+	echo "$statx_output"
+}
diff --git a/tests/scsi/009 b/tests/scsi/009
new file mode 100755
index 000000000000..f3ab00f61369
--- /dev/null
+++ b/tests/scsi/009
@@ -0,0 +1,213 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2025 Oracle and/or its affiliates
+#
+# Test SCSI Atomic Writes with scsi_debug
+
+. tests/scsi/rc
+. common/scsi_debug
+. common/xfs
+
+DESCRIPTION="test scsi atomic writes"
+QUICK=1
+
+requires() {
+	_have_driver scsi_debug
+	_have_kver 6 11
+	_have_xfs_io_ver 6 12 0
+}
+
+test() {
+	local dev
+	local scsi_debug_atomic_wr_max_length
+	local scsi_debug_atomic_wr_gran
+	local scsi_atomic_max_bytes
+	local scsi_atomic_min_bytes
+	local sysfs_max_hw_sectors_kb
+	local max_hw_bytes
+	local sysfs_logical_block_size
+	local sysfs_atomic_max_bytes
+	local sysfs_atomic_unit_max_bytes
+	local sysfs_atomic_unit_min_bytes
+	local statx_atomic_min
+	local statx_atomic_max
+	local bytes_to_write
+	local bytes_written
+
+	echo "Running ${TEST_NAME}"
+
+	local scsi_debug_params=(
+		delay=0
+		atomic_wr=1
+	)
+	_configure_scsi_debug "${scsi_debug_params[@]}"
+	dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
+	sysfs_logical_block_size=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/logical_block_size)
+	sysfs_max_hw_sectors_kb=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/max_hw_sectors_kb)
+	max_hw_bytes=$(( "$sysfs_max_hw_sectors_kb" * 1024 ))
+	sysfs_atomic_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_max_bytes)
+	sysfs_atomic_unit_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_max_bytes)
+	sysfs_atomic_unit_min_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_min_bytes)
+	scsi_debug_atomic_wr_max_length=$(cat /sys/module/scsi_debug/parameters/atomic_wr_max_length)
+	scsi_debug_atomic_wr_gran=$(cat /sys/module/scsi_debug/parameters/atomic_wr_gran)
+	scsi_atomic_max_bytes=$(( "$scsi_debug_atomic_wr_max_length" * "$sysfs_logical_block_size" ))
+	scsi_atomic_min_bytes=$(( "$scsi_debug_atomic_wr_gran" * "$sysfs_logical_block_size" ))
+
+	# TEST 1 - Verify sysfs atomic attributes
+	if [ "$max_hw_bytes" -ge "$sysfs_atomic_max_bytes" ] &&
+		[ "$sysfs_atomic_max_bytes" -ge "$sysfs_atomic_unit_max_bytes" ] &&
+		[ "$sysfs_atomic_unit_max_bytes" -ge "$sysfs_atomic_unit_min_bytes" ]
+	then
+		echo "TEST 1 - pass"
+	else
+		"TEST 1 - fail $max_hw_bytes - $sysfs_max_hw_sectors_kb -" \
+			"$sysfs_atomic_max_bytes - $sysfs_atomic_unit_max_bytes -" \
+			"$sysfs_atomic_unit_min_bytes"
+	fi
+
+	# TEST 2 - check scsi_debug atomic_wr_max_length is the same as sysfs atomic_write_max_bytes
+	if [ "$scsi_atomic_max_bytes" -le "$max_hw_bytes" ]
+	then
+		if [ "$scsi_atomic_max_bytes" = "$sysfs_atomic_max_bytes" ]
+		then
+			echo "TEST 2 - pass"
+		else
+			echo "TEST 2 - fail $scsi_atomic_max_bytes - $max_hw_bytes -" \
+				"$sysfs_atomic_max_bytes"
+		fi
+	else
+		if [ "$sysfs_atomic_max_bytes" = "$max_hw_bytes" ]
+		then
+			echo "TEST 2 - pass"
+		else
+			echo "TEST 2 - fail $scsi_atomic_max_bytes - $max_hw_bytes -" \
+				"$sysfs_atomic_max_bytes"
+		fi
+	fi
+
+	# TEST 3 - check sysfs atomic_write_unit_max_bytes <= scsi_debug atomic_wr_max_length
+	if (("$sysfs_atomic_unit_max_bytes" <= "$scsi_atomic_max_bytes"))
+	then
+		echo "TEST 3 - pass"
+	else
+		echo "TEST 3 - fail $sysfs_atomic_unit_max_bytes - $scsi_atomic_max_bytes"
+	fi
+
+	# TEST 4 - check sysfs atomic_write_unit_min_bytes = scsi_debug atomic_wr_gran
+	if [ "$sysfs_atomic_unit_min_bytes" = "$scsi_atomic_min_bytes" ]
+	then
+		echo "TEST 4 - pass"
+	else
+		echo "TEST 4 - fail $sysfs_atomic_unit_min_bytes - $scsi_atomic_min_bytes"
+	fi
+
+	# TEST 5 - check statx stx_atomic_write_unit_min
+	statx_atomic_min=$(run_xfs_io_xstat "$dev" "stat.atomic_write_unit_min")
+	if [ "$statx_atomic_min" = "$scsi_atomic_min_bytes" ]
+	then
+		echo "TEST 5 - pass"
+	else
+		echo "TEST 5 - fail $statx_atomic_min - $scsi_atomic_min_bytes"
+	fi
+
+	# TEST  6 - check statx stx_atomic_write_unit_max
+	statx_atomic_max=$(run_xfs_io_xstat "$dev" "stat.atomic_write_unit_max")
+	if [ "$statx_atomic_max" = "$sysfs_atomic_unit_max_bytes" ]
+	then
+		echo "TEST 6 - pass"
+	else
+		echo "TEST 6 - fail $statx_atomic_max - $sysfs_atomic_unit_max_bytes"
+	fi
+
+	# TEST 7 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with no RWF_ATOMIC flag - pwritev2 should
+	# be succesful.
+	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$sysfs_atomic_unit_max_bytes")
+	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
+	then
+		echo "TEST 7 - pass"
+	else
+		echo "TEST 7 - fail $bytes_written - $sysfs_atomic_unit_max_bytes"
+	fi
+
+	# TEST 8 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with RWF_ATOMIC flag - pwritev2 should
+	# be succesful.
+	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$sysfs_atomic_unit_max_bytes")
+	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
+	then
+		echo "TEST 8 - pass"
+	else
+		echo "TEST 8 - fail $bytes_written - $sysfs_atomic_unit_max_bytes"
+	fi
+
+	# TEST 9 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 512 bytes with no RWF_ATOMIC flag - pwritev2
+	# should be succesful.
+	bytes_to_write=$(( "${sysfs_atomic_unit_max_bytes}" + "$sysfs_logical_block_size" ))
+	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$bytes_to_write")
+	if [ "$bytes_written" = "$bytes_to_write" ]
+	then
+		echo "TEST 9 - pass"
+	else
+		echo "TEST 9 - fail $bytes_written - $bytes_to_write"
+	fi
+
+	# TEST 10 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 512 bytes with RWF_ATOMIC flag - pwritev2
+	# should not be succesful.
+	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$bytes_to_write")
+	if [ "$bytes_written" = "" ]
+	then
+		echo "TEST 10 - pass"
+	else
+		echo "TEST 10 - fail $bytes_written - $bytes_to_write"
+	fi
+
+	# TEST 11 - perform a pwritev2 with size of sysfs_atomic_unit_min_bytes with no RWF_ATOMIC flag - pwritev2 should
+	# be succesful.
+	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$sysfs_atomic_unit_min_bytes")
+	if [ "$bytes_written" = "$sysfs_atomic_unit_min_bytes" ]
+	then
+		echo "TEST 11 - pass"
+	else
+		echo "TEST 11 - fail $bytes_written - $scsi_atomic_min_bytes"
+	fi
+
+	# TEST 12 - perform a pwritev2 with size of sysfs_atomic_unit_min_bytes with RWF_ATOMIC flag - pwritev2 should
+	# be succesful.
+	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$sysfs_atomic_unit_min_bytes")
+	if [ "$bytes_written" = "$sysfs_atomic_unit_min_bytes" ]
+	then
+		echo "TEST 12 - pass"
+	else
+		echo "TEST 12 - fail $bytes_written - $scsi_atomic_min_bytes"
+	fi
+
+	# TEST 13 - perform a pwritev2 with a size of sysfs_atomic_unit_min_bytes - 512 bytes with no
+	# RWF_ATOMIC flag - pwritev2 should be succesful.
+	# TEST 14 - perform a pwritev2 with a size of sysfs_atomic_unit_min_bytes - 512 bytes with
+        # RWF_ATOMIC flag - pwritev2 should fail.
+	bytes_to_write=$(( "${sysfs_atomic_unit_min_bytes}" - "${sysfs_logical_block_size}" ))
+	if [ "$bytes_to_write" = 0 ]
+	then
+		# sysfs_atomic_unit_min_bytes is set to 1 logical block so these tests aren't needed.
+		echo "TEST 13 - pass"
+		echo "TEST 14 - pass"
+	else
+		bytes_written=$(run_xfs_io_pwritev2 "$dev" "$bytes_to_write")
+		if [ "$bytes_written" = "$bytes_to_write" ]
+		then
+			echo "TEST 13 - pass"
+		else
+			echo "TEST 13 - fail $bytes_written - $bytes_to_write"
+		fi
+		bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$bytes_to_write")
+		if [ "$bytes_written" = "" ]
+		then
+			echo "TEST 14 - pass"
+		else
+			echo "TEST 14 - fail $bytes_written - $bytes_to_write"
+		fi
+	fi
+
+	_exit_scsi_debug
+
+	echo "Test complete"
+}
diff --git a/tests/scsi/009.out b/tests/scsi/009.out
new file mode 100644
index 000000000000..3a3382b1190c
--- /dev/null
+++ b/tests/scsi/009.out
@@ -0,0 +1,18 @@
+Running scsi/009
+TEST 1 - pass
+TEST 2 - pass
+TEST 3 - pass
+TEST 4 - pass
+TEST 5 - pass
+TEST 6 - pass
+TEST 7 - pass
+TEST 8 - pass
+TEST 9 - pass
+pwrite: Invalid argument
+TEST 10 - pass
+TEST 11 - pass
+TEST 12 - pass
+TEST 13 - pass
+pwrite: Invalid argument
+TEST 14 - pass
+Test complete
-- 
2.43.5


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

* [PATCH blktests 2/2] nvme/059: add atomic write tests
  2025-01-21 22:25 [PATCH blktests 0/2] Add atomic write tests for scsi and nvme Alan Adamson
  2025-01-21 22:25 ` [PATCH blktests 1/2] scsi/009: add atomic write tests Alan Adamson
@ 2025-01-21 22:25 ` Alan Adamson
  1 sibling, 0 replies; 5+ messages in thread
From: Alan Adamson @ 2025-01-21 22:25 UTC (permalink / raw)
  To: linux-block; +Cc: linux-scsi, alan.adamson, linux-nvme, shinichiro.kawasaki

Tests basic atomic write functionality using NVMe devices
that support the AWUN and AWUPF Controller Atomic Parameters 
and NAWUN and NAWUPF Namespace Atomic Parameters.

Testing areas include:

- Verify sysfs atomic write attributes are consistent with
  atomic write capablities advertised by the NVMe HW.

- Verify the atomic write paramters of statx are correct using
  xfs_io.

- Perform a pwritev2() (with and without RWF_ATOMIC flag) using
  xfs_io:
    - maximum byte size (atomic_write_unit_max_bytes)
    - a write larger than atomic_write_unit_max_bytes

Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
---
 tests/nvme/059     | 138 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/059.out |  10 ++++
 2 files changed, 148 insertions(+)
 create mode 100755 tests/nvme/059
 create mode 100644 tests/nvme/059.out

diff --git a/tests/nvme/059 b/tests/nvme/059
new file mode 100755
index 000000000000..af4a4263329d
--- /dev/null
+++ b/tests/nvme/059
@@ -0,0 +1,138 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2025 Oracle and/or its affiliates
+#
+# Test NVMe Atomic Writes
+
+. tests/nvme/rc
+. common/xfs
+
+DESCRIPTION="test atomic writes"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_program nvme
+	_have_kver 6 11
+	_have_xfs_io_ver 6 12 0
+}
+
+test_device() {
+	local ns_dev
+	local ctrl_dev
+	local nvme_awupf
+	local nvme_nsfeat
+	local nvme_nsabp
+	local atomic_max_bytes
+	local statx_atomic_max
+	local sysfs_atomic_max_bytes
+	local sysfs_atomic_unit_max_bytes
+	local sysfs_logical_block_size
+	local bytes_written
+	local bytes_to_write
+
+	echo "Running ${TEST_NAME}"
+	ns_dev=${TEST_DEV##*/}
+	ctrl_dev=${ns_dev%n*}
+
+	# TEST 1 - Verify sysfs attributes
+	sysfs_logical_block_size=$(cat "${TEST_DEV_SYSFS}"/queue/logical_block_size)
+	sysfs_max_hw_sectors_kb=$(cat "${TEST_DEV_SYSFS}"/queue/max_hw_sectors_kb)
+	max_hw_bytes=$(( "$sysfs_max_hw_sectors_kb" * 1024 ))
+	sysfs_atomic_max_bytes=$(cat "${TEST_DEV_SYSFS}"/queue/atomic_write_max_bytes)
+	sysfs_atomic_unit_max_bytes=$(cat "${TEST_DEV_SYSFS}"/queue/atomic_write_unit_max_bytes)
+	sysfs_atomic_unit_min_bytes=$(cat "${TEST_DEV_SYSFS}"/queue/atomic_write_unit_min_bytes)
+
+	if [ "$max_hw_bytes" -ge "$sysfs_atomic_max_bytes" ] &&
+		[ "$sysfs_atomic_max_bytes" -ge "$sysfs_atomic_unit_max_bytes" ] &&
+		[ "$sysfs_atomic_unit_max_bytes" -ge "$sysfs_atomic_unit_min_bytes" ]
+	then
+		echo "TEST 1 - pass"
+	else
+		echo "TEST 1 - fail $max_hw_bytes - $sysfs_max_hw_sectors_kb -" \
+			"$sysfs_atomic_max_bytes - $sysfs_atomic_unit_max_bytes -" \
+			"$sysfs_atomic_unit_min_bytes"
+	fi
+
+	# TEST 2 - Verify sysfs atomic_write_unit_max_bytes is consistent with NVMe AWUPF/NAWUPF
+	nvme_nsfeat=$(nvme id-ns /dev/"${ns_dev}" | grep nsfeat | awk '{ print $3}')
+	nvme_nsabp=$((("$nvme_nsfeat" & 0x2) != 0))
+	if [ "$nvme_nsabp" = 1 ] # Check if NSABP is set
+	then
+		nvme_awupf=$(nvme id-ns /dev/"$ns_dev" | grep nawupf | awk '{ print $3}')
+		atomic_max_bytes=$(( ("$nvme_awupf" + 1) * "$sysfs_logical_block_size" ))
+	else
+		nvme_awupf=$(nvme id-ctrl /dev/"${ctrl_dev}" | grep awupf | awk '{ print $3}')
+		atomic_max_bytes=$(( ("$nvme_awupf" + 1) * "$sysfs_logical_block_size" ))
+	fi
+	if [ "$atomic_max_bytes" -le "$max_hw_bytes" ]
+	then
+		if [ "$atomic_max_bytes" = "$sysfs_atomic_max_bytes" ]
+		then
+			echo "TEST 2 - pass"
+		else
+			echo "TEST 2 - fail $nvme_nsabp - $atomic_max_bytes - $sysfs_atomic_max_bytes -" \
+				"$max_hw_bytes"
+		fi
+	else
+		if [ "$sysfs_atomic_max_bytes" = "$max_hw_bytes" ]
+		then
+			echo "TEST 2 - pass"
+		else
+			echo "TEST 2 - fail $nvme_nsabp - $atomic_max_bytes - $sysfs_atomic_max_bytes -" \
+				"$max_hw_bytes"
+		fi
+	fi
+
+	# TEST 3 - Verify statx is correctly reporting atomic_unit_max_bytes
+	statx_atomic_max=$(run_xfs_io_xstat /dev/"$ns_dev" "stat.atomic_write_unit_max")
+	if [ "$sysfs_atomic_unit_max_bytes" = "$statx_atomic_max" ]
+	then
+		echo "TEST 3 - pass"
+	else
+		echo "TEST 3 - fail $statx_atomic_max - $sysfs_atomic_unit_max_bytes"
+	fi
+
+	# TEST 4 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with no RWF_ATOMIC
+	# flag - pwritev2 should be succesful.
+        bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns_dev" "$sysfs_atomic_unit_max_bytes")
+        if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
+        then
+                echo "TEST 4 - pass"
+        else
+                echo "TEST 4 - fail $bytes_written - $sysfs_atomic_unit_max_bytes"
+        fi
+
+	# TEST 5 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with RWF_ATOMIC
+	# flag - pwritev2 should  be succesful.
+	bytes_written=$(run_xfs_io_pwritev2_atomic /dev/"$ns_dev" "$sysfs_atomic_unit_max_bytes")
+	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
+	then
+		echo "TEST 5 - pass"
+	else
+		echo "TEST 5 - fail $bytes_written - $sysfs_atomic_unit_max_bytes"
+	fi
+
+	# TEST 6 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 1 logical block with no
+	# RWF_ATOMIC flag - pwritev2 should be succesful.
+	bytes_to_write=$(( "$sysfs_atomic_unit_max_bytes" + "$sysfs_logical_block_size" ))
+	bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns_dev" "$bytes_to_write")
+	if [ "$bytes_written" = "$bytes_to_write" ]
+	then
+		echo "TEST 6 - pass"
+	else
+		echo "TEST 6 - fail $bytes_written - $bytes_to_write"
+	fi
+
+	# TEST 7 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + logical block with
+	# RWF_ATOMIC flag - pwritev2 should not be succesful.
+	bytes_written=$(run_xfs_io_pwritev2_atomic /dev/"$ns_dev" "$bytes_to_write")
+	if [ "$bytes_written" = "" ]
+	then
+		echo "TEST 7 - pass"
+	else
+		echo "TEST 7 - fail $bytes_written - $bytes_to_write"
+	fi
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/059.out b/tests/nvme/059.out
new file mode 100644
index 000000000000..45bc5d3566b4
--- /dev/null
+++ b/tests/nvme/059.out
@@ -0,0 +1,10 @@
+Running nvme/059
+TEST 1 - pass
+TEST 2 - pass
+TEST 3 - pass
+TEST 4 - pass
+TEST 5 - pass
+TEST 6 - pass
+pwrite: Invalid argument
+TEST 7 - pass
+Test complete
-- 
2.43.5


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

* Re: [PATCH blktests 1/2] scsi/009: add atomic write tests
  2025-01-21 22:25 ` [PATCH blktests 1/2] scsi/009: add atomic write tests Alan Adamson
@ 2025-01-22  0:51   ` Chaitanya Kulkarni
  2025-01-24  5:36     ` Shinichiro Kawasaki
  0 siblings, 1 reply; 5+ messages in thread
From: Chaitanya Kulkarni @ 2025-01-22  0:51 UTC (permalink / raw)
  To: Alan Adamson, linux-block@vger.kernel.org
  Cc: linux-scsi@vger.kernel.org, linux-nvme@lists.infradead.org,
	shinichiro.kawasaki@wdc.com

On 1/21/25 14:25, Alan Adamson wrote:
> Uses scsi_debug to test basic atomic write functionality. Testing
> areas include:
>
> - Verify sysfs atomic write attributes are consistent with
>    atomic write attributes advertised by scsi_debug.
> - Verify the atomic write paramters of statx are correct using
>    xfs_io.
> - Perform a pwritev2() (with and without RWF_ATOMIC flag) using
>    xfs_io:
>      - maximum byte size (atomic_write_unit_max_bytes)
>      - minimum byte size (atomic_write_unit_min_bytes)
>      - a write larger than atomic_write_unit_max_bytes
>      - a write smaller than atomic_write_unit_min_bytes
>
> Signed-off-by: Alan Adamson <alan.adamson@oracle.com>

Thanks a lot for the testcase, this is really useful based on
amount of work has been done on the kernel side.

> ---
>   common/xfs         |  49 +++++++++++
>   tests/scsi/009     | 213 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/scsi/009.out |  18 ++++
>   3 files changed, 280 insertions(+)
>   create mode 100755 tests/scsi/009
>   create mode 100644 tests/scsi/009.out
>
> diff --git a/common/xfs b/common/xfs
> index 569770fecd53..284c6d7cdc40 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -6,6 +6,28 @@
>   
>   . common/shellcheck
>   
> +_have_xfs_io() {
> +	if ! _have_program xfs_io; then
> +		return 1
> +	fi
> +	return 0
> +}
> +
> +# Check whether the version of xfs_io is greater than or equal to $1.$2.$3
> +_have_xfs_io_ver() {
> +	local d=$1 e=$2 f=$3
> +
> +	_have_xfs_io || return $?
> +
> +	IFS='.' read -r a b c < <(xfs_io -V | sed 's/xfs_io version *//')
> +	if [ $((a * 65536 + b * 256 + c)) -lt $((d * 65536 + e * 256 + f)) ];

can we add some comments for above calculations ?

> +	then
> +		SKIP_REASONS+=("xfs_io version too old")
> +		return 1
> +	fi
> +	return 0
> +}
> +
>   _have_xfs() {
>   	_have_fs xfs && _have_program mkfs.xfs
>   }
> @@ -52,3 +74,30 @@ _xfs_run_fio_verify_io() {
>   
>   	return "${rc}"
>   }
> +
> +run_xfs_io_pwritev2() {
> +	local dev=$1
> +	local bytes_to_write=$2
> +	local bytes_written
> +
> +	bytes_written=$(xfs_io -d -C "pwrite -b ${bytes_to_write} -V 1 -D 0 ${bytes_to_write}" "$dev" | grep "wrote" | sed 's/\// /g' | awk '{ print $2 }')

same here little comment would be really useful

> +	echo "$bytes_written"
> +}
> +
> +run_xfs_io_pwritev2_atomic() {
> +	local dev=$1
> +	local bytes_to_write=$2
> +	local bytes_written
> +
> +	bytes_written=$(xfs_io -d -C "pwrite -b ${bytes_to_write} -V 1 -A -D 0 ${bytes_to_write}" "$dev" | grep "wrote" | sed 's/\// /g' | awk '{ print $2 }')

same here

> +	echo "$bytes_written"
> +}
> +
> +run_xfs_io_xstat() {
> +	local dev=$1
> +	local field=$2
> +	local statx_output
> +
> +	statx_output=$(xfs_io -c "statx -r -m 0x00010000" "$dev" | grep "$field" | awk '{ print $3 }')

same here

> +	echo "$statx_output"
> +}
> diff --git a/tests/scsi/009 b/tests/scsi/009
> new file mode 100755
> index 000000000000..f3ab00f61369
> --- /dev/null
> +++ b/tests/scsi/009
> @@ -0,0 +1,213 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2025 Oracle and/or its affiliates
> +#
> +# Test SCSI Atomic Writes with scsi_debug
> +
> +. tests/scsi/rc
> +. common/scsi_debug
> +. common/xfs
> +
> +DESCRIPTION="test scsi atomic writes"
> +QUICK=1
> +
> +requires() {
> +	_have_driver scsi_debug
> +	_have_kver 6 11
> +	_have_xfs_io_ver 6 12 0
> +}
> +
> +test() {
> +	local dev
> +	local scsi_debug_atomic_wr_max_length
> +	local scsi_debug_atomic_wr_gran
> +	local scsi_atomic_max_bytes
> +	local scsi_atomic_min_bytes
> +	local sysfs_max_hw_sectors_kb
> +	local max_hw_bytes
> +	local sysfs_logical_block_size
> +	local sysfs_atomic_max_bytes
> +	local sysfs_atomic_unit_max_bytes
> +	local sysfs_atomic_unit_min_bytes
> +	local statx_atomic_min
> +	local statx_atomic_max
> +	local bytes_to_write
> +	local bytes_written
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	local scsi_debug_params=(
> +		delay=0
> +		atomic_wr=1
> +	)
> +	_configure_scsi_debug "${scsi_debug_params[@]}"
> +	dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
> +	sysfs_logical_block_size=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/logical_block_size)

you can also use the local variable for following path to trim down the
cat operations where :-

/sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue

makes the code easy to read ...

> +	sysfs_max_hw_sectors_kb=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/max_hw_sectors_kb)
> +	max_hw_bytes=$(( "$sysfs_max_hw_sectors_kb" * 1024 ))
> +	sysfs_atomic_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_max_bytes)
> +	sysfs_atomic_unit_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_max_bytes)
> +	sysfs_atomic_unit_min_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_min_bytes)
> +	scsi_debug_atomic_wr_max_length=$(cat /sys/module/scsi_debug/parameters/atomic_wr_max_length)
> +	scsi_debug_atomic_wr_gran=$(cat /sys/module/scsi_debug/parameters/atomic_wr_gran)
> +	scsi_atomic_max_bytes=$(( "$scsi_debug_atomic_wr_max_length" * "$sysfs_logical_block_size" ))
> +	scsi_atomic_min_bytes=$(( "$scsi_debug_atomic_wr_gran" * "$sysfs_logical_block_size" ))
> +
> +	# TEST 1 - Verify sysfs atomic attributes

I'd use local test_desc var and assign the above description
string in the above comment and use that in the echo

> +	if [ "$max_hw_bytes" -ge "$sysfs_atomic_max_bytes" ] &&
> +		[ "$sysfs_atomic_max_bytes" -ge "$sysfs_atomic_unit_max_bytes" ] &&
> +		[ "$sysfs_atomic_unit_max_bytes" -ge "$sysfs_atomic_unit_min_bytes" ]
> +	then
> +		echo "TEST 1 - pass"
> +	else
> +		"TEST 1 - fail $max_hw_bytes - $sysfs_max_hw_sectors_kb -" \
> +			"$sysfs_atomic_max_bytes - $sysfs_atomic_unit_max_bytes -" \
> +			"$sysfs_atomic_unit_min_bytes"

I think echo is missing above ?

> +	fi
> +
> +	# TEST 2 - check scsi_debug atomic_wr_max_length is the same as sysfs atomic_write_max_bytes
> +	if [ "$scsi_atomic_max_bytes" -le "$max_hw_bytes" ]
> +	then
> +		if [ "$scsi_atomic_max_bytes" = "$sysfs_atomic_max_bytes" ]
> +		then
> +			echo "TEST 2 - pass"
> +		else
> +			echo "TEST 2 - fail $scsi_atomic_max_bytes - $max_hw_bytes -" \
> +				"$sysfs_atomic_max_bytes"
> +		fi
> +	else
> +		if [ "$sysfs_atomic_max_bytes" = "$max_hw_bytes" ]
> +		then
> +			echo "TEST 2 - pass"
> +		else
> +			echo "TEST 2 - fail $scsi_atomic_max_bytes - $max_hw_bytes -" \
> +				"$sysfs_atomic_max_bytes"
> +		fi
> +	fi
> +

same comment here for test_desc

> +	# TEST 3 - check sysfs atomic_write_unit_max_bytes <= scsi_debug atomic_wr_max_length
> +	if (("$sysfs_atomic_unit_max_bytes" <= "$scsi_atomic_max_bytes"))
> +	then
> +		echo "TEST 3 - pass"
> +	else
> +		echo "TEST 3 - fail $sysfs_atomic_unit_max_bytes - $scsi_atomic_max_bytes"
> +	fi
> +

same comment here for test_desc

> +	# TEST 4 - check sysfs atomic_write_unit_min_bytes = scsi_debug atomic_wr_gran
> +	if [ "$sysfs_atomic_unit_min_bytes" = "$scsi_atomic_min_bytes" ]
> +	then
> +		echo "TEST 4 - pass"
> +	else
> +		echo "TEST 4 - fail $sysfs_atomic_unit_min_bytes - $scsi_atomic_min_bytes"
> +	fi
> +

same comment here for test_desc

> +	# TEST 5 - check statx stx_atomic_write_unit_min
> +	statx_atomic_min=$(run_xfs_io_xstat "$dev" "stat.atomic_write_unit_min")
> +	if [ "$statx_atomic_min" = "$scsi_atomic_min_bytes" ]
> +	then
> +		echo "TEST 5 - pass"
> +	else
> +		echo "TEST 5 - fail $statx_atomic_min - $scsi_atomic_min_bytes"
> +	fi
> +

same comment here for test_desc

> +	# TEST  6 - check statx stx_atomic_write_unit_max
> +	statx_atomic_max=$(run_xfs_io_xstat "$dev" "stat.atomic_write_unit_max")
> +	if [ "$statx_atomic_max" = "$sysfs_atomic_unit_max_bytes" ]
> +	then
> +		echo "TEST 6 - pass"
> +	else
> +		echo "TEST 6 - fail $statx_atomic_max - $sysfs_atomic_unit_max_bytes"
> +	fi
> +

same comment here for test_desc

> +	# TEST 7 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with no RWF_ATOMIC flag - pwritev2 should
> +	# be succesful.
> +	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$sysfs_atomic_unit_max_bytes")
> +	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
> +	then
> +		echo "TEST 7 - pass"
> +	else
> +		echo "TEST 7 - fail $bytes_written - $sysfs_atomic_unit_max_bytes"
> +	fi
> +
> +	# TEST 8 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with RWF_ATOMIC flag - pwritev2 should
> +	# be succesful.
> +	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$sysfs_atomic_unit_max_bytes")
> +	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
> +	then
> +		echo "TEST 8 - pass"
> +	else
> +		echo "TEST 8 - fail $bytes_written - $sysfs_atomic_unit_max_bytes"
> +	fi
> +

same comment here for test_desc

> +	# TEST 9 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 512 bytes with no RWF_ATOMIC flag - pwritev2
> +	# should be succesful.
> +	bytes_to_write=$(( "${sysfs_atomic_unit_max_bytes}" + "$sysfs_logical_block_size" ))
> +	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$bytes_to_write")
> +	if [ "$bytes_written" = "$bytes_to_write" ]
> +	then
> +		echo "TEST 9 - pass"
> +	else
> +		echo "TEST 9 - fail $bytes_written - $bytes_to_write"
> +	fi
> +

same comment here for test_desc

> +	# TEST 10 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 512 bytes with RWF_ATOMIC flag - pwritev2
> +	# should not be succesful.
> +	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$bytes_to_write")
> +	if [ "$bytes_written" = "" ]
> +	then
> +		echo "TEST 10 - pass"
> +	else
> +		echo "TEST 10 - fail $bytes_written - $bytes_to_write"
> +	fi
> +

same comment here for test_desc

> +	# TEST 11 - perform a pwritev2 with size of sysfs_atomic_unit_min_bytes with no RWF_ATOMIC flag - pwritev2 should
> +	# be succesful.
> +	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$sysfs_atomic_unit_min_bytes")
> +	if [ "$bytes_written" = "$sysfs_atomic_unit_min_bytes" ]
> +	then
> +		echo "TEST 11 - pass"
> +	else
> +		echo "TEST 11 - fail $bytes_written - $scsi_atomic_min_bytes"
> +	fi
> +

same comment here for test_desc

> +	# TEST 12 - perform a pwritev2 with size of sysfs_atomic_unit_min_bytes with RWF_ATOMIC flag - pwritev2 should
> +	# be succesful.
> +	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$sysfs_atomic_unit_min_bytes")
> +	if [ "$bytes_written" = "$sysfs_atomic_unit_min_bytes" ]
> +	then
> +		echo "TEST 12 - pass"
> +	else
> +		echo "TEST 12 - fail $bytes_written - $scsi_atomic_min_bytes"
> +	fi

same comment here for test_desc

> +	# TEST 13 - perform a pwritev2 with a size of sysfs_atomic_unit_min_bytes - 512 bytes with no
> +	# RWF_ATOMIC flag - pwritev2 should be succesful.
> +	# TEST 14 - perform a pwritev2 with a size of sysfs_atomic_unit_min_bytes - 512 bytes with
> +        # RWF_ATOMIC flag - pwritev2 should fail.
> +	bytes_to_write=$(( "${sysfs_atomic_unit_min_bytes}" - "${sysfs_logical_block_size}" ))
> +	if [ "$bytes_to_write" = 0 ]
> +	then
> +		# sysfs_atomic_unit_min_bytes is set to 1 logical block so these tests aren't needed.
> +		echo "TEST 13 - pass"
> +		echo "TEST 14 - pass"
> +	else
> +		bytes_written=$(run_xfs_io_pwritev2 "$dev" "$bytes_to_write")
> +		if [ "$bytes_written" = "$bytes_to_write" ]
> +		then
> +			echo "TEST 13 - pass"
> +		else
> +			echo "TEST 13 - fail $bytes_written - $bytes_to_write"
> +		fi
> +		bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$bytes_to_write")
> +		if [ "$bytes_written" = "" ]
> +		then
> +			echo "TEST 14 - pass"
> +		else
> +			echo "TEST 14 - fail $bytes_written - $bytes_to_write"
> +		fi
> +	fi
> +

same comment here for test_desc


-ck


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

* Re: [PATCH blktests 1/2] scsi/009: add atomic write tests
  2025-01-22  0:51   ` Chaitanya Kulkarni
@ 2025-01-24  5:36     ` Shinichiro Kawasaki
  0 siblings, 0 replies; 5+ messages in thread
From: Shinichiro Kawasaki @ 2025-01-24  5:36 UTC (permalink / raw)
  To: Chaitanya Kulkarni
  Cc: Alan Adamson, linux-block@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-nvme@lists.infradead.org

On Jan 22, 2025 / 00:51, Chaitanya Kulkarni wrote:
> On 1/21/25 14:25, Alan Adamson wrote:
> > Uses scsi_debug to test basic atomic write functionality. Testing
> > areas include:
> >
> > - Verify sysfs atomic write attributes are consistent with
> >    atomic write attributes advertised by scsi_debug.
> > - Verify the atomic write paramters of statx are correct using
> >    xfs_io.
> > - Perform a pwritev2() (with and without RWF_ATOMIC flag) using
> >    xfs_io:
> >      - maximum byte size (atomic_write_unit_max_bytes)
> >      - minimum byte size (atomic_write_unit_min_bytes)
> >      - a write larger than atomic_write_unit_max_bytes
> >      - a write smaller than atomic_write_unit_min_bytes
> >
> > Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
> 
> Thanks a lot for the testcase, this is really useful based on
> amount of work has been done on the kernel side.

Agreed, I also think these test cases are valuable :)

> 
> > ---
> >   common/xfs         |  49 +++++++++++
> >   tests/scsi/009     | 213 +++++++++++++++++++++++++++++++++++++++++++++
> >   tests/scsi/009.out |  18 ++++
> >   3 files changed, 280 insertions(+)
> >   create mode 100755 tests/scsi/009
> >   create mode 100644 tests/scsi/009.out
> >
> > diff --git a/common/xfs b/common/xfs
> > index 569770fecd53..284c6d7cdc40 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -6,6 +6,28 @@
> >   
> >   . common/shellcheck
> >   
> > +_have_xfs_io() {
> > +	if ! _have_program xfs_io; then
> > +		return 1
> > +	fi
> > +	return 0
> > +}
> > +
> > +# Check whether the version of xfs_io is greater than or equal to $1.$2.$3
> > +_have_xfs_io_ver() {
> > +	local d=$1 e=$2 f=$3
> > +
> > +	_have_xfs_io || return $?
> > +
> > +	IFS='.' read -r a b c < <(xfs_io -V | sed 's/xfs_io version *//')
> > +	if [ $((a * 65536 + b * 256 + c)) -lt $((d * 65536 + e * 256 + f)) ];
> 
> can we add some comments for above calculations ?

These are checking xfs_io command version, and I think the numbers and the logic
were copied form _have_kver().

Anyway, I wonder if we really need this helper function. In general, version
dependency is not the best approach and we should avoid them as much as we can.
Instead, I suggest to check output of "xfs_io -c help" command. The old version,
the output is as follows and the -A option is not printed.

$ xfs_io -c help | grep pwrite
pwrite [-i infile [-qdDwNOW] [-s skip]] [-b bs] [-S seed] [-FBR [-Z N]] [-V N] off len -- writes a number of bytes at a specified offset

With this approach, we can implement a helper function with name
_have_xfs_io_atomic_write() or something, and call it from requires() in
scsi/009 and nvme/059.

> 
> > +	then
> > +		SKIP_REASONS+=("xfs_io version too old")
> > +		return 1
> > +	fi
> > +	return 0
> > +}
> > +
> >   _have_xfs() {
> >   	_have_fs xfs && _have_program mkfs.xfs
> >   }
> > @@ -52,3 +74,30 @@ _xfs_run_fio_verify_io() {
> >   
> >   	return "${rc}"
> >   }
> > +
> > +run_xfs_io_pwritev2() {
> > +	local dev=$1
> > +	local bytes_to_write=$2
> > +	local bytes_written
> > +
> > +	bytes_written=$(xfs_io -d -C "pwrite -b ${bytes_to_write} -V 1 -D 0 ${bytes_to_write}" "$dev" | grep "wrote" | sed 's/\// /g' | awk '{ print $2 }')
> 
> same here little comment would be really useful
> 
> > +	echo "$bytes_written"
> > +}
> > +
> > +run_xfs_io_pwritev2_atomic() {
> > +	local dev=$1
> > +	local bytes_to_write=$2
> > +	local bytes_written
> > +
> > +	bytes_written=$(xfs_io -d -C "pwrite -b ${bytes_to_write} -V 1 -A -D 0 ${bytes_to_write}" "$dev" | grep "wrote" | sed 's/\// /g' | awk '{ print $2 }')
> 
> same here
> 
> > +	echo "$bytes_written"
> > +}
> > +
> > +run_xfs_io_xstat() {
> > +	local dev=$1
> > +	local field=$2
> > +	local statx_output
> > +
> > +	statx_output=$(xfs_io -c "statx -r -m 0x00010000" "$dev" | grep "$field" | awk '{ print $3 }')
> 
> same here
> 
> > +	echo "$statx_output"
> > +}
> > diff --git a/tests/scsi/009 b/tests/scsi/009
> > new file mode 100755
> > index 000000000000..f3ab00f61369
> > --- /dev/null
> > +++ b/tests/scsi/009
> > @@ -0,0 +1,213 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-3.0+
> > +# Copyright (C) 2025 Oracle and/or its affiliates
> > +#
> > +# Test SCSI Atomic Writes with scsi_debug
> > +
> > +. tests/scsi/rc
> > +. common/scsi_debug
> > +. common/xfs
> > +
> > +DESCRIPTION="test scsi atomic writes"
> > +QUICK=1
> > +
> > +requires() {
> > +	_have_driver scsi_debug
> > +	_have_kver 6 11

I suggest to remove this kernel version dependency also. Let me describe how to
do it below.

> > +	_have_xfs_io_ver 6 12 0
> > +}
> > +
> > +test() {
> > +	local dev
> > +	local scsi_debug_atomic_wr_max_length
> > +	local scsi_debug_atomic_wr_gran
> > +	local scsi_atomic_max_bytes
> > +	local scsi_atomic_min_bytes
> > +	local sysfs_max_hw_sectors_kb
> > +	local max_hw_bytes
> > +	local sysfs_logical_block_size
> > +	local sysfs_atomic_max_bytes
> > +	local sysfs_atomic_unit_max_bytes
> > +	local sysfs_atomic_unit_min_bytes
> > +	local statx_atomic_min
> > +	local statx_atomic_max
> > +	local bytes_to_write
> > +	local bytes_written
> > +
> > +	echo "Running ${TEST_NAME}"
> > +
> > +	local scsi_debug_params=(
> > +		delay=0
> > +		atomic_wr=1
> > +	)
> > +	_configure_scsi_debug "${scsi_debug_params[@]}"
> > +	dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
> > +	sysfs_logical_block_size=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/logical_block_size)
> 
> you can also use the local variable for following path to trim down the
> cat operations where :-
> 
> /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue
> 
> makes the code easy to read ...
> 
> > +	sysfs_max_hw_sectors_kb=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/max_hw_sectors_kb)
> > +	max_hw_bytes=$(( "$sysfs_max_hw_sectors_kb" * 1024 ))
> > +	sysfs_atomic_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_max_bytes)
> > +	sysfs_atomic_unit_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_max_bytes)
> > +	sysfs_atomic_unit_min_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_min_bytes)

I agree that these lines are lengthy.

Taking this chance, let me suggest to use fallback_device() and
cleanup_fallback_device(). (They are not well documented, but are used in some
test cases like zbd/001 or block/007). When a test case with test_device()
implements these hooks, blktests runs the test case even when TEST_DEVS is
empty. It calls fallback_device() to set up the test target TEST_DEV then run
test_device(). After the test completion, it calls cleanup_fallback_device() to
clean up TEST_DEV. For this test case, fallback_device() can call
_configure_scsi_debug with atmoic_wr=1 option. With this approach, we can refer
to TEST_DEV and TEST_DEV_SYSFS in test_device().

This approach has following benefits:

- Using TEST_DEV_SYSFS, the sysfs attribute reference codes will simpler and
  similar as nvme/059 in the next patch.
- We can call "_require_test_dev_sysfs queue/atomic_write_max_bytes" in
  device_requires to check that the kernel supports amotic writes. This will
  avoid the kernel version dependency.
- If users have real scsi devices with atomic write support, the users can
  specify the devices in TEST_DEVS and run this test case with them.
  (We need to skip the test if the devices do not support atomic writes,
   by checking the queue/atomic_write_max_bytes value in device_requires())

For your reference, here I share the change needed for this approach. It is
untested. I hope it is enough to convey my idea.

diff --git a/tests/scsi/009 b/tests/scsi/009
index f3ab00f..686cc8a 100755
--- a/tests/scsi/009
+++ b/tests/scsi/009
@@ -13,12 +13,34 @@ QUICK=1
 
 requires() {
 	_have_driver scsi_debug
-	_have_kver 6 11
-	_have_xfs_io_ver 6 12 0
 }
 
-test() {
-	local dev
+device_requires() {
+	_require_test_dev_sysfs queue/atomic_write_max_bytes
+	if (( $(< ${TEST_DEV_SYSFS}/queue/atomic_write_max_bytes) == 0 )); then
+		SKIP_REASONS+=("${TEST_DEV} does not support atomic write")
+		return 1
+	fi
+}
+
+fallback_device() {
+	local scsi_debug_params=(
+		delay=0
+		atomic_wr=1
+	)
+	if ! _configure_scsi_debug "${scsi_debug_params[@]}"; then
+		return 1
+	fi
+	echo "/dev/${SCSI_DEBUG_DEVICES[0]}"
+}
+
+cleanup_fallback_device() {
+	_exit_scsi_debug
+
+}
+
+test_device() {
 	local scsi_debug_atomic_wr_max_length
 	local scsi_debug_atomic_wr_gran
 	local scsi_atomic_max_bytes
@@ -36,20 +58,14 @@ test() {
 
 	echo "Running ${TEST_NAME}"
 
-	local scsi_debug_params=(
-		delay=0
-		atomic_wr=1
-	)
-	_configure_scsi_debug "${scsi_debug_params[@]}"
-	dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
-	sysfs_logical_block_size=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/logical_block_size)
-	sysfs_max_hw_sectors_kb=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/max_hw_sectors_kb)
+	sysfs_logical_block_size=$(< "${TEST_DEV_SYSFS}"/queue/logical_block_size)
+	sysfs_max_hw_sectors_kb=$(< "${TEST_DEV_SYSFS}"/queue/max_hw_sectors_kb)
 	max_hw_bytes=$(( "$sysfs_max_hw_sectors_kb" * 1024 ))
-	sysfs_atomic_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_max_bytes)
-	sysfs_atomic_unit_max_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_max_bytes)
-	sysfs_atomic_unit_min_bytes=$(cat /sys/block/"${SCSI_DEBUG_DEVICES[0]}"/queue/atomic_write_unit_min_bytes)
-	scsi_debug_atomic_wr_max_length=$(cat /sys/module/scsi_debug/parameters/atomic_wr_max_length)
-	scsi_debug_atomic_wr_gran=$(cat /sys/module/scsi_debug/parameters/atomic_wr_gran)
+	sysfs_atomic_max_bytes=$(< "${TEST_DEV_SYSFS}"/queue/atomic_write_max_bytes)
+	sysfs_atomic_unit_max_bytes=$(< "${TEST_DEV_SYSFS}"/queue/atomic_write_unit_max_bytes)
+	sysfs_atomic_unit_min_bytes=$(< "${TEST_DEV_SYSFS}"/queue/atomic_write_unit_min_bytes)
+	scsi_debug_atomic_wr_max_length=$(< /sys/module/scsi_debug/parameters/atomic_wr_max_length)
+	scsi_debug_atomic_wr_gran=$(< /sys/module/scsi_debug/parameters/atomic_wr_gran)
 	scsi_atomic_max_bytes=$(( "$scsi_debug_atomic_wr_max_length" * "$sysfs_logical_block_size" ))
 	scsi_atomic_min_bytes=$(( "$scsi_debug_atomic_wr_gran" * "$sysfs_logical_block_size" ))
 
@@ -102,7 +118,7 @@ test() {
 	fi
 
 	# TEST 5 - check statx stx_atomic_write_unit_min
-	statx_atomic_min=$(run_xfs_io_xstat "$dev" "stat.atomic_write_unit_min")
+	statx_atomic_min=$(run_xfs_io_xstat "$TEST_DEV" "stat.atomic_write_unit_min")
 	if [ "$statx_atomic_min" = "$scsi_atomic_min_bytes" ]
 	then
 		echo "TEST 5 - pass"
@@ -111,7 +127,7 @@ test() {
 	fi
 
 	# TEST  6 - check statx stx_atomic_write_unit_max
-	statx_atomic_max=$(run_xfs_io_xstat "$dev" "stat.atomic_write_unit_max")
+	statx_atomic_max=$(run_xfs_io_xstat "$TEST_DEV" "stat.atomic_write_unit_max")
 	if [ "$statx_atomic_max" = "$sysfs_atomic_unit_max_bytes" ]
 	then
 		echo "TEST 6 - pass"
@@ -121,7 +137,7 @@ test() {
 
 	# TEST 7 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with no RWF_ATOMIC flag - pwritev2 should
 	# be succesful.
-	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$sysfs_atomic_unit_max_bytes")
+	bytes_written=$(run_xfs_io_pwritev2 "$TEST_DEV" "$sysfs_atomic_unit_max_bytes")
 	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
 	then
 		echo "TEST 7 - pass"
@@ -131,7 +147,7 @@ test() {
 
 	# TEST 8 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes with RWF_ATOMIC flag - pwritev2 should
 	# be succesful.
-	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$sysfs_atomic_unit_max_bytes")
+	bytes_written=$(run_xfs_io_pwritev2_atomic "$TEST_DEV" "$sysfs_atomic_unit_max_bytes")
 	if [ "$bytes_written" = "$sysfs_atomic_unit_max_bytes" ]
 	then
 		echo "TEST 8 - pass"
@@ -142,7 +158,7 @@ test() {
 	# TEST 9 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 512 bytes with no RWF_ATOMIC flag - pwritev2
 	# should be succesful.
 	bytes_to_write=$(( "${sysfs_atomic_unit_max_bytes}" + "$sysfs_logical_block_size" ))
-	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$bytes_to_write")
+	bytes_written=$(run_xfs_io_pwritev2 "$TEST_DEV" "$bytes_to_write")
 	if [ "$bytes_written" = "$bytes_to_write" ]
 	then
 		echo "TEST 9 - pass"
@@ -152,7 +168,7 @@ test() {
 
 	# TEST 10 - perform a pwritev2 with size of sysfs_atomic_unit_max_bytes + 512 bytes with RWF_ATOMIC flag - pwritev2
 	# should not be succesful.
-	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$bytes_to_write")
+	bytes_written=$(run_xfs_io_pwritev2_atomic "$TEST_DEV" "$bytes_to_write")
 	if [ "$bytes_written" = "" ]
 	then
 		echo "TEST 10 - pass"
@@ -162,7 +178,7 @@ test() {
 
 	# TEST 11 - perform a pwritev2 with size of sysfs_atomic_unit_min_bytes with no RWF_ATOMIC flag - pwritev2 should
 	# be succesful.
-	bytes_written=$(run_xfs_io_pwritev2 "$dev" "$sysfs_atomic_unit_min_bytes")
+	bytes_written=$(run_xfs_io_pwritev2 "$TEST_DEV" "$sysfs_atomic_unit_min_bytes")
 	if [ "$bytes_written" = "$sysfs_atomic_unit_min_bytes" ]
 	then
 		echo "TEST 11 - pass"
@@ -172,7 +188,7 @@ test() {
 
 	# TEST 12 - perform a pwritev2 with size of sysfs_atomic_unit_min_bytes with RWF_ATOMIC flag - pwritev2 should
 	# be succesful.
-	bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$sysfs_atomic_unit_min_bytes")
+	bytes_written=$(run_xfs_io_pwritev2_atomic "$TEST_DEV" "$sysfs_atomic_unit_min_bytes")
 	if [ "$bytes_written" = "$sysfs_atomic_unit_min_bytes" ]
 	then
 		echo "TEST 12 - pass"
@@ -191,14 +207,14 @@ test() {
 		echo "TEST 13 - pass"
 		echo "TEST 14 - pass"
 	else
-		bytes_written=$(run_xfs_io_pwritev2 "$dev" "$bytes_to_write")
+		bytes_written=$(run_xfs_io_pwritev2 "$TEST_DEV" "$bytes_to_write")
 		if [ "$bytes_written" = "$bytes_to_write" ]
 		then
 			echo "TEST 13 - pass"
 		else
 			echo "TEST 13 - fail $bytes_written - $bytes_to_write"
 		fi
-		bytes_written=$(run_xfs_io_pwritev2_atomic "$dev" "$bytes_to_write")
+		bytes_written=$(run_xfs_io_pwritev2_atomic "$TEST_DEV" "$bytes_to_write")
 		if [ "$bytes_written" = "" ]
 		then
 			echo "TEST 14 - pass"
@@ -207,7 +223,5 @@ test() {
 		fi
 	fi
 
-	_exit_scsi_debug
-
 	echo "Test complete"
 }

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

end of thread, other threads:[~2025-01-24  5:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-21 22:25 [PATCH blktests 0/2] Add atomic write tests for scsi and nvme Alan Adamson
2025-01-21 22:25 ` [PATCH blktests 1/2] scsi/009: add atomic write tests Alan Adamson
2025-01-22  0:51   ` Chaitanya Kulkarni
2025-01-24  5:36     ` Shinichiro Kawasaki
2025-01-21 22:25 ` [PATCH blktests 2/2] nvme/059: " Alan Adamson

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