Re: [RFC PATCH v1 01/17] nvme-multipath: retarget failedover bios from requeue work

Hi=EF=BC=8C

=E5=9C=A8 2026/7/13 20:12, Christoph Hellwig =E5=86=99=E9=81=93:
> On Mon, Jul 13, 2026 at 11:29:35AM +0200, Hannes Reinecke wrote:
>> On 7/4/26 9:51 PM, Yu Kuai wrote:
>>> From: Yu Kuai <[email protected]>
>>>
>>> bio_set_dev() is about to become explicitly sleepable because it can
>>> associate the bio with a blkg for the destination queue.  NVMe failover
>>> can run from request completion context, and nvme_failover_req() also h=
olds
>>> head->requeue_lock with interrupts disabled while it steals bios from t=
he
>>> failed request.  Calling bio_set_dev() there is not safe once the helpe=
r is
>>> allowed to sleep.
>>>
>>> The requeue lock only protects head->requeue_list.  Keep the list
>>> manipulation under that lock, but defer retargeting to nvme_requeue_wor=
k(),
>>> which already drains the list from process context before resubmitting =
each
>>> bio.  The bios remain private to the requeue list until the worker pops
>>> them, so moving the device switch there preserves the existing retry fl=
ow
>>> while avoiding a sleepable helper in completion context.
>>>
>>> Signed-off-by: Yu Kuai <[email protected]>
>>> ---
>>>    drivers/nvme/host/multipath.c | 4 +---
>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipat=
h.c
>>> index 9b9a657fa330..76baa180ae1c 100644
>>> --- a/drivers/nvme/host/multipath.c
>>> +++ b/drivers/nvme/host/multipath.c
>>> @@ -149,7 +149,6 @@ void nvme_failover_req(struct request *req)
>>>    	struct nvme_ns *ns =3D req->q->queuedata;
>>>    	u16 status =3D nvme_req(req)->status & NVME_SCT_SC_MASK;
>>>    	unsigned long flags;
>>> -	struct bio *bio;
>>>      	nvme_mpath_clear_current_path(ns);
>>>    	atomic_long_inc(&ns->failover);
>>> @@ -165,8 +164,6 @@ void nvme_failover_req(struct request *req)
>>>    	}
>>>      	spin_lock_irqsave(&ns->head->requeue_lock, flags);
>>> -	for (bio =3D req->bio; bio; bio =3D bio->bi_next)
>>> -		bio_set_dev(bio, ns->head->disk->part0);
>> If you remove this the original device remains being referenced by
>> the bio, so there might be a chance of some accidentally referencing
>> the (now invalid) bdev.
>> I think it might be better if you were set it to NULL here, to
>> signal that this bio currently has no bdev associated.
> What should reference it?  This moves setting bi_bdev from the only
> place adding to the list to the only place removing from the list.

I'll follow your advice to move blkg association to bio submit. And this pa=
tch
will not be needed anymore.

>
--=20
Thanks,
Kuai