Re: [PATCH 90/98] nvmet-rdma: disable interrupts under rsp_wr_wait_lock

[Bart Van Assche <bvanassche@acm.org>]

On 10/28/26 09:42, Jens Axboe wrote:
> -		spin_unlock(&queue->rsp_wr_wait_lock);
> +		spin_unlock_irqrestore(&queue->rsp_wr_wait_lock, flags);
>  		ret = nvmet_rdma_execute_command(rsp);
> -		spin_lock(&queue->rsp_wr_wait_lock);
> +		spin_lock_irqsave(&queue->rsp_wr_wait_lock, flags);

Using spin_lock_irqsave() fixes the rsp_wr_wait_lock deadlock itself,
but when nvmet_rdma_process_wr_wait_list() is reached from a bio end_io
callback in hardirq or softirq context (via nvmet_req_complete() ->
nvmet_rdma_queue_response() -> failed ib_post_send() ->
nvmet_rdma_release_rsp()), spin_unlock_irqrestore() restores that
hardirq/softirq context and calls nvmet_rdma_execute_command(rsp) ->
rsp->req.execute(&rsp->req).

The nvmet ->execute() handlers require process context:
- nvmet_bdev_execute_rw() allocates bios with GFP_KERNEL and calls
  submit_bio() -> submit_bio_noacct(), which hits might_sleep() and
  manipulates current->bio_list without interrupt synchronization.
- Admin, fabrics, auth, and PR handlers perform GFP_KERNEL allocations
  and take mutexes / rwsems.

If nvmet_rdma_process_wr_wait_list() checks !in_task() under
rsp_wr_wait_lock and schedules a per-queue wr_wait_work on system_dfl_wq
when rsp_wr_wait_list is non-empty (cancelled after ib_drain_qp() in
nvmet_rdma_destroy_queue_ib()), normal task/CQ-workqueue callers pay no
extra cost while hardirq/softirq completions safely defer command
execution to process context.

Note: patches 91/98 and 92/98 compose cleanly on top by placing the
if (done && done->wait_released) check right above if (!in_task()) and
passing NULL from nvmet_rdma_wr_wait_work().

Here is a replacement patch (checked with checkpatch.pl and
syntax-checked with Clang; not runtime-tested):

---
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 26ea4e1d62b3..60ea488db8f4 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -104,6 +104,7 @@ struct nvmet_rdma_queue {
 	struct nvmet_rdma_cmd	*cmds;
 
 	struct work_struct	release_work;
+	struct work_struct	wr_wait_work;
 	struct list_head	rsp_wait_list;
 	struct list_head	rsp_wr_wait_list;
 	spinlock_t		rsp_wr_wait_lock;
@@ -513,7 +514,15 @@ static int nvmet_rdma_post_recv(struct nvmet_rdma_device *ndev,
 
 static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue)
 {
-	spin_lock(&queue->rsp_wr_wait_lock);
+	unsigned long flags;
+
+	spin_lock_irqsave(&queue->rsp_wr_wait_lock, flags);
+	if (!in_task()) {
+		if (!list_empty(&queue->rsp_wr_wait_list))
+			schedule_work(&queue->wr_wait_work);
+		spin_unlock_irqrestore(&queue->rsp_wr_wait_lock, flags);
+		return;
+	}
 	while (!list_empty(&queue->rsp_wr_wait_list)) {
 		struct nvmet_rdma_rsp *rsp;
 		bool ret;
@@ -522,16 +531,24 @@ static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue)
 				struct nvmet_rdma_rsp, wait_list);
 		list_del(&rsp->wait_list);
 
-		spin_unlock(&queue->rsp_wr_wait_lock);
+		spin_unlock_irqrestore(&queue->rsp_wr_wait_lock, flags);
 		ret = nvmet_rdma_execute_command(rsp);
-		spin_lock(&queue->rsp_wr_wait_lock);
+		spin_lock_irqsave(&queue->rsp_wr_wait_lock, flags);
 
 		if (!ret) {
 			list_add(&rsp->wait_list, &queue->rsp_wr_wait_list);
 			break;
 		}
 	}
-	spin_unlock(&queue->rsp_wr_wait_lock);
+	spin_unlock_irqrestore(&queue->rsp_wr_wait_lock, flags);
+}
+
+static void nvmet_rdma_wr_wait_work(struct work_struct *w)
+{
+	struct nvmet_rdma_queue *queue =
+		container_of(w, struct nvmet_rdma_queue, wr_wait_work);
+
+	nvmet_rdma_process_wr_wait_list(queue);
 }
 
 static u16 nvmet_rdma_check_pi_status(struct ib_mr *sig_mr)
@@ -976,6 +993,7 @@ static bool nvmet_rdma_execute_command(struct nvmet_rdma_rsp *rsp)
 static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue,
 		struct nvmet_rdma_rsp *cmd)
 {
+	unsigned long flags;
 	u16 status;
 
 	ib_dma_sync_single_for_cpu(queue->dev->device,
@@ -993,9 +1011,9 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue,
 		goto out_err;
 
 	if (unlikely(!nvmet_rdma_execute_command(cmd))) {
-		spin_lock(&queue->rsp_wr_wait_lock);
+		spin_lock_irqsave(&queue->rsp_wr_wait_lock, flags);
 		list_add_tail(&cmd->wait_list, &queue->rsp_wr_wait_list);
-		spin_unlock(&queue->rsp_wr_wait_lock);
+		spin_unlock_irqrestore(&queue->rsp_wr_wait_lock, flags);
 	}
 
 	return;
@@ -1374,6 +1392,7 @@ static bool nvmet_rdma_reclaim_rsp(struct sbitmap *sb, unsigned int bitnr,
 static void nvmet_rdma_destroy_queue_ib(struct nvmet_rdma_queue *queue)
 {
 	ib_drain_qp(queue->qp);
+	cancel_work_sync(&queue->wr_wait_work);
 
 	/*
 	 * Reclaim resources of a response that is still in-flight when the
@@ -1494,6 +1513,7 @@ nvmet_rdma_alloc_queue(struct nvmet_rdma_device *ndev,
 	 * inside a CM callback would trigger a deadlock. (great API design..)
 	 */
 	INIT_WORK(&queue->release_work, nvmet_rdma_release_queue_work);
+	INIT_WORK(&queue->wr_wait_work, nvmet_rdma_wr_wait_work);
 	queue->dev = ndev;
 	queue->cm_id = cm_id;
 	queue->port = port->nport;