[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]>
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

[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
-- 
2.25.1
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.