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

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

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