Re: [GIT PULL] first round of SCSI updates for the 5.14+ merge window

[Jens Axboe <axboe@kernel.dk>]

On 9/2/21 5:23 PM, James Bottomley wrote:
> On Thu, 2021-09-02 at 15:38 -0700, Linus Torvalds wrote:
>> On Thu, Sep 2, 2021 at 9:50 AM James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>>> We also picked up a non trivial conflict with the already upstream
>>> block tree in st.c
>>
>> Hmm. Resolving that conflict, I just reacted to how the st.c code
>> passes in a NULL gendisk to scsi_ioctl() and then on to
>> blk_execute_rq().
>>
>> Just checking that was fine, and I notice how *many* places do that.
>>
>> Should the blk_execute_rq() function even take that "struct gendisk
>> *bd_disk" argument at all?
>>
>> Maybe the right thing to do would be for the people who care to just
>> set rq->rq_disk before starting the request..
>>
>> But I guess it's traditional, and nobody cares.
> 
> It's certainly traditional, but Christoph has been caring a lot about
> cleaning up our gendisks recently, so he might be interested in seeing
> if he can fix it ...

It's trivially doable, it's more a question of whether it's a good idea
or not... At least when passed in you have to make a conscious decision
on it being NULL. And just like Linus did, it's something you'll notice
and see what other callers are doing.

As I said, I don't feel that strongly about it. Hopefully most cases
that would forget to set it would trigger a NULL defer when they test
their code, and the core does initialize ->rq_disk to NULL. It's less
risky now than earlier, where the request alloc path was less
streamlined.

-- 
Jens Axboe