[PATCH 18/19] nvmet-rdma: fix a lost run of the wait list

[Keith Busch]

From: Chris Mason <mason at kernel.org>

A command is checked for free send queue slots before it's on the wait
list, in nvmet_rdma_handle_command() and when the processing loop takes
it off to retry, and nvmet_rdma_release_rsp() looks at the list without
a lock after giving slots back. The CQ worker and the CM thread (via
nvmet_rdma_queue_established()) can handle the commands of one queue
concurrently, so a release can see an empty list while the other side
is between the failed slot check and the list add:

    handle a command                  release a response
    too few free slots
                                      give the slots back
                                      wait list empty: return
    add the command to the list       /* nobody runs the list */

If that was the last response in flight the command stays on the list
forever. It holds sq->ref, so nvmet_sq_destroy() hangs at disconnect
and the queue's memory is never freed. A host hits it by sending more
Connects at once than the send queue has slots.

Look at the slots again after a command is on the list: the loop retries
after a put-back with a full barrier in between, nvmet_rdma_release_rsp()
has a matching barrier between returning the slots and looking at the
list, and nvmet_rdma_handle_command() runs the list once after adding
its command.

Fixes: 8f000cac6e7a ("nvmet-rdma: add a NVMe over Fabrics RDMA target driver")
Cc: Christoph Hellwig <hch at lst.de>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Chaitanya Kulkarni <kch at nvidia.com>
Signed-off-by: Chris Mason <mason at kernel.org>
Signed-off-by: Jens Axboe <axboe at kernel.dk>
Signed-off-by: Keith Busch <kbusch at kernel.org>
---
 drivers/nvme/target/rdma.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 1e434cc451c28..7f22a2a1886dc 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -519,7 +519,7 @@ static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue,
 	unsigned long flags;
 
 	spin_lock_irqsave(&queue->rsp_wr_wait_lock, flags);
-	if (done->wait_released) {
+	if (done && done->wait_released) {
 		/*
 		 * The loop below has not come back from this command yet. It is
 		 * further up this stack if the command completed in place and
@@ -551,6 +551,15 @@ static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue,
 			WRITE_ONCE(rsp->wait_released, NULL);
 		if (!ret) {
 			list_add(&rsp->wait_list, &queue->rsp_wr_wait_list);
+			/*
+			 * A release that came while the command was off the
+			 * list can have seen an empty list, so look at the
+			 * room again.
+			 * Pairs with the barrier in nvmet_rdma_release_rsp().
+			 */
+			smp_mb();
+			if (atomic_read(&queue->sq_wr_avail) > rsp->n_rdma)
+				continue;
 			break;
 		}
 	}
@@ -698,6 +707,11 @@ static void nvmet_rdma_release_rsp(struct nvmet_rdma_rsp *rsp)
 	struct nvmet_rdma_queue *queue = rsp->queue;
 
 	atomic_add(1 + rsp->n_rdma, &queue->sq_wr_avail);
+	/*
+	 * Give the room back ahead of the look at the wait list below.
+	 * Pairs with the smp_mb() in nvmet_rdma_process_wr_wait_list().
+	 */
+	smp_mb__after_atomic();
 
 	nvmet_rdma_free_rsp_resources(rsp);
 
@@ -1025,6 +1039,8 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue,
 		spin_lock_irqsave(&queue->rsp_wr_wait_lock, flags);
 		list_add_tail(&cmd->wait_list, &queue->rsp_wr_wait_list);
 		spin_unlock_irqrestore(&queue->rsp_wr_wait_lock, flags);
+		/* The last response may have been released in between. */
+		nvmet_rdma_process_wr_wait_list(queue, NULL);
 	}
 
 	return;
-- 
2.52.0