From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 86F253D6494; Sat, 17 Oct 2026 03:23:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792207406; cv=none; b=Q6RUGx2d2KUg9N+PxodgTecSox/UbGdt1cAbcrTLnWHQDr1R3JtrbAqgxspk0aPxyd8I1yZ+osyDHzJPJGuHVqcnG99sjCBfEQJwRFUv6yg9/yrA/wZOQnDROwHWN/A+u0ua8KazNWK0+xvEl6otZPzdLEeVUmw3MwMIP6VJfQ8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792207406; c=relaxed/simple; bh=QDurr9a3JKmMDkZ7LTmEZljLCP9JCNcPdZJIqMqaTe4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CkKSvulwXhWHpDAgwt1WGBkeTf+TnI2fK40Lc9evueAQzJEJ3YU8iapR3C9LukOwfQPwTGEh6MM73gXxaAJWvZUOJB6eVjETAf5hVZDBeL/B6bSUpW7I1SJ8DrNT/s+Uj0jN3zZRK7FoxmHqvsEGH0zBkqgj3ixwZUYLJOIxV+w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=EoO9pEsV; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="EoO9pEsV" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1D1941F000C9; Sat, 17 Oct 2026 03:23:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1792207405; bh=6xaoxifvY23UPd5Pl5k1g4wGQeX4X9m3tRKXdO7g+0k=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=EoO9pEsVuB/yldGkZCfFKiBFMs5eBN4E0RmVYO+rOJHQDCZusYYL1foczWZmmDpva AhfvQveCkgkVZHf9Rpsyslnl5FS+GnDA+ajQUqRCoCksX7nutwNY6aLI3hNi4Y2o8D 1UPdJy677Nk2UrZE6lazYra29ZACAIXi4au7G/g0NAmk1RSEfrxVG6l7FHBYutdVAW +YpsCogiq0dTxl03PV4jocKvWjd4ZcXjbogSJ0WVHuW8W5Gj5DyKfKS6c40XDhJfIA cjg4jDEM7y4ajc+FSLWzt/PyqfrMvsLEj1US/Lwew9gkOLXtmgWraYN0fWqfCZG/WB 2Tgs2sj5ntRdw== From: Allison Henderson 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 v10 10/13] net/rds: take cp_lock to purge cp_send_queue in the quiesce Date: Fri, 16 Oct 2026 20:23:16 -0700 Message-ID: <20261017032319.2563014-11-achender@kernel.org> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20261017032319.2563014-1-achender@kernel.org> References: <20261017032319.2563014-1-achender@kernel.org> Precedence: bulk X-Mailing-List: linux-rdma@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 --- 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.50.1