[PATCH blktests] md/002: add atomic write tests for md/stacked devices

[Shinichiro Kawasaki]

On Jun 05, 2025 / 15:57, Alan Adamson wrote:
> Add a new test (md/002) to verify atomic write support for MD devices
> (RAID 0, 1, and 10) stacked on top of SCSI devices using scsi_debug with
> atomic write emulation enabled.
> 
> This test validates that atomic write sysfs attributes are correctly
> propagated through MD layers, and that pwritev2() with RWF_ATOMIC
> behaves as expected on these devices.
> 
> Specifically, the test checks:
>     - That atomic write attributes in /sys/block/.../queue are consistent
>       between MD and underlying SCSI devices
>     - That atomic write limits are respected in user-space via xfs_io
>     - That statx reports accurate atomic_write_unit_{min,max} values
>     - That invalid writes (too small or too large) fail as expected
>     - That chunk size affects max atomic write limits (for RAID 0/10)
> 
> Signed-off-by: Alan Adamson <alan.adamson at oracle.com>

Hello Allan, thanks for the patch. I ran the new test case and it looks working
good. Please find my comments in line.

> ---
>  tests/md/002     | 245 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/md/002.out |  43 +++++++++
>  2 files changed, 288 insertions(+)
>  create mode 100755 tests/md/002
>  create mode 100644 tests/md/002.out
> 
> diff --git a/tests/md/002 b/tests/md/002
> new file mode 100755
> index 000000000000..4b71ebf7d496
> --- /dev/null
> +++ b/tests/md/002
> @@ -0,0 +1,245 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2025 Oracle and/or its affiliates
> +#
> +# Test SCSI Atomic Writes with MD devices
> +
> +. tests/scsi/rc

I think you meant "tests/md/rc", didn't you?

> +. common/scsi_debug
> +. common/xfs
> +
> +DESCRIPTION="test md atomic writes"
> +QUICK=1
> +
> +requires() {
> +	_have_kver 6 14 0

I wanted to confirm that the kernel version check is the only way to
confirm the kernel dependency of this test case. Is there any other
way to check it from userland, such as sysfs attributes?

> +	group_requires

I don't think the line above is necessary.

> +	_have_program mdadm
> +	_have_driver scsi_debug
> +	_have_xfs_io_atomic_write
> +}
> +
> +test() {
> +	local scsi_debug_atomic_wr_max_length
> +	local scsi_debug_atomic_wr_gran
> +	local scsi_sysfs_atomic_max_bytes
> +	local scsi_sysfs_atomic_unit_max_bytes
> +	local scsi_sysfs_atomic_unit_min_bytes
> +	local md_atomic_max_bytes
> +	local md_atomic_min_bytes
> +	local md_sysfs_max_hw_sectors_kb
> +	local md_max_hw_bytes
> +	local md_chunk_size
> +	local md_sysfs_logical_block_size
> +	local md_sysfs_atomic_max_bytes
> +	local md_sysfs_atomic_unit_max_bytes
> +	local md_sysfs_atomic_unit_min_bytes
> +	local bytes_to_write
> +	local bytes_written
> +	local test_desc
> +	local scsi_0
> +	local scsi_1
> +	local scsi_2
> +	local scsi_3
> +	local scsi_dev_sysfs
> +	local md_dev
> +	local md_dev_sysfs
> +	local scsi_debug_params=(
> +		delay=0
> +		atomic_wr=1
> +		num_tgts=1
> +		add_host=4
> +		per_host_store=true
> +	)
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	if ! _configure_scsi_debug "${scsi_debug_params[@]}"; then
> +                return 1
> +                fi

Nit: Whitespaces are used for indenet in the above two lines.

> +
> +	scsi_0="${SCSI_DEBUG_DEVICES[0]}"
> +	scsi_1="${SCSI_DEBUG_DEVICES[1]}"
> +	scsi_2="${SCSI_DEBUG_DEVICES[2]}"
> +	scsi_3="${SCSI_DEBUG_DEVICES[3]}"
> +
> +	scsi_dev_sysfs="/sys/block/${scsi_0}"
> +	scsi_sysfs_atomic_max_bytes=$(< "${scsi_dev_sysfs}"/queue/atomic_write_max_bytes)
> +	scsi_sysfs_atomic_unit_max_bytes=$(< "${scsi_dev_sysfs}"/queue/atomic_write_unit_max_bytes)
> +	scsi_sysfs_atomic_unit_min_bytes=$(< "${scsi_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)
> +
> +	for raid_level in 0 1 10; do

Nit: this for loop block is rather large and has deep nest and long lines, then
it would be good to factor out it into another function (Bash users dynamic
scoping then the local variables defined in test() will be usable in the new
function()). I'm ok with current implementation, just a mere suggestion.

Thanks!