Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
From: Allison Henderson <achender@kernel.org>
To: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	pabeni@redhat.com, edumazet@google.com, kuba@kernel.org,
	horms@kernel.org
Cc: achender@kernel.org
Subject: [PATCH net-next v8 10/13] net/rds: take cp_lock to purge cp_send_queue in the quiesce
Date: Sat,  3 Oct 2026 09:32:12 -0700	[thread overview]
Message-ID: <20261003163215.250253-11-achender@kernel.org> (raw)
In-Reply-To: <20261003163215.250253-1-achender@kernel.org>

rds_conn_path_quiesce() empties cp_send_queue by walking it with no
lock held, while every path that adds to that queue -
rds_send_queue_rm(), rds_send_probe(), the retransmit requeue in
rds_send_path_reset() - does so under cp_lock.  The unlocked walk was
justified by the destroy running with nothing else alive: at every
destroy trigger there is - netns teardown and module unload - no
socket can still be sending on the connection, since a bound socket
pins its transport module and a namespace closes its sockets before
its RDS connections are torn down.

That argument still holds, but it is an argument about the callers,
not a property of the code.  Splice the queue away under cp_lock and
drop the message references outside it, so that the one walker of the
list follows the same lock discipline as its adders.  This does not
by itself make a sender that is still running safe - nothing here
tells such a sender that the queue is closed - and it does not need
to, since no such sender exists at any destroy trigger.

While at it, make the purge coherent with rds_send_drop_to(), the
other path that removes messages from a connection queue.  drop_to
decides whether it owns the queue's reference by test_and_clear on
RDS_MSG_ON_CONN, and unlinks m_conn_item from whatever list the
message is on.  The purge used to BUG_ON() a message that a socket
still had queued, and that assertion is also what kept a drop_to
racing it from putting the queue's reference a second time or
unlinking from the purge list; the lock above is what closes the
stale-next hazard of the old walk, not the assertion.  Clear the bit
under cp_lock as part of the splice, so that drop_to leaves a purged
message alone, and turn the BUG_ON() into a WARN_ON_ONCE(): a socket
with messages queued at destroy is still a condition worth reporting -
no sender or closer can be running at any destroy trigger today - but
not one worth a panic, since the socket side keeps its own reference
and retires the message on close.

Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
 net/rds/connection.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c7655e9339ca..9c8c4b28d2b2 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -540,6 +540,8 @@ void rds_conn_shutdown(struct rds_conn_path *cp)
 static void rds_conn_path_quiesce(struct rds_conn_path *cp)
 {
 	struct rds_message *rm, *rtmp;
+	unsigned long flags;
+	LIST_HEAD(purge);
 
 	if (!cp->cp_transport_data)
 		return;
@@ -551,12 +553,29 @@ static void rds_conn_path_quiesce(struct rds_conn_path *cp)
 	rds_conn_path_drop(cp, true);
 	flush_work(&cp->cp_down_w);
 
-	/* tear down queued messages */
-	list_for_each_entry_safe(rm, rtmp,
-				 &cp->cp_send_queue,
-				 m_conn_item) {
+	/* Tear down queued messages.  Every path that adds to
+	 * cp_send_queue does so under cp_lock; take it here too.  No
+	 * sender can still be running at any destroy trigger, so this is
+	 * lock discipline rather than a race fix: nothing here tells a
+	 * sender that the queue is closed.
+	 */
+	spin_lock_irqsave(&cp->cp_lock, flags);
+	list_splice_init(&cp->cp_send_queue, &purge);
+	/* Give up the queue's claim on each message while still under
+	 * the lock, so that rds_send_drop_to(), which decides ownership
+	 * of the connection-queue reference by this bit, neither drops
+	 * it a second time nor unlinks the message from our list.
+	 */
+	list_for_each_entry(rm, &purge, m_conn_item)
+		clear_bit(RDS_MSG_ON_CONN, &rm->m_flags);
+	spin_unlock_irqrestore(&cp->cp_lock, flags);
+	list_for_each_entry_safe(rm, rtmp, &purge, m_conn_item) {
+		/* No socket can still have messages queued on a connection
+		 * at any destroy trigger; say so if one does, since the
+		 * socket side then retires the message on its own.
+		 */
+		WARN_ON_ONCE(!list_empty(&rm->m_sock_item));
 		list_del_init(&rm->m_conn_item);
-		BUG_ON(!list_empty(&rm->m_sock_item));
 		rds_message_put(rm);
 	}
 	if (cp->cp_xmit_rm)
-- 
2.25.1


  parent reply	other threads:[~2026-10-03 16:32 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-03 16:32 [PATCH net-next v8 00/13] net/rds: make connection lifetime reference-counted Allison Henderson
2026-10-03 16:32 ` [PATCH net-next v8 01/13] net/rds: ib: don't enable interrupts in rds_ib_conn_free() Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-03 16:32 ` [PATCH net-next v8 02/13] net/rds: undo conn_alloc() the same way on every __rds_conn_create() exit Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-03 16:32 ` [PATCH net-next v8 03/13] net/rds: ib: refuse to attach a connection to a device being removed Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:34   ` netdev-bot+sashiko
2026-10-03 16:32 ` [PATCH net-next v8 04/13] net/rds: guard every work-requeueing site with rds_destroy_pending() Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:34   ` netdev-bot+sashiko
2026-10-03 16:32 ` [PATCH net-next v8 05/13] net/rds: make rds_destroy_pending() report a connection's own destroy Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-03 16:32 ` [PATCH net-next v8 06/13] net/rds: split connection destroy into quiesce and kref-governed free Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-03 16:32 ` [PATCH net-next v8 07/13] net/rds: unlink transport nodes before a possibly deferred connection free Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:35   ` netdev-bot+sashiko
2026-10-03 16:32 ` [PATCH net-next v8 08/13] net/rds: wait for connections to be freed on transport unload Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:35   ` netdev-bot+sashiko
2026-10-03 16:32 ` [PATCH net-next v8 09/13] net/rds: hold a connection reference from struct rds_incoming Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:35   ` netdev-bot+sashiko
2026-10-03 16:32 ` Allison Henderson [this message]
2026-10-03 17:56   ` [PATCH net-next v8 10/13] net/rds: take cp_lock to purge cp_send_queue in the quiesce sashiko-bot
2026-10-03 16:32 ` [PATCH net-next v8 11/13] net/rds: hold connection references in lookup, sockets and c_passive Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:35   ` netdev-bot+sashiko
2026-10-03 16:32 ` [PATCH net-next v8 12/13] net/rds: pin the connection across RDMA-CM event handling Allison Henderson
2026-10-03 17:56   ` sashiko-bot
2026-10-04 16:35   ` netdev-bot+sashiko
2026-10-03 16:32 ` [PATCH net-next v8 13/13] net/rds: drop rds_conn_count in favor of t_conn_count Allison Henderson
2026-10-03 17:56   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261003163215.250253-11-achender@kernel.org \
    --to=achender@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox