Re: [PATCH 0/3 rfc] Fix nvme-tcp and nvme-rdma controller reset hangs

[Sagi Grimberg <sagi@grimberg.me>]

>>> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
>>> index a1d476e1ac020f..92adebfaf86fd1 100644
>>> --- a/drivers/nvme/host/multipath.c
>>> +++ b/drivers/nvme/host/multipath.c
>>> @@ -309,6 +309,7 @@ blk_qc_t nvme_ns_head_submit_bio(struct bio *bio)
>>>    	 */
>>>    	blk_queue_split(&bio);
>>>    +retry:
>>>    	srcu_idx = srcu_read_lock(&head->srcu);
>>>    	ns = nvme_find_path(head);
>>>    	if (likely(ns)) {
>>> @@ -316,7 +317,12 @@ blk_qc_t nvme_ns_head_submit_bio(struct bio *bio)
>>>    		bio->bi_opf |= REQ_NVME_MPATH;
>>>    		trace_block_bio_remap(bio, disk_devt(ns->head->disk),
>>>    				      bio->bi_iter.bi_sector);
>>> -		ret = submit_bio_noacct(bio);
>>> +
>>> +		if (!blk_mq_submit_bio_direct(bio, &ret)) {
>>> +			nvme_mpath_clear_current_path(ns);
>>> +			srcu_read_unlock(&head->srcu, srcu_idx);
>>
>> Its a bit unusual to see mutation of a data protected by srcu while
>> under the srcu_read_lock, can that be problematic somehow?
> 
> Hmm.  I don't think head->srcu is intended to protect the current path.
> We also call nvme_mpath_clear_current_path from nvme_complete_rq or
> nvme_ns_remove, which has no locking at all.  The srcu protection is
> for head->list, but leaks into the current path due to the __rcu
> annotations.

OK, care to send a formal patch that I can give a test drive?

Also, given that this issue has gone back to stable 5.4 and 5.10 we will
need to take care of those too. We should make sure to annotate the
fixes tags in this patch and probably also understand how we can create
a version of this to apply cleanly (for sure there are some extra
dependencies).

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme