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
next prev 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