linux-ext4 - Re: [PATCH blktests 2/3] dm/003: 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: <t4vmmsupkbffrp3p33okbdjtf6il2ahp5omp2s5fvuxkngipeo@4thxzp4zlcse>
Date: Thu, 3 Apr 2025 07:43:42 +0000
From: Shinichiro Kawasaki <shinichiro.kawasaki@....com>
To: Zhang Yi <yi.zhang@...weicloud.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 2/3] dm/003: add unmap write zeroes tests

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