[PATCH blktests 0/2] blktests: add ability for multiple dev sysfs checks

[PATCH blktests 0/2] blktests: add ability for multiple dev sysfs checks

[John Pittman]

This patchset adds the ability to loop within _require_test_dev_sysfs()
and check multiple sysfs values rather than only one. In older kernels,
as we've seen in recent testing, its common for sysfs values to be
missing, so it's good to check these files prior to testing.  We also
use the new format in block/042 to resolve recently seen failures.

John Pittman (2):
  common/rc: support multiple arguments for _require_test_dev_sysfs()
  block/042: check sysfs values prior to running

 common/rc       | 13 ++++++++-----
 tests/block/042 |  4 +++-
 2 files changed, 11 insertions(+), 6 deletions(-)

-- 
2.51.1

[PATCH blktests 1/2] common/rc: support multiple arguments for _require_test_dev_sysfs()

[John Pittman]

In some cases we may need to check multiple sysfs values for tests.
If this happens, create the ability to pass in multiple arguments to
_require_test_dev_sysfs() instead of having to call the function
multiple times.

Signed-off-by: John Pittman <jpittman@redhat.com>
---
 common/rc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/common/rc b/common/rc
index b76a856..e4b5196 100644
--- a/common/rc
+++ b/common/rc
@@ -326,11 +326,14 @@ _test_dev_is_rotational() {
 }
 
 _require_test_dev_sysfs() {
-	if [[ ! -e "${TEST_DEV_SYSFS}/$1" ]]; then
-		SKIP_REASONS+=("${TEST_DEV} does not have sysfs attribute $1")
-		return 1
-	fi
-	return 0
+	local ret=0
+	for attr in "$@"; do
+		if [[ ! -e "${TEST_DEV_SYSFS}/$attr" ]]; then
+			SKIP_REASONS+=("${TEST_DEV} does not have sysfs attribute $attr")
+			ret=1
+		fi
+	done
+	return $ret
 }
 
 _require_test_dev_is_rotational() {
-- 
2.51.1

[PATCH blktests 2/2] block/042: check sysfs values prior to running

[John Pittman]

In testing some older kernels recently, block/042 has failed due
to dma_alignment and virt_boundary_mask not being present.

   Running block/042
   +cat: '.../queue/dma_alignment': No such file or directory
   +cat: '.../queue/virt_boundary_mask': No such file or directory
   +dio-offsets: test_dma_aligned: failed to write buf: Invalid argument

To ensure we skip if this is the case, check all sysfs values prior
to run.

Signed-off-by: John Pittman <jpittman@redhat.com>
---
 tests/block/042 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/block/042 b/tests/block/042
index 28ac4a2..bbf13fd 100644
--- a/tests/block/042
+++ b/tests/block/042
@@ -11,7 +11,9 @@ DESCRIPTION="Test unusual direct-io offsets"
 QUICK=1
 
 device_requires() {
-        _require_test_dev_sysfs
+        _require_test_dev_sysfs "" "queue/max_segments" "queue/dma_alignment" \
+		"queue/virt_boundary_mask" "queue/logical_block_size" \
+		"queue/max_sectors_kb"
 }
 
 test_device() {
-- 
2.51.1

Re: [PATCH blktests 2/2] block/042: check sysfs values prior to running

[Shinichiro Kawasaki]

On Jan 14, 2026 / 16:08, John Pittman wrote:
> In testing some older kernels recently, block/042 has failed due
> to dma_alignment and virt_boundary_mask not being present.
> 
>    Running block/042
>    +cat: '.../queue/dma_alignment': No such file or directory
>    +cat: '.../queue/virt_boundary_mask': No such file or directory
>    +dio-offsets: test_dma_aligned: failed to write buf: Invalid argument
> 
> To ensure we skip if this is the case, check all sysfs values prior
> to run.
> 
> Signed-off-by: John Pittman <jpittman@redhat.com>

John, thank you for this series.

> ---
>  tests/block/042 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/block/042 b/tests/block/042
> index 28ac4a2..bbf13fd 100644
> --- a/tests/block/042
> +++ b/tests/block/042
> @@ -11,7 +11,9 @@ DESCRIPTION="Test unusual direct-io offsets"
>  QUICK=1
>  
>  device_requires() {
> -        _require_test_dev_sysfs
> +        _require_test_dev_sysfs "" "queue/max_segments" "queue/dma_alignment" \

Do we need the first argument "" ?  It looks useless.
Nit: spaces are used for indent. I suggest to replace them with a space.

> +		"queue/virt_boundary_mask" "queue/logical_block_size" \
> +		"queue/max_sectors_kb"
>  }
>  
>  test_device() {
> -- 
> 2.51.1
> 

Re: [PATCH blktests 1/2] common/rc: support multiple arguments for _require_test_dev_sysfs()

[Shinichiro Kawasaki]

On Jan 14, 2026 / 16:08, John Pittman wrote:
> In some cases we may need to check multiple sysfs values for tests.
> If this happens, create the ability to pass in multiple arguments to
> _require_test_dev_sysfs() instead of having to call the function
> multiple times.
> 
> Signed-off-by: John Pittman <jpittman@redhat.com>
> ---
>  common/rc | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index b76a856..e4b5196 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -326,11 +326,14 @@ _test_dev_is_rotational() {
>  }
>  
>  _require_test_dev_sysfs() {
> -	if [[ ! -e "${TEST_DEV_SYSFS}/$1" ]]; then
> -		SKIP_REASONS+=("${TEST_DEV} does not have sysfs attribute $1")
> -		return 1
> -	fi
> -	return 0
> +	local ret=0

Nit: it's a bit safer to declare "attr" as a local variable here.

> +	for attr in "$@"; do
> +		if [[ ! -e "${TEST_DEV_SYSFS}/$attr" ]]; then
> +			SKIP_REASONS+=("${TEST_DEV} does not have sysfs attribute $attr")
> +			ret=1
> +		fi
> +	done
> +	return $ret
>  }
>  
>  _require_test_dev_is_rotational() {
> -- 
> 2.51.1
> 

Re: [PATCH blktests 2/2] block/042: check sysfs values prior to running

[John Pittman]

Hi Shinichiro!  Thanks so much for the review.  I'm correcting the
attr and the unneeded argument preparing the V2, but I'm confused
about the below request:

>> Nit: spaces are used for indent. I suggest to replace them with a space.

Using vim to show the tabs (using commands ":set list" then "set
listchars=tab:>-,trail:~"), I see in block/042 that I am indeed using
tabs:

device_requires() {
        _require_test_dev_sysfs "queue/max_segments" "queue/dma_alignment" \
>------->-------"queue/virt_boundary_mask" "queue/logical_block_size" \
>------->-------"queue/max_sectors_kb"
}

And lower down in block/042, I see that in a past patch you used a
single tab then spaces:

>-------if ! src/dio-offsets "${TEST_DEV}" "$sys_max_segments" \
>-------     "$sys_max_sectors_kb" "$sys_dma_alignment" \
>-------     "$sys_virt_boundary_mask" "$sys_logical_block_size"; then
>------->-------echo "src/dio-offsets failed"
>-------fi

However, when I check other test files, it seems like people have been
using only tabs for indent rather than spaces.  For example in
block/010:

run_fio_job() {
>-------_fio_perf --size=8g --bs=4k --direct=1 --ioengine=libaio --iodepth=32 \
>------->---------group_reporting=1 --rw=randread --norandommap --name=nullb0 \
>------->---------filename=/dev/nullb0 --name=nullb1 --filename=/dev/nullb1 \
>------->---------name=nullb2 --filename=/dev/nullb2 --name=nullb3 \
>------->---------filename=/dev/nullb3 --name=nullb4 --filename=/dev/nullb4 \
SNIP....

So, I'm unsure what to do.  Do you want me to use one tab then spaces
as you did lower in block/042?  Or stick with only tabs as others seem
to have been doing?  Or only spaces?  Sorry for the confusion and
thanks for any help!

John


On Fri, Jan 16, 2026 at 8:03 AM Shinichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> On Jan 14, 2026 / 16:08, John Pittman wrote:
> > In testing some older kernels recently, block/042 has failed due
> > to dma_alignment and virt_boundary_mask not being present.
> >
> >    Running block/042
> >    +cat: '.../queue/dma_alignment': No such file or directory
> >    +cat: '.../queue/virt_boundary_mask': No such file or directory
> >    +dio-offsets: test_dma_aligned: failed to write buf: Invalid argument
> >
> > To ensure we skip if this is the case, check all sysfs values prior
> > to run.
> >
> > Signed-off-by: John Pittman <jpittman@redhat.com>
>
> John, thank you for this series.
>
> > ---
> >  tests/block/042 | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/block/042 b/tests/block/042
> > index 28ac4a2..bbf13fd 100644
> > --- a/tests/block/042
> > +++ b/tests/block/042
> > @@ -11,7 +11,9 @@ DESCRIPTION="Test unusual direct-io offsets"
> >  QUICK=1
> >
> >  device_requires() {
> > -        _require_test_dev_sysfs
> > +        _require_test_dev_sysfs "" "queue/max_segments" "queue/dma_alignment" \
>
> Do we need the first argument "" ?  It looks useless.
> Nit: spaces are used for indent. I suggest to replace them with a space.
>
> > +             "queue/virt_boundary_mask" "queue/logical_block_size" \
> > +             "queue/max_sectors_kb"
> >  }
> >
> >  test_device() {
> > --
> > 2.51.1
> >
>

Re: [PATCH blktests 2/2] block/042: check sysfs values prior to running

[Shinichiro Kawasaki]

On Jan 20, 2026 / 11:16, John Pittman wrote:
> Hi Shinichiro!  Thanks so much for the review.  I'm correcting the
> attr and the unneeded argument preparing the V2, but I'm confused
> about the below request:
> 
> >> Nit: spaces are used for indent. I suggest to replace them with a space.
> 
> Using vim to show the tabs (using commands ":set list" then "set
> listchars=tab:>-,trail:~"), I see in block/042 that I am indeed using
> tabs:
> 
> device_requires() {
>         _require_test_dev_sysfs "queue/max_segments" "queue/dma_alignment" \
> >------->-------"queue/virt_boundary_mask" "queue/logical_block_size" \
> >------->-------"queue/max_sectors_kb"
> }

Hi John, yes, that point needs some more clarification. My nit comment mentioned
about the line "_require_test_dev_sysfs ...". I did not mean the two new lines
you added. Before you modified the existing line, it had used spaces for indent
which were against the guide. After your modification, you kept the spaces for
indent as they were. This is natural, but this is a chance to fix the indent by
spaces. So, I should have said that "taking this chance, let's replace the
spaces with a tab".

> 
> And lower down in block/042, I see that in a past patch you used a
> single tab then spaces:
> 
> >-------if ! src/dio-offsets "${TEST_DEV}" "$sys_max_segments" \
> >-------     "$sys_max_sectors_kb" "$sys_dma_alignment" \
> >-------     "$sys_virt_boundary_mask" "$sys_logical_block_size"; then
> >------->-------echo "src/dio-offsets failed"
> >-------fi
> 
> However, when I check other test files, it seems like people have been
> using only tabs for indent rather than spaces.  For example in
> block/010:
> 
> run_fio_job() {
> >-------_fio_perf --size=8g --bs=4k --direct=1 --ioengine=libaio --iodepth=32 \
> >------->---------group_reporting=1 --rw=randread --norandommap --name=nullb0 \
> >------->---------filename=/dev/nullb0 --name=nullb1 --filename=/dev/nullb1 \
> >------->---------name=nullb2 --filename=/dev/nullb2 --name=nullb3 \
> >------->---------filename=/dev/nullb3 --name=nullb4 --filename=/dev/nullb4 \
> SNIP....
> 
> So, I'm unsure what to do.  Do you want me to use one tab then spaces
> as you did lower in block/042?  Or stick with only tabs as others seem
> to have been doing?  Or only spaces?  Sorry for the confusion and
> thanks for any help!

Ah, I understand that my comment confused you... In the two examples you quoted,
a long statement is broken into multiple lines. In both examples, tabs are used
for the first level of indentation. But the first example uses spaces for the
second level of indentation, to align the continuation lines to the first line:
"$sys_max_sectors_kb" aligns to src/dio/offsetes. On the other hand, the second
example doesn't use spaces. It just uses tabs, then the starts of the
continuation lines are not aligned to the first line. Which way we should
follow? "use additional spaces to align the continuation lines to the first
line" or "just use tabs in the continuation lines"?

In the blktests "new" file, it just says "Indent with tabs". So it does not
strictly guide about the space-vs-tab usage for the alignment of continuation
lines. In general, I think blktests should follow Linux kernel guide. As far as
I read through, Linux kernel coding-style document [1], it does not guide about
it either.

Looking into the Linux code, "additional spaces to align the continuation lines"
looks more common in Linux kernel code. For example, I can find such space usage
in block/blk-core.c [2]. On the other hand, I know that f2fs sub-system "just
uses tabs" for the continuation lines [3] (When I posted a patch to f2fs, I was
guided to use tabs instead of spaces).

Then, it is not strictly guided in Linux kernel and I would say it's the same
for blktests: both ways are fine, and the choice is up to you. Just in case you
do not have preference, I suggest "to use additional spaces to align the
continuation lines" since it looks major in the block sub-system.

[1] https://github.com/torvalds/linux/blob/v6.19-rc6/Documentation/process/coding-style.rst
[2] https://github.com/torvalds/linux/blob/24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7/block/blk-core.c#L321C4-L321C7
[3] https://github.com/torvalds/linux/blob/24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7/fs/f2fs/segment.c#L451