Re: [PATCH 0/3 rfc] Fix nvme-tcp and nvme-rdma controller reset hangs
From: Sagi Grimberg <sagi@grimberg.me>
Date: 2021-03-21 06:49:57
quoted
quoted
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