Thread (31 messages) 31 messages, 8 authors, 2016-10-11

RE: [PATCH 9/9] [RFC] nvme: Fix a race condition

From: Steve Wise <hidden>
Date: 2016-09-27 16:56:19
Also in: linux-nvme, linux-rdma, linux-scsi

On 09/27/2016 09:31 AM, Steve Wise wrote:
quoted
quoted
@@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
 void nvme_stop_queues(struct nvme_ctrl *ctrl)
 {
 	struct nvme_ns *ns;
+	struct request_queue *q;

 	mutex_lock(&ctrl->namespaces_mutex);
 	list_for_each_entry(ns, &ctrl->namespaces, list) {
-		blk_mq_cancel_requeue_work(ns->queue);
-		blk_mq_stop_hw_queues(ns->queue);
+		q = ns->queue;
+		blk_quiesce_queue(q);
+		blk_mq_cancel_requeue_work(q);
+		blk_mq_stop_hw_queues(q);
+		blk_resume_queue(q);
 	}
 	mutex_unlock(&ctrl->namespaces_mutex);
Hey Bart, should nvme_stop_queues() really be resuming the blk queue?
Hello Steve,

Would you perhaps prefer that blk_resume_queue(q) is called from
nvme_start_queues()? I think that would make the NVMe code harder to
review. 
I'm still learning the blk code (and nvme code :)), but I would think
blk_resume_queue() would cause requests to start being submit on the NVME
queues, which I believe shouldn't happen when they are stopped.  I'm currently
debugging a problem where requests are submitted to the nvme-rdma driver while
it has supposedly stopped all the nvme and blk mqs.  I tried your series at
Christoph's request to see if it resolved my problem, but it didn't.  
The above code won't cause any unexpected side effects if an
NVMe namespace is removed after nvme_stop_queues() has been called and
before nvme_start_queues() is called. Moving the blk_resume_queue(q)
call into nvme_start_queues() will only work as expected if no
namespaces are added nor removed between the nvme_stop_queues() and
nvme_start_queues() calls. I'm not familiar enough with the NVMe code to
know whether or not this change is safe ...
I'll have to look and see if new namespaces can be added/deleted while a nvme
controller is in the RECONNECTING state.   In the meantime, I'm going to move
the blk_resume_queue() to nvme_start_queues() and see if it helps my problem.

Christoph:  Thoughts?

Steve.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help