Re: [PATCH] nvmet-rdma: recheck queue state is LIVE in state lock in recv done

[Christoph Hellwig <hch@lst.de>]

> +++ b/drivers/nvme/target/rdma.c
> @@ -1042,12 +1042,16 @@ static void nvmet_rdma_recv_done(struct ib_cq *cq, struct ib_wc *wc)
>  		unsigned long flags;
>  
>  		spin_lock_irqsave(&queue->state_lock, flags);
> -		if (queue->state == NVMET_RDMA_Q_CONNECTING)
> -			list_add_tail(&rsp->wait_list, &queue->rsp_wait_list);
> -		else
> -			nvmet_rdma_put_rsp(rsp);
> +		if (queue->state != NVMET_RDMA_Q_LIVE) {

You'll want a comment based on the commit message that the first live
check is racy. 

> +			if (queue->state == NVMET_RDMA_Q_CONNECTING)
> +				list_add_tail(&rsp->wait_list, &queue->rsp_wait_list);
> +			else
> +				nvmet_rdma_put_rsp(rsp);
> +
> +			spin_unlock_irqrestore(&queue->state_lock, flags);
> +			return;

This gets a bit messy now.  What about a struture like this instead?

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 1afd93026f9b..ae6a123ac63f 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -996,6 +996,23 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue,
 	nvmet_req_complete(&cmd->req, status);
 }
 
+static bool nvmet_rdma_recv_not_live(struct nvmet_rdma_queue *queue,
+		struct nvmet_rdma_rsp *rsp)
+{
+	unsigned long flags;
+	bool ret = true;
+
+	spin_lock_irqsave(&queue->state_lock, flags);
+	if (queue->state == NVMET_RDMA_Q_LIVE)
+		ret = false;
+	else if (queue->state == NVMET_RDMA_Q_CONNECTING)
+		list_add_tail(&rsp->wait_list, &queue->rsp_wait_list);
+	else
+		nvmet_rdma_put_rsp(rsp);
+	spin_unlock_irqrestore(&queue->state_lock, flags);
+	return ret;
+}
+
 static void nvmet_rdma_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 {
 	struct nvmet_rdma_cmd *cmd =
@@ -1038,18 +1055,9 @@ static void nvmet_rdma_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 	rsp->n_rdma = 0;
 	rsp->invalidate_rkey = 0;
 
-	if (unlikely(queue->state != NVMET_RDMA_Q_LIVE)) {
-		unsigned long flags;
-
-		spin_lock_irqsave(&queue->state_lock, flags);
-		if (queue->state == NVMET_RDMA_Q_CONNECTING)
-			list_add_tail(&rsp->wait_list, &queue->rsp_wait_list);
-		else
-			nvmet_rdma_put_rsp(rsp);
-		spin_unlock_irqrestore(&queue->state_lock, flags);
+	if (unlikely(queue->state != NVMET_RDMA_Q_LIVE) &&
+	    nvmet_rdma_recv_not_live(queue, rsp))
 		return;
-	}
-
 	nvmet_rdma_handle_command(queue, rsp);
 }