mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] RDMA/srp: Reserve send queue and CQ space for ib_drain_qp()
@ 2026-11-10 22:24 Praneeth Balasubramanian
  2026-11-11  0:44 ` Bart Van Assche
  0 siblings, 1 reply; 2+ messages in thread
From: Praneeth Balasubramanian @ 2026-11-10 22:24 UTC (permalink / raw)
  To: Bart Van Assche, Jason Gunthorpe, Leon Romanovsky
  Cc: linux-rdma, linux-kernel, Praneeth Balasubramanian

srp_destroy_qp() calls ib_drain_qp(), which also drains the send queue.
ib_drain_sq() requires the caller to "ensure there is room in the CQ and
SQ for the drain work request and completion".

SRP reserves that room on the receive side, where the queue and CQ have
queue_size + 1 entries. Commit 9294000d6d89 ("IB/srp: Drain the send
queue before destroying a QP") switched from ib_drain_rq() to
ib_drain_qp() but did not grow the send side. The send queue and send
CQ hold exactly m * queue_size entries, which is the most work requests
SRP can have outstanding. If the channel is torn down while the send
queue is full, for example during error handling after a target stops
responding, posting the drain work request can fail with "failed to
drain send queue", or its completion can find the CQ full. The QP is
then destroyed without the guarantee that ib_drain_qp() is meant to
provide.

Add one entry to the send queue and the send CQ, as nvme-rdma and iser
already do.

The missing reservation was found with an LLM-based tool that checks
callers against the rules stated in kernel comments.

Fixes: 9294000d6d89 ("IB/srp: Drain the send queue before destroying a QP")
Signed-off-by: Praneeth Balasubramanian <praneeth@ucsb.edu>
---

Notes:
    This was found by an LLM-based static analysis (GPT-5.6) that checks
    kernel code against the rules stated in its comments, as part of research
    at UCSB SecLab. The patch and changelog were drafted with Claude Opus 5.5.
    I reviewed the patch and take responsibility for it.
    
    Testing: builds without new warnings with W=1 (x86_64). Not
    runtime-tested. I do not have an SRP target setup, and I have not
    observed the drain failure myself. The change only follows the
    ib_drain_sq() contract and matches the receive side and nvme-rdma.
    
    The patch applies to mainline 9ba46df34488 and to next-20261109.

 drivers/infiniband/ulp/srp/ib_srp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 955f36efeebd..2cd0b728424e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -546,7 +546,8 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
 		goto err;
 	}
 
-	send_cq = ib_alloc_cq(dev->dev, ch, m * target->queue_size,
+	/* m * queue_size + 1 for ib_drain_sq() */
+	send_cq = ib_alloc_cq(dev->dev, ch, m * target->queue_size + 1,
 				ch->comp_vector, IB_POLL_DIRECT);
 	if (IS_ERR(send_cq)) {
 		ret = PTR_ERR(send_cq);
@@ -554,7 +555,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
 	}
 
 	init_attr->event_handler       = srp_qp_event;
-	init_attr->cap.max_send_wr     = m * target->queue_size;
+	init_attr->cap.max_send_wr     = m * target->queue_size + 1;
 	init_attr->cap.max_recv_wr     = target->queue_size + 1;
 	init_attr->cap.max_recv_sge    = 1;
 	init_attr->cap.max_send_sge    = min(attr->max_send_sge, SRP_MAX_SGE);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] RDMA/srp: Reserve send queue and CQ space for ib_drain_qp()
  2026-11-10 22:24 [PATCH] RDMA/srp: Reserve send queue and CQ space for ib_drain_qp() Praneeth Balasubramanian
@ 2026-11-11  0:44 ` Bart Van Assche
  0 siblings, 0 replies; 2+ messages in thread
From: Bart Van Assche @ 2026-11-11  0:44 UTC (permalink / raw)
  To: Praneeth Balasubramanian, Jason Gunthorpe, Leon Romanovsky
  Cc: linux-rdma, linux-kernel, Praneeth Balasubramanian

On 11/10/26 2:24 PM, Praneeth Balasubramanian wrote:
 > srp_destroy_qp() calls ib_drain_qp(), which also drains the send queue.
 > ib_drain_sq() requires the caller to "ensure there is room in the CQ and
 > SQ for the drain work request and completion".
 >
 > SRP reserves that room on the receive side, where the queue and CQ have
 > queue_size + 1 entries. Commit 9294000d6d89 ("IB/srp: Drain the send
 > queue before destroying a QP") switched from ib_drain_rq() to
 > ib_drain_qp() but did not grow the send side.

Agreed. srp_destroy_qp() reaps the send CQ before it calls
ib_drain_qp(). After the QP has been moved into the error state, up to
m * queue_size flushed work requests may complete on the send CQ, and
the drain work request needs a free send queue slot. Hence, both the
send queue and the send CQ need one additional entry.

 > Signed-off-by: Praneeth Balasubramanian <praneeth@ucsb.edu>

The From: header of this email is praneethb2002@gmail.com while the
Signed-off-by: tag uses praneeth@ucsb.edu. If this patch is applied
as-is, the gmail address will be recorded as the author. Please either
add an in-body "From: Praneeth Balasubramanian <praneeth@ucsb.edu>" line
when reposting or make sure that both addresses match.

With that addressed:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-11  0:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-10 22:24 [PATCH] RDMA/srp: Reserve send queue and CQ space for ib_drain_qp() Praneeth Balasubramanian
2026-11-11  0:44 ` Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®