diff mbox series

[blktests,1/3] scsi/010: add unmap write zeroes tests

Message ID 20250318072835.3508696-2-yi.zhang@huaweicloud.com
State New
Headers show
Series blktest: add unmap write zeroes tests | expand

Commit Message

Zhang Yi March 18, 2025, 7:28 a.m. UTC 
From: Zhang Yi <yi.zhang@huawei.com>

Test block device unmap write zeroes sysfs interface with various SCSI
debug devices. The /sys/block/<disk>/queue/write_zeroes_unmap interface
should return 1 if the SCSI device enable the WRITE SAME command with
unmap functionality, and it should return 0 otherwise.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 tests/scsi/010     | 56 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/scsi/010.out |  2 ++
 2 files changed, 58 insertions(+)
 create mode 100755 tests/scsi/010
 create mode 100644 tests/scsi/010.out

Comments

Shinichiro Kawasaki April 3, 2025, 7:26 a.m. UTC | #1 
Hello Zhang, thank you for the patches.

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 various SCSI
> debug devices. The /sys/block/<disk>/queue/write_zeroes_unmap interface
> should return 1 if the SCSI device enable the WRITE SAME command with
> unmap functionality, and it should return 0 otherwise.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  tests/scsi/010     | 56 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/scsi/010.out |  2 ++
>  2 files changed, 58 insertions(+)
>  create mode 100755 tests/scsi/010
>  create mode 100644 tests/scsi/010.out
> 
> diff --git a/tests/scsi/010 b/tests/scsi/010
> new file mode 100755
> index 0000000..27a672c
> --- /dev/null
> +++ b/tests/scsi/010
> @@ -0,0 +1,56 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2025 Huawei.
> +#
> +# Test block device unmap write zeroes sysfs interface with various scsi
> +# devices.
> +
> +. tests/scsi/rc
> +. common/scsi_debug
> +
> +DESCRIPTION="test unmap write zeroes sysfs interface with scsi devices"
> +QUICK=1
> +
> +requires() {
> +	_have_scsi_debug
> +}
> +
> +device_requries() {
> +	_require_test_dev_sysfs queue/write_zeroes_unmap
> +}

The device_requries() hook does not work for test cases which implement test().
It is rather dirty, but I think we need to delay the check for
write_zeroes_unmap sysfs attribute availability until test() gets called.
See below for my idea.

> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	# disable WRITE SAME with unmap
> +	if ! _configure_scsi_debug lbprz=0; then
> +		return 1
> +	fi

I suggest to check queue/write_zeroes_unmap here. If it's not available, set
SKIP_REASONS and return like this (totally untested):

	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

> +	umap="$(cat "/sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap")"
> +	if [[ $umap -ne 0 ]]; then
> +		echo "Test disable WRITE SAME with unmap failed."
> +	fi
> +	_exit_scsi_debug
> +
> +	# enable WRITE SAME(16) with unmap
> +	if ! _configure_scsi_debug lbprz=1 lbpws=1; then
> +		return 1
> +	fi
> +	umap="$(cat "/sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap")"
> +	if [[ $umap -ne 1 ]]; then
> +		echo "Test enable WRITE SAME(16) with unmap failed."
> +	fi
> +	_exit_scsi_debug
> +
> +	# enable WRITE SAME(10) with unmap
> +	if ! _configure_scsi_debug lbprz=1 lbpws10=1; then
> +		return 1
> +	fi
> +	umap="$(cat "/sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap")"
> +	if [[ $umap -ne 1 ]]; then
> +		echo "Test enable WRITE SAME(10) with unmap failed."
> +	fi
> +	_exit_scsi_debug
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/scsi/010.out b/tests/scsi/010.out
> new file mode 100644
> index 0000000..6581d5e
> --- /dev/null
> +++ b/tests/scsi/010.out
> @@ -0,0 +1,2 @@
> +Running scsi/010
> +Test complete
> -- 
> 2.46.1
>
Zhang Yi April 28, 2025, 3:32 a.m. UTC | #2 
Hello Shinichiro！

I apologize for the significant delay, and I greatly appreciate your
review and suggestions.

On 2025/4/3 15:26, Shinichiro Kawasaki wrote:
> Hello Zhang, thank you for the patches.
> 
> 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 various SCSI
>> debug devices. The /sys/block/<disk>/queue/write_zeroes_unmap interface
>> should return 1 if the SCSI device enable the WRITE SAME command with
>> unmap functionality, and it should return 0 otherwise.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> ---
>>  tests/scsi/010     | 56 ++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/scsi/010.out |  2 ++
>>  2 files changed, 58 insertions(+)
>>  create mode 100755 tests/scsi/010
>>  create mode 100644 tests/scsi/010.out
>>
>> diff --git a/tests/scsi/010 b/tests/scsi/010
>> new file mode 100755
>> index 0000000..27a672c
>> --- /dev/null
>> +++ b/tests/scsi/010
>> @@ -0,0 +1,56 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2025 Huawei.
>> +#
>> +# Test block device unmap write zeroes sysfs interface with various scsi
>> +# devices.
>> +
>> +. tests/scsi/rc
>> +. common/scsi_debug
>> +
>> +DESCRIPTION="test unmap write zeroes sysfs interface with scsi devices"
>> +QUICK=1
>> +
>> +requires() {
>> +	_have_scsi_debug
>> +}
>> +
>> +device_requries() {
>> +	_require_test_dev_sysfs queue/write_zeroes_unmap
>> +}
> 
> The device_requries() hook does not work for test cases which implement test().
> It is rather dirty, but I think we need to delay the check for
> write_zeroes_unmap sysfs attribute availability until test() gets called.
> See below for my idea.
> 

Indeed, I completely missed that.

>> +
>> +test() {
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	# disable WRITE SAME with unmap
>> +	if ! _configure_scsi_debug lbprz=0; then
>> +		return 1
>> +	fi
> 
> I suggest to check queue/write_zeroes_unmap here. If it's not available, set
> SKIP_REASONS and return like this (totally untested):
> 
> 	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
> 

Yeah, I agree with you. For now, there is no helper available for
checking the sysfs interface of the SCSI debugging device.

I will add a new helper setup_test_device() in this test as the
following two patches do, and put this check into that helper.

Thanks,
Yi.
diff mbox series

Patch

diff --git a/tests/scsi/010 b/tests/scsi/010
new file mode 100755
index 0000000..27a672c
--- /dev/null
+++ b/tests/scsi/010
@@ -0,0 +1,56 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2025 Huawei.
+#
+# Test block device unmap write zeroes sysfs interface with various scsi
+# devices.
+
+. tests/scsi/rc
+. common/scsi_debug
+
+DESCRIPTION="test unmap write zeroes sysfs interface with scsi devices"
+QUICK=1
+
+requires() {
+	_have_scsi_debug
+}
+
+device_requries() {
+	_require_test_dev_sysfs queue/write_zeroes_unmap
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	# disable WRITE SAME with unmap
+	if ! _configure_scsi_debug lbprz=0; then
+		return 1
+	fi
+	umap="$(cat "/sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap")"
+	if [[ $umap -ne 0 ]]; then
+		echo "Test disable WRITE SAME with unmap failed."
+	fi
+	_exit_scsi_debug
+
+	# enable WRITE SAME(16) with unmap
+	if ! _configure_scsi_debug lbprz=1 lbpws=1; then
+		return 1
+	fi
+	umap="$(cat "/sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap")"
+	if [[ $umap -ne 1 ]]; then
+		echo "Test enable WRITE SAME(16) with unmap failed."
+	fi
+	_exit_scsi_debug
+
+	# enable WRITE SAME(10) with unmap
+	if ! _configure_scsi_debug lbprz=1 lbpws10=1; then
+		return 1
+	fi
+	umap="$(cat "/sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap")"
+	if [[ $umap -ne 1 ]]; then
+		echo "Test enable WRITE SAME(10) with unmap failed."
+	fi
+	_exit_scsi_debug
+
+	echo "Test complete"
+}
diff --git a/tests/scsi/010.out b/tests/scsi/010.out
new file mode 100644
index 0000000..6581d5e
--- /dev/null
+++ b/tests/scsi/010.out
@@ -0,0 +1,2 @@ 
+Running scsi/010
+Test complete