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@xxxxxxxxxxxxxxxxxxxxx> 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
