Skip to contentSkip to conversation

nvmet-rdma: add queue to queue list only if connection established

PATCH New Sagi Grimberg 2 participants 3 messages Nov 6, 2016

Subsystems (2)
NVMe NVMe target
lore
Share & tools
lore

Sagi Grimberg

author maintainer
#0 lore raw

PATCH 1/1 nvmet-rdma: add queue to queue list only if connection established

In case we accepted a queue connection and it failed, we might not remove the queue from the list. Instead of dealing with it, simply add the insert the queue only if we successfully established.

Signed-off-by Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/target/rdma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/nvme/target/rdma.c +4 -4
index bafd9898b5f6..c25a5f58eea6 100644
@@ -1184,10 +1184,6 @@ static int nvmet_rdma_queue_connect(struct rdma_cm_id *cm_id,
 	if (ret)
 		goto release_queue;
 
-	mutex_lock(&nvmet_rdma_queue_mutex);
-	list_add_tail(&queue->queue_list, &nvmet_rdma_queue_list);
-	mutex_unlock(&nvmet_rdma_queue_mutex);
-
 	return 0;
 
 release_queue:

@@ -1209,6 +1205,10 @@ static void nvmet_rdma_queue_established(struct nvmet_rdma_queue *queue)
 	}
 	queue->state = NVMET_RDMA_Q_LIVE;
 
+	mutex_lock(&nvmet_rdma_queue_mutex);
+	list_add_tail(&queue->queue_list, &nvmet_rdma_queue_list);
+	mutex_unlock(&nvmet_rdma_queue_mutex);
+
 	while (!list_empty(&queue->rsp_wait_list)) {
 		struct nvmet_rdma_rsp *cmd;
Signature · 1 line
2.7.4
maintainer
#1 lore raw

PATCH 1/1 nvmet-rdma: add queue to queue list only if connection established

Quoted 3 lines · Sagi Grimberg

In case we accepted a queue connection and it failed, we might not remove the queue from the list. Instead of dealing with it, simply add the insert the queue only if we successfully established.

But that means we'd potentionally leak it when unloading the module, if that happens at just the wrong time, don't we?

author maintainer
#2 lore raw

PATCH 1/1 nvmet-rdma: add queue to queue list only if connection established

Quoted 7 lines
Quoted 4 lines

In case we accepted a queue connection and it failed, we might not remove the queue from the list. Instead of dealing with it, simply add the insert the queue only if we successfully established.

But that means we'd potentionally leak it when unloading the module, if that happens at just the wrong time, don't we?

Yea... didn't think of that. I started with removing from the list on connect failure too but thought this would be better...

I'll reverse the strategy again...