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

Shinichiro Kawasaki
2024-01-11T07:34:36Z

On Jan 10, 2024 / 09:50, Bart Van Assche wrote:
> 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.

Thanks for pointing out the incomprehensible sentence. The grammar checker looks
useful for improving my writing. I will use it to revise the commit message as
well as the code block comments.

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