Re: [PATCH net-next 0/4] net/rds: own the fastpath locks across connection teardown

Allison Henderson <[email protected]>
Newsgroups org.kernel.vger.linux-rdma,org.kernel.vger.netdev
Message-ID <[email protected]>
On Thu, 2026-08-13 at 18:34 -0700, Allison Henderson wrote:
> Hi all,
> 
> This is a follow-up set to the recent "net/rds: Bug fix ports, part 2"
> series [1]. During review of v1, the later half of the set needed more
> work than just a respin, so it was split off into a separate set here
> along with a few other companion fixes identified in the reivew.
> 
> RDS connection teardown quiesces the transmit and receive-refill fast
> paths by waiting for the RDS_IN_XMIT/RDS_RECV_REFILL bits to be
> sampled clear.  Sampling a bit clear is not owning it: the fast path
> can re-take its bit right after the wait returns and then run
> concurrently with the transport shutdown and the send-state reset.
> Oracle UEK closed this by making teardown acquire the bits as locks
> ("rds: Make sure transmit path and connection tear-down does not run
> concurrently"); patches 3 and 4 do the same for the two
> rds_send_path_reset() call sites upstream.  These pacthes are
> effectively v2 of patches 3 and 4 from the aforementioned
> "net/rds: Bug fix ports, part 2" [1] set.
> 
> Making teardown block on the bits as locks promotes two latent
> ordering bugs from rare to load-bearing, so they are fixed first:
> 
>   Patch 1: release_in_xmit() checks waitqueue_active() after
>   clear_bit_unlock(), which does not order that read; the wake-up of
>   the (now uninterruptible, untimed) teardown wait can be lost.  Use
>   wq_has_sleeper().
> 
>   Patch 2: rds_conn_path_reset() wipes the whole cp_flags word with a
>   plain store.  Once teardown owns bits in that word across the
>   reset, a blanket store would end lock ownership early - and it
>   already races atomic RMWs on the same word today.  Clear the bits
>   the reset is responsible for individually, as Oracle UEK also does.
> 
> With those in place, patch 3 has rds_conn_shutdown() hold both bit
> locks across the transport shutdown and path reset, and patch 4
> converts rds_tcp_reset_callbacks() from waiting on RDS_IN_XMIT to
> acquiring it, which also serializes the duelling-SYN accept path
> against a concurrent teardown of the same path (the old code could
> resume with a socket pointer the teardown had already released).
> 
> [PATCH net-next 1/4] net/rds: use wq_has_sleeper() in release_in_xmit()
>   Restore full barrier before wake-up checks in release_in_xmit()
> 
> [PATCH net-next 2/4] net/rds: clear cp_flags bits individually in rds_conn_path_reset()
>   Partial port of commit d04896037223 ("net/rds: Preserve essential connection state flags")
>   https://github.com/oracle/linux-uek/commit/d04896037223
> 
> [PATCH net 3/4] net/rds: acquire the fastpath locks in rds_conn_shutdown()
>   Port commit 2b8aaa4f163b ("rds: Make sure transmit path and connection tear-down does not run concurrently")
>   https://github.com/oracle/linux-uek/commit/2b8aaa4f163b 
> 
> [PATCH net 4/4] net/rds: acquire RDS_IN_XMIT in rds_tcp_reset_callbacks()
>   Extend the last port to cover extra call sites in rds_tcp_reset_callbacks()
> 
> Questions and comments appreciated!
> 
> Thanks,
> Allison

Sent v2 to address sashiko complaints
https://lore.kernel.org/all/[email protected]

> 
> [1] https://lore.kernel.org/netdev/[email protected]/
> 
> Allison Henderson (3):
>   net/rds: use wq_has_sleeper() in release_in_xmit()
>   net/rds: clear cp_flags bits individually in rds_conn_path_reset()
>   net/rds: acquire RDS_IN_XMIT in rds_tcp_reset_callbacks()
> 
> Håkon Bugge (1):
>   net/rds: acquire the fastpath locks in rds_conn_shutdown()
> 
>  net/rds/connection.c | 32 +++++++++++++++++++++++++---
>  net/rds/send.c       | 12 ++++++++---
>  net/rds/tcp.c        | 50 +++++++++++++++++++++++++++-----------------
>  3 files changed, 69 insertions(+), 25 deletions(-)
> 
> 
> base-commit: 3da8c3c8b8fa99505624b65ef590482f48e766b6
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.