linux-ext4 - Re: [PATCH blktests 1/3] scsi/010: add unmap write zeroes tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6db8d3e6-44ce-4b2b-b496-ec0104aee997@huaweicloud.com>
Date: Mon, 28 Apr 2025 11:32:45 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Shinichiro Kawasaki <shinichiro.kawasaki@....com>
Cc: "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
 "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
 "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
 "dm-devel@...ts.linux.dev" <dm-devel@...ts.linux.dev>,
 "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
 "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
 "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 hch <hch@....de>, "tytso@....edu" <tytso@....edu>,
 "djwong@...nel.org" <djwong@...nel.org>,
 "john.g.garry@...cle.com" <john.g.garry@...cle.com>,
 "bmarzins@...hat.com" <bmarzins@...hat.com>,
 "chaitanyak@...dia.com" <chaitanyak@...dia.com>,
 "yi.zhang@...wei.com" <yi.zhang@...wei.com>,
 "chengzhihao1@...wei.com" <chengzhihao1@...wei.com>,
 "yukuai3@...wei.com" <yukuai3@...wei.com>,
 "yangerkun@...wei.com" <yangerkun@...wei.com>
Subject: Re: [PATCH blktests 1/3] scsi/010: add unmap write zeroes tests

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@...wei.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@...wei.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.