Re: [PATCH blktests v2 2/2] block/031: allow to run with built-in null_blk driver

Bart Van Assche
2024-01-10T17:50:17Z

On 1/9/24 18:19, Shinichiro Kawasaki wrote:
> On Jan 09, 2024 / 12:19, Bart Van Assche wrote:
>> On 1/9/24 02:44, Shin'ichiro Kawasaki wrote:
>>> The parameter has been set as a module parameter then null_blk
>>> driver must be loadable.
>>
>> It seems like the word "If" is missing from the start of the above sentence?
>
> With this sentence, I wanted to describe current implmenetation fact. Before
> applying this patch, shared_tag_bitmap=1 was set as a module parameter, not
> through a configfs file. So I don't think "If" is missing.

If I upload the sentence that I quoted to a free online grammar checker
(https://quillbot.com/grammar-check) then it tells me that there is a
grammatical issue with that sentence. That's why I asked whether a word is
perhaps missing. I'm not trying to be pedantic - I raised my question because
the above sentence is incomprehensible to me.

> I think the inline comment above was not clear enough and caused the confusion.
> How about to improve the comment as follows? I hope it explains why
> _init_null_blk is called in the if-statement.
>
> # _configure_null_blk() sets null_blk parameters via configfs, while
> # _init_null_blk() sets null_blk parameters as module parameters.
> # Old kernel requires shared_tag_bitmap as a module parameter. In that
> # case, call _init_null_blk() for shared_tag_bitmap.

That sounds better to me.

Thanks,

Bart.