[PATCH net-next 0/9] tcp: avoid struct socket cache line miss in tcp_check_space()

[PATCH net-next 0/9] tcp: avoid struct socket cache line miss in tcp_check_space()

[Eric Dumazet]

tcp_check_space() is called on every incoming ACK and every transmitted
packet, and tests SOCK_NOSPACE in sk->sk_socket->flags.  Because struct
socket lives in its own cache line and the TCP fast paths do not touch
it for anything else, that test pulls in an extra cache line that is
cold when the working set of active sockets is large.

This series mirrors SOCK_NOSPACE into a new u8 field, tp->tcp_nospace,
placed in the tcp_sock_write_txrx cache line group (fitting in an
existing 4-byte hole after pred_flags, so sizeof(struct tcp_sock) is
unchanged).  Both the transmit and receive fast paths already dirty
that cache line, making the fast-path test in tcp_check_space() free of
extra cache misses while keeping __tcp_check_space() authoritative on
SOCK_NOSPACE.

To maintain the invariant (SOCK_NOSPACE set => tp->tcp_nospace set) from
a single choke point:

 - Patch 1 fixes a long-standing bug in dlm where SOCKWQ_ASYNC_NOSPACE
   was tested and cleared on con->sock->flags instead of SOCK_NOSPACE.
 - Patch 2 introduces sk_set_nospace() and sk_clear_nospace() and
   converts the core networking callers.
 - Patches 3-8 convert the remaining in-kernel callers (sunrpc, rds,
   dlm, drbd, nvme-tcp, libceph) so that no open-coded set_bit() or
   clear_bit() of SOCK_NOSPACE remains in the tree.
 - Patch 9 adds tp->tcp_nospace, wires it into sk_set_nospace() and
   sk_clear_nospace(), and switches tcp_check_space() to test it.

Eric Dumazet (9):
  dlm: fix send buffer backpressure handling
  net: add sk_set_nospace() and sk_clear_nospace()
  sunrpc: use sk_set_nospace() and sk_clear_nospace()
  rds: use sk_set_nospace()
  dlm: use sk_set_nospace() and sk_clear_nospace()
  drbd: use sk_set_nospace()
  nvme-tcp: use sk_clear_nospace()
  libceph: use sk_clear_nospace()
  tcp: add tp->tcp_nospace

 .../networking/net_cachelines/tcp_sock.rst    |  1 +
 drivers/block/drbd/drbd_worker.c              |  3 +-
 drivers/nvme/host/tcp.c                       |  2 +-
 drivers/nvme/target/tcp.c                     |  2 +-
 fs/dlm/lowcomms.c                             | 10 ++--
 include/linux/tcp.h                           |  4 ++
 include/net/sock.h                            |  2 +
 include/net/tcp.h                             | 23 ++++++++-
 net/ceph/messenger.c                          |  2 +-
 net/core/sock.c                               | 47 ++++++++++++++++++-
 net/core/stream.c                             |  6 +--
 net/ipv4/tcp.c                                |  5 +-
 net/ipv4/tcp_bpf.c                            |  2 +-
 net/ipv4/tcp_input.c                          |  8 +++-
 net/kcm/kcmsock.c                             |  4 +-
 net/mptcp/protocol.c                          |  4 +-
 net/mptcp/subflow.c                           |  5 ++
 net/rds/tcp_send.c                            |  5 +-
 net/smc/af_smc.c                              |  2 +-
 net/smc/smc_tx.c                              |  6 +--
 net/sunrpc/svcsock.c                          |  4 +-
 net/sunrpc/xprtsock.c                         |  4 +-
 net/tls/tls_sw.c                              |  2 +-
 23 files changed, 116 insertions(+), 37 deletions(-)

-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 1/9] dlm: fix send buffer backpressure handling

[Eric Dumazet]

lowcomms.c tests and clears SOCKWQ_ASYNC_NOSPACE in con->sock->flags,
but this bit has not been stored there for ten years.

Commit 9cd3e072b0be ("net: rename SOCK_ASYNC_NOSPACE and
SOCK_ASYNC_WAITDATA") mechanically renamed the two dlm users, then
commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") moved
the bit from socket->flags to the RCU protected socket_wq->flags, where
it is reachable only through sk_set_bit() and sk_clear_bit(), and is
only maintained for sockets having SOCK_FASYNC set.  dlm uses kernel
sockets, which never have SOCK_FASYNC set, and never sets the bit
itself, so the test in send_to_sock() has been false ever since.

The consequence is that when sock_sendmsg() returns -EAGAIN because the
socket send buffer is full, dlm no longer sets CF_APP_LIMITED, does not
increment sk_write_pending, and does not return DLM_IO_END to wait for
lowcomms_write_space().  It returns DLM_IO_RESCHED instead, and
process_send_sockets() immediately requeues the send work.  A connection
to a peer that is slow to drain thus keeps cycling through
sock_sendmsg() and -EAGAIN, burning CPU, instead of sleeping until TCP
reports that space is available again.

Test SOCK_NOSPACE instead.  This is the bit that lives in socket->flags,
that TCP sets whenever sendmsg() returns -EAGAIN for lack of send buffer
space (tcp_sendmsg_locked() and sk_stream_wait_memory()), and that
lowcomms_write_space() already clears.  This restores the semantics dlm
had before the bit moved.

Also remove the clear_bit() of SOCKWQ_ASYNC_NOSPACE from
lowcomms_write_space(), for the same reason.

SCTP connections are deliberately left as they are.  SCTP does not set
SOCK_NOSPACE, and it never calls sk->sk_write_space(): sctp_wfree() ends
up in sctp_wake_up_waiters(), which calls sctp_write_space() directly.
lowcomms_write_space() is thus never invoked for an SCTP connection, and
the test added here stays false, so send_to_sock() keeps returning
DLM_IO_RESCHED as it does today.  This is the only safe behavior, as
returning DLM_IO_END would wait for a callback that never comes.

Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
Cc: stable@vger.kernel.org
Cc: Alexander Aring <aahringo@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 fs/dlm/lowcomms.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 2aff1c7c17de49c41f9fd02a1fae00bcc9e6af5b..abe9ae4c643fae061d2e168c03219a4f4e1bc007 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -522,10 +522,8 @@ static void lowcomms_write_space(struct sock *sk)
 	clear_bit(SOCK_NOSPACE, &con->sock->flags);
 
 	spin_lock_bh(&con->writequeue_lock);
-	if (test_and_clear_bit(CF_APP_LIMITED, &con->flags)) {
+	if (test_and_clear_bit(CF_APP_LIMITED, &con->flags))
 		con->sock->sk->sk_write_pending--;
-		clear_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags);
-	}
 
 	lowcomms_queue_swork(con);
 	spin_unlock_bh(&con->writequeue_lock);
@@ -1391,7 +1389,7 @@ static int send_to_sock(struct connection *con)
 	if (ret == -EAGAIN || ret == 0) {
 		lock_sock(con->sock->sk);
 		spin_lock_bh(&con->writequeue_lock);
-		if (test_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags) &&
+		if (test_bit(SOCK_NOSPACE, &con->sock->flags) &&
 		    !test_and_set_bit(CF_APP_LIMITED, &con->flags)) {
 			/* Notify TCP that we're limited by the
 			 * application window size.
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 2/9] net: add sk_set_nospace() and sk_clear_nospace()

[Eric Dumazet]

SOCK_NOSPACE lives in sk->sk_socket->flags and is manipulated from
about thirty places in the tree, all of them open coding the
sk->sk_socket dereference, some with a NULL check, some without,
and some reaching the struct socket by yet another path.

Add sk_set_nospace() and sk_clear_nospace() helpers and convert all
the setters and clearers to them, so that "git grep _bit(SOCK_NOSPACE"
only reports the two helpers and the remaining test_bit() sites.

Callers that had no NULL check are all called from user context with a
socket attached, so folding the check into the helpers only makes them
more robust.

No functional change intended.

This is a preparation patch: a following one gives TCP a cheaper
private copy of this bit, and needs a single choke point to keep it
in sync.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/sock.h   |  2 ++
 net/core/sock.c      | 37 +++++++++++++++++++++++++++++++++++--
 net/core/stream.c    |  6 +++---
 net/ipv4/tcp.c       |  4 ++--
 net/ipv4/tcp_bpf.c   |  2 +-
 net/kcm/kcmsock.c    |  4 ++--
 net/mptcp/protocol.c |  4 ++--
 net/smc/af_smc.c     |  2 +-
 net/smc/smc_tx.c     |  6 +++---
 net/tls/tls_sw.c     |  2 +-
 10 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 60ea55dc18854a9759f5df618cc8c904d2323e95..f4dd2e105171386f1b68f808175457c51aff183a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1129,6 +1129,8 @@ static inline void sk_forward_alloc_add(struct sock *sk, int val)
 }
 
 void sk_stream_write_space(struct sock *sk);
+void sk_set_nospace(struct sock *sk);
+void sk_clear_nospace(struct sock *sk);
 
 /* OOB backlog add */
 static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
diff --git a/net/core/sock.c b/net/core/sock.c
index 2948dffcc3e1b49a9a55e30f1380ec165a88859f..11a22aec7e414152aab115e8d11e30067ab3775f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2957,6 +2957,39 @@ void sock_kzfree_s(struct sock *sk, void *mem, int size)
 }
 EXPORT_SYMBOL(sock_kzfree_s);
 
+/**
+ *	sk_set_nospace - tell the transport a writer is waiting for space
+ *	@sk: socket
+ *
+ *	Must be called before the final check of the available send space,
+ *	so that the transport can not miss the request and forget to call
+ *	sk->sk_write_space() once space is available again.
+ */
+void sk_set_nospace(struct sock *sk)
+{
+	struct socket *sock = sk->sk_socket;
+
+	if (sock)
+		set_bit(SOCK_NOSPACE, &sock->flags);
+}
+EXPORT_SYMBOL(sk_set_nospace);
+
+/**
+ *	sk_clear_nospace - tell the transport no writer is waiting for space
+ *	@sk: socket
+ *
+ *	Called from ->sk_write_space() handlers, once send space has been
+ *	made available to writers.
+ */
+void sk_clear_nospace(struct sock *sk)
+{
+	struct socket *sock = sk->sk_socket;
+
+	if (sock)
+		clear_bit(SOCK_NOSPACE, &sock->flags);
+}
+EXPORT_SYMBOL(sk_clear_nospace);
+
 /* It is almost wait_for_tcp_memory minus release_sock/lock_sock.
    I think, these locks should be removed for datagram sockets.
  */
@@ -2970,7 +3003,7 @@ static long sock_wait_for_wmem(struct sock *sk, long timeo)
 			break;
 		if (signal_pending(current))
 			break;
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 		if (refcount_read(&sk->sk_wmem_alloc) < READ_ONCE(sk->sk_sndbuf))
 			break;
@@ -3011,7 +3044,7 @@ struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len,
 			break;
 
 		sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		err = -EAGAIN;
 		if (!timeo)
 			goto failure;
diff --git a/net/core/stream.c b/net/core/stream.c
index 2d748581862d0eda8523f6f371cd2402e1dd6e01..a853b60afdc35a4735c017eb16c748cf2fab1b99 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -37,7 +37,7 @@ void sk_stream_write_space(struct sock *sk)
 	struct socket_wq *wq;
 
 	if (__sk_stream_is_writeable(sk, 1) && sock) {
-		clear_bit(SOCK_NOSPACE, &sock->flags);
+		sk_clear_nospace(sk);
 
 		rcu_read_lock();
 		wq = rcu_dereference(sk->sk_wq);
@@ -143,7 +143,7 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
 		if (sk_stream_memory_free(sk) && !vm_wait)
 			break;
 
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		sk->sk_write_pending++;
 		ret = sk_wait_event(sk, &current_timeo, READ_ONCE(sk->sk_err) ||
 				    (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) ||
@@ -177,7 +177,7 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
 	 * When TCP receives ACK packets that make room, tcp_check_space()
 	 * only calls tcp_new_space() if SOCK_NOSPACE is set.
 	 */
-	set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+	sk_set_nospace(sk);
 	err = -EAGAIN;
 	goto out;
 do_interrupted:
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3ac4856852794736c5d49f042ecd08e4246bdd6d..1cde000cfab4704e6756872f6ddec16851ccc55d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -607,7 +607,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
 				mask |= EPOLLOUT | EPOLLWRNORM;
 			} else {  /* send SIGIO later */
 				sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
-				set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+				sk_set_nospace(sk);
 
 				/* Race breaker. If space is freed after
 				 * wspace test but before the flags are set,
@@ -1398,7 +1398,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 		continue;
 
 wait_for_space:
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		tcp_remove_empty_skb(sk);
 		if (copied)
 			tcp_push(sk, flags & ~MSG_MORE, mss_now,
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 2e234d155b5e616d496611d1f367c34ed90f3000..64d74d92af52d1ca2fa3d24910a254a5092466f2 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -602,7 +602,7 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 			goto out_err;
 		continue;
 wait_for_sndbuf:
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 wait_for_memory:
 		err = sk_stream_wait_memory(sk, &timeo);
 		if (err) {
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 71af69d442f211996e514d8d76a4c9d658f7b352..b29c0ed9caf5220ff741584e49632da45626beb7 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -735,7 +735,7 @@ static void kcm_tx_work(struct work_struct *w)
 	/* Primarily for SOCK_SEQPACKET sockets */
 	if (likely(sk->sk_socket) &&
 	    test_bit(SOCK_NOSPACE, &sk->sk_socket->flags)) {
-		clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_clear_nospace(sk);
 		sk->sk_write_space(sk);
 	}
 
@@ -779,7 +779,7 @@ static int kcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	/* Call the sk_stream functions to manage the sndbuf mem. */
 	if (!sk_stream_memory_free(sk)) {
 		kcm_push(kcm);
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		err = sk_stream_wait_memory(sk, &timeo);
 		if (err)
 			goto out_error;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index e89a69ab927c9139c68c9039327cb0e55c33356b..74b1a512072878a54b229bc80aa76bb3e417c862 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2106,7 +2106,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		continue;
 
 wait_for_memory:
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		__mptcp_push_pending(sk, msg->msg_flags);
 		ret = sk_stream_wait_memory(sk, &timeo);
 		if (ret)
@@ -4472,7 +4472,7 @@ static __poll_t mptcp_check_writeable(struct mptcp_sock *msk)
 	if (__mptcp_stream_is_writeable(sk, 1))
 		return EPOLLOUT | EPOLLWRNORM;
 
-	set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+	sk_set_nospace(sk);
 	smp_mb__after_atomic(); /* NOSPACE is changed by mptcp_write_space() */
 	if (__mptcp_stream_is_writeable(sk, 1))
 		return EPOLLOUT | EPOLLWRNORM;
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e9f93b3ab435bbcce0a493f24205bd91bdf15b6a..4f2afbbdef2d7e62431e14562dc26bdfcdaa9f79 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -2919,7 +2919,7 @@ __poll_t smc_poll(struct file *file, struct socket *sock,
 				mask |= EPOLLOUT | EPOLLWRNORM;
 			} else {
 				sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
-				set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+				sk_set_nospace(sk);
 
 				if (sk->sk_state != SMC_INIT) {
 					/* Race breaker the same way as tcp_poll(). */
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3144b4b1fe29013cabc63d72671d256761a4c367..52e5395484fd3e0c4b1356226ecdcf4fb190d487 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -48,7 +48,7 @@ static void smc_tx_write_space(struct sock *sk)
 	if (atomic_read(&smc->conn.sndbuf_space) && sock) {
 		if (test_bit(SOCK_NOSPACE, &sock->flags))
 			SMC_STAT_RMB_TX_FULL(smc, !smc->conn.lnk);
-		clear_bit(SOCK_NOSPACE, &sock->flags);
+		sk_clear_nospace(sk);
 		rcu_read_lock();
 		wq = rcu_dereference(sk->sk_wq);
 		if (skwq_has_sleeper(wq))
@@ -100,7 +100,7 @@ static int smc_tx_wait(struct smc_sock *smc, int flags)
 		}
 		if (!timeo) {
 			/* ensure EPOLLOUT is subsequently generated */
-			set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+			sk_set_nospace(sk);
 			rc = -EAGAIN;
 			break;
 		}
@@ -111,7 +111,7 @@ static int smc_tx_wait(struct smc_sock *smc, int flags)
 		sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
 		if (atomic_read(&conn->sndbuf_space) && !conn->urg_tx_pend)
 			break; /* at least 1 byte of free & no urgent data */
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		sk_wait_event(sk, &timeo,
 			      READ_ONCE(sk->sk_err) ||
 			      (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) ||
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d1ad31986cf2cee88afbde43a2908791eeabb0fb..312e51270f293dc4df8efbec498473c4c3b48d15 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -966,7 +966,7 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
 		continue;
 
 wait_for_sndbuf:
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 wait_for_memory:
 		ret = sk_stream_wait_memory(sk, &timeo);
 		if (ret) {
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 3/9] sunrpc: use sk_set_nospace() and sk_clear_nospace()

[Eric Dumazet]

Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.

No functional change intended.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sunrpc/svcsock.c  | 4 ++--
 net/sunrpc/xprtsock.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 50e5e7f5b762de28c34d0f58cb0c6feb1e3ce79f..2d8cdee0798d540ba8eae589deaac5bd3ff889fa 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -792,11 +792,11 @@ static int svc_udp_has_wspace(struct svc_xprt *xprt)
 	 * Set the SOCK_NOSPACE flag before checking the available
 	 * sock space.
 	 */
-	set_bit(SOCK_NOSPACE, &svsk->sk_sock->flags);
+	sk_set_nospace(svsk->sk_sk);
 	required = atomic_read(&svsk->sk_xprt.xpt_reserved) + serv->sv_max_mesg;
 	if (required*2 > sock_wspace(svsk->sk_sk))
 		return 0;
-	clear_bit(SOCK_NOSPACE, &svsk->sk_sock->flags);
+	sk_clear_nospace(svsk->sk_sk);
 	return 1;
 }
 
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 7f60723fa64d84887e260e5e130e44cdbec6f850..b97c70c12f627630509510facfd3c3cf96ab6cb6 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -858,7 +858,7 @@ static int xs_nospace(struct rpc_rqst *req, struct sock_xprt *transport)
 	if (xprt_connected(xprt)) {
 		/* wait for more buffer space */
 		set_bit(XPRT_SOCK_NOSPACE, &transport->sock_state);
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_set_nospace(sk);
 		sk->sk_write_pending++;
 		xprt_wait_for_buffer_space(xprt);
 	} else
@@ -1615,7 +1615,7 @@ static void xs_write_space(struct sock *sk)
 
 	if (!sk->sk_socket)
 		return;
-	clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+	sk_clear_nospace(sk);
 
 	if (unlikely(!(xprt = xprt_from_sock(sk))))
 		return;
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 4/9] rds: use sk_set_nospace()

[Eric Dumazet]

Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.

No functional change intended.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/rds/tcp_send.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/rds/tcp_send.c b/net/rds/tcp_send.c
index 7c52acc749cf4d4003c749a2646c2b4f87f207fe..824222c475119d91e333123c5afc9291005a8696 100644
--- a/net/rds/tcp_send.c
+++ b/net/rds/tcp_send.c
@@ -100,7 +100,7 @@ int rds_tcp_xmit(struct rds_connection *conn, struct rds_message *rm,
 
 	if (hdr_off < sizeof(struct rds_header)) {
 		/* see rds_tcp_write_space() */
-		set_bit(SOCK_NOSPACE, &tc->t_sock->sk->sk_socket->flags);
+		sk_set_nospace(tc->t_sock->sk);
 
 		ret = rds_tcp_sendmsg(tc->t_sock,
 				      (void *)&rm->m_inc.i_hdr + hdr_off,
@@ -221,6 +221,5 @@ void rds_tcp_write_space(struct sock *sk)
 	 */
 	write_space(sk);
 
-	if (sk->sk_socket)
-		set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+	sk_set_nospace(sk);
 }
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 5/9] dlm: use sk_set_nospace() and sk_clear_nospace()

[Eric Dumazet]

Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.

No functional change intended.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 fs/dlm/lowcomms.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index abe9ae4c643fae061d2e168c03219a4f4e1bc007..6aa68a24acab574429664acc3f5ecc25ddd1553d 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -519,7 +519,7 @@ static void lowcomms_write_space(struct sock *sk)
 {
 	struct connection *con = sock2con(sk);
 
-	clear_bit(SOCK_NOSPACE, &con->sock->flags);
+	sk_clear_nospace(sk);
 
 	spin_lock_bh(&con->writequeue_lock);
 	if (test_and_clear_bit(CF_APP_LIMITED, &con->flags))
@@ -1394,7 +1394,7 @@ static int send_to_sock(struct connection *con)
 			/* Notify TCP that we're limited by the
 			 * application window size.
 			 */
-			set_bit(SOCK_NOSPACE, &con->sock->sk->sk_socket->flags);
+			sk_set_nospace(con->sock->sk);
 			con->sock->sk->sk_write_pending++;
 
 			clear_bit(CF_SEND_PENDING, &con->flags);
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 6/9] drbd: use sk_set_nospace()

[Eric Dumazet]

Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.

No functional change intended.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/block/drbd/drbd_worker.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index 0697f99fed18d7e70f00b7c559a131517ee8aa42..8e50131b5a5692a8dfb99eafe706602153be94b7 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -635,8 +635,7 @@ static int make_resync_request(struct drbd_peer_device *const peer_device, int c
 			int sndbuf = sk->sk_sndbuf;
 			if (queued > sndbuf / 2) {
 				requeue = 1;
-				if (sk->sk_socket)
-					set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+				sk_set_nospace(sk);
 			}
 		} else
 			requeue = 1;
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 7/9] nvme-tcp: use sk_clear_nospace()

[Eric Dumazet]

Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.

No functional change intended.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/nvme/host/tcp.c   | 2 +-
 drivers/nvme/target/tcp.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 921934028e0b22c2e4c9fd60d09ed8e50051262d..b853ca2d598fa4cb8ea9cb66513f1e1070cd1fb4 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1133,7 +1133,7 @@ static void nvme_tcp_write_space(struct sock *sk)
 	read_lock_bh(&sk->sk_callback_lock);
 	queue = sk->sk_user_data;
 	if (likely(queue && sk_stream_is_writeable(sk))) {
-		clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_clear_nospace(sk);
 		/* Ensure pending TLS partial records are retried */
 		if (nvme_tcp_queue_tls(queue))
 			queue->write_space(sk);
diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index e598101752620448423925d2385b606e0b4346e8..c38ea3501ea5def168b6f180b56734b99d21bdea 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1686,7 +1686,7 @@ static void nvmet_tcp_write_space(struct sock *sk)
 	}
 
 	if (sk_stream_is_writeable(sk)) {
-		clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+		sk_clear_nospace(sk);
 		queue_work_on(queue_cpu(queue), nvmet_tcp_wq, &queue->io_work);
 	}
 out:
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 8/9] libceph: use sk_clear_nospace()

[Eric Dumazet]

Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.

No functional change intended.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ceph/messenger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 212e7797f9e48edde4a4536bba6f1ff293628b5b..cc7db8c61db37a998b698566caa7c936fa83fe4a 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -375,7 +375,7 @@ static void ceph_sock_write_space(struct sock *sk)
 	if (ceph_con_flag_test(con, CEPH_CON_F_WRITE_PENDING)) {
 		if (sk_stream_is_writeable(sk)) {
 			dout("%s %p queueing write work\n", __func__, con);
-			clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+			sk_clear_nospace(sk);
 			queue_con(con);
 		}
 	} else {
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net-next 9/9] tcp: add tp->tcp_nospace

[Eric Dumazet]

tcp_check_space() runs for every incoming ACK and for every packet we
send, and reads SOCK_NOSPACE from sk->sk_socket->flags.

struct socket lives in its own cache line, which the TCP fast paths do
not otherwise touch, so testing sk->sk_socket->flags pulls in an extra
cache line that is cold when the working set is large.

Add tp->tcp_nospace, a mirror of SOCK_NOSPACE placed in the
tcp_sock_write_txrx group, that is in a cache line both the transmit
and the receive paths already have to touch.  It fits in an existing
hole, sizeof(struct tcp_sock) is unchanged.

The two flags are now only changed from sk_set_nospace() and
sk_clear_nospace(), which maintain this invariant:

	SOCK_NOSPACE set  =>  tp->tcp_nospace set

tcp_check_space() can thus test tp->tcp_nospace alone and leave the
authoritative SOCK_NOSPACE test to __tcp_check_space().  The
invariant is one directional on purpose: a stale tp->tcp_nospace only
costs an extra call to __tcp_check_space(), which is what we do
unconditionally today, while a stale SOCK_NOSPACE would cost a
missed EPOLLOUT.

Note the smp_mb() is kept.  tcp_poll() sets the flag without the
socket lock, and the store-buffer pattern it forms with
tcp_check_space() needs a full barrier on both sides.

MPTCP subflows share the struct socket of their parent, hence its
SOCK_NOSPACE bit, which can not be mirrored in the subflow tcp_sock.
Pin their tp->tcp_nospace in subflow_ulp_init(), so that they always
reach __tcp_check_space() and keep the current behavior.

Microbenchmark on an AMD EPYC 7B13, 64 threads, each thread calling
tcp_check_space() in a loop over a private set of sockets.  Numbers
are cycles per call above a baseline loop that performs the work the
callers already did (the cache lines tcp_write_xmit() and
tcp_clean_rtx_queue() touched) but not tcp_check_space() itself, so
they are the marginal cost of the function.  Median of 11 runs.
The set size controls whether struct socket is still cached:

    sockets/thread	before	 after
    1			  +0.6	  +0.7
    256    (64 KB)	  +8.3	  +3.9
    4096   (1 MB)	 +22.4	  +1.1
    262144 (64 MB)	 +48.4	  -0.8

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 .../networking/net_cachelines/tcp_sock.rst    |  1 +
 include/linux/tcp.h                           |  4 ++++
 include/net/tcp.h                             | 23 ++++++++++++++++++-
 net/core/sock.c                               | 18 +++++++++++----
 net/ipv4/tcp.c                                |  1 +
 net/ipv4/tcp_input.c                          |  8 ++++++-
 net/mptcp/subflow.c                           |  5 ++++
 7 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/net_cachelines/tcp_sock.rst b/Documentation/networking/net_cachelines/tcp_sock.rst
index 0f6088c4ab8bb872e7fc86f02479592e84c0247a..3f225cdf12983d00c5347cd3ab16049b14b59047 100644
--- a/Documentation/networking/net_cachelines/tcp_sock.rst
+++ b/Documentation/networking/net_cachelines/tcp_sock.rst
@@ -12,6 +12,7 @@ struct inet_connection_sock   inet_conn
 u16                           tcp_header_len          read_mostly         read_mostly         tcp_bound_to_half_wnd,tcp_current_mss(tx);tcp_rcv_established(rx)
 u16                           gso_segs                read_mostly                             tcp_xmit_size_goal
 __be32                        pred_flags              read_write          read_mostly         tcp_select_window(tx);tcp_rcv_established(rx)
+u8                            tcp_nospace             read_mostly         read_mostly         tcp_check_space(tx);tcp_check_space(rx)
 u64                           bytes_received                              read_write          tcp_rcv_nxt_update(rx)
 u32                           segs_in                 read_write          read_write          tcp_segs_in(),tcp_v6_rcv(rx),tcp_v4_rcv()
 u32                           data_segs_in                                read_write          tcp_v6_rcv(rx)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 6a8c77719322f9caee305d954a107892c76d4ef7..d51aae60aa45bba31ae2f06b004383733caaf599 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -307,6 +307,10 @@ struct tcp_sock {
 		accecn_opt_demand:2,/* Demand AccECN option for n next ACKs */
 		prev_ecnfield:2; /* ECN bits from the previous segment */
 	__be32	pred_flags;
+	u8	tcp_nospace;	/* mirrors SOCK_NOSPACE, but in a cache line
+				 * that tcp_check_space() already needs.
+				 * Can only be set if SOCK_NOSPACE is set.
+				 */
 	u64	tcp_clock_cache; /* cache last tcp_clock_ns() (see tcp_mstamp_refresh()) */
 	u64	tcp_mstamp;	/* most recent packet received/sent */
 	u32	rcv_nxt;	/* What we want to receive next		*/
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 5e5f5f9b89a386568fc5efebfa3d3c7e1ff62683..1e1950dd184ec3daecd5f691a0c099382e873937 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -783,14 +783,35 @@ void tcp_done_with_error(struct sock *sk, int err);
 void tcp_reset(struct sock *sk, struct sk_buff *skb);
 void tcp_fin(struct sock *sk);
 void __tcp_check_space(struct sock *sk);
+
+/* Mirror of SOCK_NOSPACE in tcp_sock, maintained by sk_set_nospace()
+ * and sk_clear_nospace().
+ *
+ * MPTCP subflows share the parent socket, and thus its SOCK_NOSPACE bit.
+ * Keep their mirror always set (see subflow_ulp_init()) so that they
+ * always reach __tcp_check_space() and behave as before.
+ */
+static inline void tcp_set_nospace(struct sock *sk)
+{
+	if (sk_is_tcp(sk))
+		WRITE_ONCE(tcp_sk(sk)->tcp_nospace, 1);
+}
+
+static inline void tcp_clear_nospace(struct sock *sk)
+{
+	if (sk_is_tcp(sk) && !sk_is_mptcp(sk))
+		WRITE_ONCE(tcp_sk(sk)->tcp_nospace, 0);
+}
+
 static inline void tcp_check_space(struct sock *sk)
 {
 	/* pairs with tcp_poll() */
 	smp_mb();
 
-	if (sk->sk_socket && test_bit(SOCK_NOSPACE, &sk->sk_socket->flags))
+	if (unlikely(READ_ONCE(tcp_sk(sk)->tcp_nospace)))
 		__tcp_check_space(sk);
 }
+
 void tcp_sack_compress_send_ack(struct sock *sk);
 
 static inline void tcp_cleanup_skb(struct sk_buff *skb)
diff --git a/net/core/sock.c b/net/core/sock.c
index 11a22aec7e414152aab115e8d11e30067ab3775f..dce4e8e4e2bbef710eae641a270395a315bef56e 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2969,8 +2969,13 @@ void sk_set_nospace(struct sock *sk)
 {
 	struct socket *sock = sk->sk_socket;
 
-	if (sock)
-		set_bit(SOCK_NOSPACE, &sock->flags);
+	if (!sock)
+		return;
+	/* Mirror first: callers relying on the barrier implied by
+	 * set_bit() + smp_mb__after_atomic() are then also covered.
+	 */
+	tcp_set_nospace(sk);
+	set_bit(SOCK_NOSPACE, &sock->flags);
 }
 EXPORT_SYMBOL(sk_set_nospace);
 
@@ -2985,8 +2990,13 @@ void sk_clear_nospace(struct sock *sk)
 {
 	struct socket *sock = sk->sk_socket;
 
-	if (sock)
-		clear_bit(SOCK_NOSPACE, &sock->flags);
+	if (!sock)
+		return;
+	clear_bit(SOCK_NOSPACE, &sock->flags);
+	/* Mirror last: a stale mirror only costs a slow path, while a
+	 * stale SOCK_NOSPACE would cost a missed EPOLLOUT.
+	 */
+	tcp_clear_nospace(sk);
 }
 EXPORT_SYMBOL(sk_clear_nospace);
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1cde000cfab4704e6756872f6ddec16851ccc55d..92728e4c1e3df928cc7aa1bcbcf93fb15afaa28e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -5261,6 +5261,7 @@ static void __init tcp_struct_check(void)
 
 	/* TXRX read-write hotpath cache lines */
 	CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx, pred_flags);
+	CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx, tcp_nospace);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx, tcp_clock_cache);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx, tcp_mstamp);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx, rcv_nxt);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 92bc60716f33d81e9ce90d9de2e5d989ba71c8a3..914d708326bf841641a4519341bfd77deba9e7e0 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6098,8 +6098,14 @@ static void tcp_new_space(struct sock *sk)
  */
 void __tcp_check_space(struct sock *sk)
 {
+	struct socket *sock = sk->sk_socket;
+
+	/* tp->tcp_nospace is only a hint, SOCK_NOSPACE is authoritative. */
+	if (!sock || !test_bit(SOCK_NOSPACE, &sock->flags))
+		return;
+
 	tcp_new_space(sk);
-	if (!test_bit(SOCK_NOSPACE, &sk->sk_socket->flags))
+	if (!test_bit(SOCK_NOSPACE, &sock->flags))
 		tcp_chrono_stop(sk, TCP_CHRONO_SNDBUF_LIMITED);
 }
 
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index f0a6725d2c3762def75c879e769e9da37f92215a..e297c88be503b42979886baaf43acfb6b7e78055 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -2001,6 +2001,11 @@ static int subflow_ulp_init(struct sock *sk)
 	pr_debug("subflow=%p, family=%d\n", ctx, sk->sk_family);
 
 	tp->is_mptcp = 1;
+	/* Subflows share the MPTCP socket, and thus its SOCK_NOSPACE bit,
+	 * which tcp_check_space() can not mirror. Pin the mirror so that
+	 * __tcp_check_space() always tests the shared bit.
+	 */
+	tp->tcp_nospace = 1;
 	ctx->icsk_af_ops = icsk->icsk_af_ops;
 	icsk->icsk_af_ops = subflow_default_af_ops(sk);
 	ctx->tcp_state_change = sk->sk_state_change;
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH net-next 1/9] dlm: fix send buffer backpressure handling

[Alexander Aring]

Hi,

On Tue, Sep 22, 2026 at 8:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> lowcomms.c tests and clears SOCKWQ_ASYNC_NOSPACE in con->sock->flags,
> but this bit has not been stored there for ten years.
>
> Commit 9cd3e072b0be ("net: rename SOCK_ASYNC_NOSPACE and
> SOCK_ASYNC_WAITDATA") mechanically renamed the two dlm users, then
> commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") moved
> the bit from socket->flags to the RCU protected socket_wq->flags, where
> it is reachable only through sk_set_bit() and sk_clear_bit(), and is
> only maintained for sockets having SOCK_FASYNC set.  dlm uses kernel
> sockets, which never have SOCK_FASYNC set, and never sets the bit
> itself, so the test in send_to_sock() has been false ever since.
>
> The consequence is that when sock_sendmsg() returns -EAGAIN because the
> socket send buffer is full, dlm no longer sets CF_APP_LIMITED, does not
> increment sk_write_pending, and does not return DLM_IO_END to wait for
> lowcomms_write_space().  It returns DLM_IO_RESCHED instead, and
> process_send_sockets() immediately requeues the send work.  A connection
> to a peer that is slow to drain thus keeps cycling through
> sock_sendmsg() and -EAGAIN, burning CPU, instead of sleeping until TCP
> reports that space is available again.
>
> Test SOCK_NOSPACE instead.  This is the bit that lives in socket->flags,
> that TCP sets whenever sendmsg() returns -EAGAIN for lack of send buffer
> space (tcp_sendmsg_locked() and sk_stream_wait_memory()), and that
> lowcomms_write_space() already clears.  This restores the semantics dlm
> had before the bit moved.
>
> Also remove the clear_bit() of SOCKWQ_ASYNC_NOSPACE from
> lowcomms_write_space(), for the same reason.
>
> SCTP connections are deliberately left as they are.  SCTP does not set
> SOCK_NOSPACE, and it never calls sk->sk_write_space(): sctp_wfree() ends
> up in sctp_wake_up_waiters(), which calls sctp_write_space() directly.
> lowcomms_write_space() is thus never invoked for an SCTP connection, and
> the test added here stays false, so send_to_sock() keeps returning
> DLM_IO_RESCHED as it does today.  This is the only safe behavior, as
> returning DLM_IO_END would wait for a callback that never comes.
>
> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
> Cc: stable@vger.kernel.org
> Cc: Alexander Aring <aahringo@redhat.com>
> Cc: David Teigland <teigland@redhat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>


Acked-by: Alexander Aring <aahringo@redhat.com>

thanks for looking into that!

- Alex

Re: [PATCH net-next 1/9] dlm: fix send buffer backpressure handling

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> lowcomms.c tests and clears SOCKWQ_ASYNC_NOSPACE in con->sock->flags,
> but this bit has not been stored there for ten years.
>
> Commit 9cd3e072b0be ("net: rename SOCK_ASYNC_NOSPACE and
> SOCK_ASYNC_WAITDATA") mechanically renamed the two dlm users, then
> commit ceb5d58b2170 ("net: fix sock_wake_async() rcu protection") moved
> the bit from socket->flags to the RCU protected socket_wq->flags, where
> it is reachable only through sk_set_bit() and sk_clear_bit(), and is
> only maintained for sockets having SOCK_FASYNC set.  dlm uses kernel
> sockets, which never have SOCK_FASYNC set, and never sets the bit
> itself, so the test in send_to_sock() has been false ever since.
>
> The consequence is that when sock_sendmsg() returns -EAGAIN because the
> socket send buffer is full, dlm no longer sets CF_APP_LIMITED, does not
> increment sk_write_pending, and does not return DLM_IO_END to wait for
> lowcomms_write_space().  It returns DLM_IO_RESCHED instead, and
> process_send_sockets() immediately requeues the send work.  A connection
> to a peer that is slow to drain thus keeps cycling through
> sock_sendmsg() and -EAGAIN, burning CPU, instead of sleeping until TCP
> reports that space is available again.
>
> Test SOCK_NOSPACE instead.  This is the bit that lives in socket->flags,
> that TCP sets whenever sendmsg() returns -EAGAIN for lack of send buffer
> space (tcp_sendmsg_locked() and sk_stream_wait_memory()), and that
> lowcomms_write_space() already clears.  This restores the semantics dlm
> had before the bit moved.
>
> Also remove the clear_bit() of SOCKWQ_ASYNC_NOSPACE from
> lowcomms_write_space(), for the same reason.
>
> SCTP connections are deliberately left as they are.  SCTP does not set
> SOCK_NOSPACE, and it never calls sk->sk_write_space(): sctp_wfree() ends
> up in sctp_wake_up_waiters(), which calls sctp_write_space() directly.
> lowcomms_write_space() is thus never invoked for an SCTP connection, and
> the test added here stays false, so send_to_sock() keeps returning
> DLM_IO_RESCHED as it does today.  This is the only safe behavior, as
> returning DLM_IO_END would wait for a callback that never comes.
>
> Fixes: ceb5d58b2170 ("net: fix sock_wake_async() rcu protection")
> Cc: stable@vger.kernel.org
> Cc: Alexander Aring <aahringo@redhat.com>
> Cc: David Teigland <teigland@redhat.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 2/9] net: add sk_set_nospace() and sk_clear_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> SOCK_NOSPACE lives in sk->sk_socket->flags and is manipulated from
> about thirty places in the tree, all of them open coding the
> sk->sk_socket dereference, some with a NULL check, some without,
> and some reaching the struct socket by yet another path.
>
> Add sk_set_nospace() and sk_clear_nospace() helpers and convert all
> the setters and clearers to them, so that "git grep _bit(SOCK_NOSPACE"
> only reports the two helpers and the remaining test_bit() sites.
>
> Callers that had no NULL check are all called from user context with a
> socket attached, so folding the check into the helpers only makes them
> more robust.
>
> No functional change intended.
>
> This is a preparation patch: a following one gives TCP a cheaper
> private copy of this bit, and needs a single choke point to keep it
> in sync.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 3/9] sunrpc: use sk_set_nospace() and sk_clear_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use the new helpers instead of open coding the SOCK_NOSPACE
> manipulation, so that TCP can later maintain a cheaper private
> copy of this bit.
>
> No functional change intended.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 4/9] rds: use sk_set_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use the new helpers instead of open coding the SOCK_NOSPACE
> manipulation, so that TCP can later maintain a cheaper private
> copy of this bit.
>
> No functional change intended.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 5/9] dlm: use sk_set_nospace() and sk_clear_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use the new helpers instead of open coding the SOCK_NOSPACE
> manipulation, so that TCP can later maintain a cheaper private
> copy of this bit.
>
> No functional change intended.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 6/9] drbd: use sk_set_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use the new helpers instead of open coding the SOCK_NOSPACE
> manipulation, so that TCP can later maintain a cheaper private
> copy of this bit.
>
> No functional change intended.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 7/9] nvme-tcp: use sk_clear_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use the new helpers instead of open coding the SOCK_NOSPACE
> manipulation, so that TCP can later maintain a cheaper private
> copy of this bit.
>
> No functional change intended.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 8/9] libceph: use sk_clear_nospace()

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Use the new helpers instead of open coding the SOCK_NOSPACE
> manipulation, so that TCP can later maintain a cheaper private
> copy of this bit.
>
> No functional change intended.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 9/9] tcp: add tp->tcp_nospace

[Kuniyuki Iwashima]

On Tue, Sep 22, 2026 at 5:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> tcp_check_space() runs for every incoming ACK and for every packet we
> send, and reads SOCK_NOSPACE from sk->sk_socket->flags.
>
> struct socket lives in its own cache line, which the TCP fast paths do
> not otherwise touch, so testing sk->sk_socket->flags pulls in an extra
> cache line that is cold when the working set is large.
>
> Add tp->tcp_nospace, a mirror of SOCK_NOSPACE placed in the
> tcp_sock_write_txrx group, that is in a cache line both the transmit
> and the receive paths already have to touch.  It fits in an existing
> hole, sizeof(struct tcp_sock) is unchanged.
>
> The two flags are now only changed from sk_set_nospace() and
> sk_clear_nospace(), which maintain this invariant:
>
>         SOCK_NOSPACE set  =>  tp->tcp_nospace set
>
> tcp_check_space() can thus test tp->tcp_nospace alone and leave the
> authoritative SOCK_NOSPACE test to __tcp_check_space().  The
> invariant is one directional on purpose: a stale tp->tcp_nospace only
> costs an extra call to __tcp_check_space(), which is what we do
> unconditionally today, while a stale SOCK_NOSPACE would cost a
> missed EPOLLOUT.
>
> Note the smp_mb() is kept.  tcp_poll() sets the flag without the
> socket lock, and the store-buffer pattern it forms with
> tcp_check_space() needs a full barrier on both sides.
>
> MPTCP subflows share the struct socket of their parent, hence its
> SOCK_NOSPACE bit, which can not be mirrored in the subflow tcp_sock.
> Pin their tp->tcp_nospace in subflow_ulp_init(), so that they always
> reach __tcp_check_space() and keep the current behavior.
>
> Microbenchmark on an AMD EPYC 7B13, 64 threads, each thread calling
> tcp_check_space() in a loop over a private set of sockets.  Numbers
> are cycles per call above a baseline loop that performs the work the
> callers already did (the cache lines tcp_write_xmit() and
> tcp_clean_rtx_queue() touched) but not tcp_check_space() itself, so
> they are the marginal cost of the function.  Median of 11 runs.
> The set size controls whether struct socket is still cached:
>
>     sockets/thread      before   after
>     1                     +0.6    +0.7
>     256    (64 KB)        +8.3    +3.9
>     4096   (1 MB)        +22.4    +1.1
>     262144 (64 MB)       +48.4    -0.8
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net-next 1/9] dlm: fix send buffer backpressure handling

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] fs/dlm/lowcomms.c:send_to_sock() — making the CF_APP_LIMITED
  branch live for the first time in ten years introduces a permanent…
- [Low] fs/dlm/lowcomms.c:send_to_sock(): after switching the guard to
  SOCK_NOSPACE, the guard and the "Notify TCP" action now operate on the…

--

dlm: fix send buffer backpressure handling

The send path in fs/dlm/lowcomms.c tested SOCKWQ_ASYNC_NOSPACE in
con->sock->flags, a bit that has not lived there since commit ceb5d58b2170,
so the test was always false and send_to_sock() always returned
DLM_IO_RESCHED on -EAGAIN. The patch switches the test to SOCK_NOSPACE and
drops the matching clear_bit() in lowcomms_write_space().

> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 2aff1c7c17de4..abe9ae4c643fa 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -522,10 +522,8 @@ static void lowcomms_write_space(struct sock *sk)
>  	clear_bit(SOCK_NOSPACE, &con->sock->flags);
>  
>  	spin_lock_bh(&con->writequeue_lock);
> -	if (test_and_clear_bit(CF_APP_LIMITED, &con->flags)) {
> +	if (test_and_clear_bit(CF_APP_LIMITED, &con->flags))
>  		con->sock->sk->sk_write_pending--;
> -		clear_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags);
> -	}
>  
>  	lowcomms_queue_swork(con);
>  	spin_unlock_bh(&con->writequeue_lock);
> @@ -1391,7 +1389,7 @@ static int send_to_sock(struct connection *con)
>  	if (ret == -EAGAIN || ret == 0) {
>  		lock_sock(con->sock->sk);
>  		spin_lock_bh(&con->writequeue_lock);
> -		if (test_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags) &&
> +		if (test_bit(SOCK_NOSPACE, &con->sock->flags) &&
>  		    !test_and_set_bit(CF_APP_LIMITED, &con->flags)) {

[Severity: High]
This makes the CF_APP_LIMITED branch reachable for the first time in ten
years.  Once it is taken, is there anything left that can restart this
connection if the TCP connection then dies?

send_to_sock() parks the send worker:

	if (test_bit(SOCK_NOSPACE, &con->sock->flags) &&
	    !test_and_set_bit(CF_APP_LIMITED, &con->flags)) {
		...
		con->sock->sk->sk_write_pending++;
		clear_bit(CF_SEND_PENDING, &con->flags);
		...
		/* wait for write_space() event */
		return DLM_IO_END;
	}

process_send_sockets() just breaks on DLM_IO_END, so nothing is queued on
io_workqueue for this connection any more.

The only place that clears CF_APP_LIMITED on a live socket is
lowcomms_write_space(), and that is only reached from TCP ACK processing
(tcp_check_space() -> tcp_new_space() -> sk->sk_write_space()).

If the peer stops acking, tcp_reset() or tcp_write_err() end up in
tcp_done_with_error(), which purges the write queue, calls tcp_done() and
sk_error_report().  None of those call sk->sk_write_space(), and an RST
does not produce an sk_data_ready() either, so neither worker is woken.

Every remaining restart point goes through lowcomms_queue_swork(), which
refuses to queue while CF_APP_LIMITED is set:

	if (!test_bit(CF_IO_STOP, &con->flags) &&
	    !test_bit(CF_APP_LIMITED, &con->flags) &&
	    !test_and_set_bit(CF_SEND_PENDING, &con->flags))
		queue_work(io_workqueue, &con->swork);

That gate covers _dlm_lowcomms_commit_msg() for new messages, the
retransmit path lowcomms_error_report() -> dlm_midcomms_unack_msg_resend()
-> dlm_lowcomms_resend_msg() -> _dlm_lowcomms_commit_msg(), and the
receive-side error recovery in process_recv_sockets():

		if (ret < 0) {
			if (test_bit(CF_IS_OTHERCON, &con->flags)) {
				close_connection(con, false);
			} else {
				spin_lock_bh(&con->writequeue_lock);
				lowcomms_queue_swork(con);
				spin_unlock_bh(&con->writequeue_lock);
			}

lowcomms_error_report() only logs, asks for a retransmit and forwards to
the default error report; it neither clears CF_APP_LIMITED nor closes the
connection.  dlm_lowcomms_connect_node() only queues swork when
con->sock == NULL, and the dead socket is never closed, so con->sock stays
non-NULL.

Before this change the (dead) test meant send_to_sock() returned
DLM_IO_RESCHED, swork was requeued, the next sock_sendmsg() picked up the
socket error and the ret < 0 path did close_connection() plus
lowcomms_queue_swork().  So the CPU-burning loop the patch removes was
also the error detection and reconnect mechanism.

Would it make sense to add a fallback that clears CF_APP_LIMITED or closes
the connection when sk_err is set, so a peer RST or retransmission timeout
during the app-limited state does not leave the connection permanently
silent?

>  			/* Notify TCP that we're limited by the
>  			 * application window size.

[Severity: Low]
With the test switched to SOCK_NOSPACE, does the body of this branch still
do anything?  The guard and the notification now touch the same bit in the
same word:

	if (test_bit(SOCK_NOSPACE, &con->sock->flags) &&
	    !test_and_set_bit(CF_APP_LIMITED, &con->flags)) {
		/* Notify TCP that we're limited by the
		 * application window size.
		 */
		set_bit(SOCK_NOSPACE, &con->sock->sk->sk_socket->flags);

For the kernel sockets dlm creates, sk->sk_socket == con->sock, which is
also how lowcomms_write_space() spells it:

	clear_bit(SOCK_NOSPACE, &con->sock->flags);

lock_sock(con->sock->sk) is held across the test and the set, so the bit
cannot be cleared in between, and the set_bit() can never change state.
The comment about notifying TCP then describes something the code no
longer does.

The two different spellings of one flags word inside a single if statement
are the same kind of trap that produced the dead test this patch fixes, so
perhaps the set_bit() and its comment could be dropped, or at least
spelled consistently as con->sock->flags.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922122721.3568295-1-edumazet%40google.com

Re: [PATCH net-next 2/9] net: add sk_set_nospace() and sk_clear_nospace()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Changelog overstatement at this commit only: the commit message
  of "net: add sk_set_nospace() and sk_clear_nospace()" claims it…

--

net: add sk_set_nospace() and sk_clear_nospace()

This adds two helpers in net/core/sock.c that fold the sk->sk_socket
dereference and its NULL check into a single place, and converts the
SOCK_NOSPACE setters and clearers in core, tcp, kcm, mptcp, smc and tls
over to them.  It is described as a preparation patch for a later one
that gives TCP a private mirror of the bit.

> Add sk_set_nospace() and sk_clear_nospace() helpers and convert all
> the setters and clearers to them, so that "git grep _bit(SOCK_NOSPACE"
> only reports the two helpers and the remaining test_bit() sites.

[Severity: Low]
Is this description accurate for this patch on its own?  At this commit
the grep still reports twelve open-coded sites outside the two helpers:

  drivers/block/drbd/drbd_worker.c  set_bit(SOCK_NOSPACE, &sk->sk_socket->flags)
  drivers/nvme/host/tcp.c           clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags)
  drivers/nvme/target/tcp.c         clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags)
  fs/dlm/lowcomms.c                 clear_bit(SOCK_NOSPACE, &con->sock->flags)
  fs/dlm/lowcomms.c                 set_bit(SOCK_NOSPACE, &con->sock->sk->sk_socket->flags)
  net/ceph/messenger.c              clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags)
  net/rds/tcp_send.c                set_bit(SOCK_NOSPACE, &tc->t_sock->sk->sk_socket->flags)
  net/rds/tcp_send.c                set_bit(SOCK_NOSPACE, &sk->sk_socket->flags)
  net/sunrpc/svcsock.c              set_bit(SOCK_NOSPACE, &svsk->sk_sock->flags)
  net/sunrpc/svcsock.c              clear_bit(SOCK_NOSPACE, &svsk->sk_sock->flags)
  net/sunrpc/xprtsock.c             set_bit(SOCK_NOSPACE, &sk->sk_socket->flags)
  net/sunrpc/xprtsock.c             clear_bit(SOCK_NOSPACE, &sk->sk_socket->flags)

The remaining sites are all converted by later patches in the series
("sunrpc: use sk_set_nospace() and sk_clear_nospace()", "rds: use
sk_set_nospace()", "dlm: use sk_set_nospace() and sk_clear_nospace()",
"drbd: use sk_set_nospace()", "nvme-tcp: use sk_clear_nospace()" and
"libceph: use sk_clear_nospace()"), and all six land before "tcp: add
tp->tcp_nospace", so the single choke point the mirror depends on is in
place by the time the mirror exists and no intermediate tree state is
broken.

Could the changelog say that the conversion is completed by the following
patches rather than by this one?

> Callers that had no NULL check are all called from user context with a
> socket attached, so folding the check into the helpers only makes them
> more robust.

The same "some with a NULL check, some without" wording in the first
paragraph reads as a description of the state after this patch, when
drbd_worker.c and net/ceph/messenger.c still carry their own checks.

<br>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922122721.3568295-1-edumazet%40google.com

Re: [PATCH net-next 9/9] tcp: add tp->tcp_nospace

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Commit message claim "It fits in an existing hole,
  sizeof(struct tcp_sock) is unchanged" is wrong for the chosen position.
- [Medium] The invariant the commit message and include/net/tcp.h:787-793
  claim to maintain — "SOCK_NOSPACE set => tp->tcp_nospace set" — is not…
- [Low] The new field comment in include/linux/tcp.h states an invariant
  that in-tree code added by the same patch deliberately violates.

--

tcp: add tp->tcp_nospace

This adds a u8 mirror of SOCK_NOSPACE to struct tcp_sock so that
tcp_check_space() no longer has to touch sk->sk_socket->flags on every
incoming ACK and every transmitted packet.  The mirror is maintained by
sk_set_nospace()/sk_clear_nospace(), __tcp_check_space() keeps the
authoritative SOCK_NOSPACE test, and MPTCP subflows pin the mirror in
subflow_ulp_init().

A few questions below about the claimed layout and about the claimed
invariant.

[ ... ]

> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 6a8c77719322f..d51aae60aa45b 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -307,6 +307,10 @@ struct tcp_sock {
>  		accecn_opt_demand:2,/* Demand AccECN option for n next ACKs */
>  		prev_ecnfield:2; /* ECN bits from the previous segment */
>  	__be32	pred_flags;
> +	u8	tcp_nospace;	/* mirrors SOCK_NOSPACE, but in a cache line
> +				 * that tcp_check_space() already needs.

[Severity: Medium]

The changelog says:

    Add tp->tcp_nospace, a mirror of SOCK_NOSPACE placed in the
    tcp_sock_write_txrx group [...] It fits in an existing hole,
    sizeof(struct tcp_sock) is unchanged.

Is there actually a hole at this particular position?

Counting from the 64-byte aligned start of the tcp_sock_write_tx group
(the __cacheline_group_begin/end markers are zero-length u8 arrays and
consume no space), ecn_flags ends at group-relative offset 145, so the
three u8 bitfield bytes that open tcp_sock_write_txrx occupy 145, 146 and
147:

	u8	nonagle : 4,
		rate_app_limited:1;			/* byte 145 */
	u8	received_ce_pending:4, ...		/* byte 146 */
	u8	accecn_minlen:2, ...			/* byte 147 */
	__be32	pred_flags;				/* 148..151 */
	u64	tcp_clock_cache;			/* 152 */

pred_flags then lands exactly at 148..151 and the 8-byte aligned
tcp_clock_cache exactly at 152, with no padding in between.  The 2-byte
hole that used to precede pred_flags was consumed by the AccECN u8
bitfields that now sit in front of it.

Doesn't inserting a u8 between pred_flags and tcp_clock_cache therefore
add 7 bytes of padding and push tcp_clock_cache, tcp_mstamp, rcv_nxt and
everything up to segs_out 8 bytes further into the hottest group, rather
than filling a hole?

Two places in the same cache line look genuinely free: the spare
unused2:3 bits in the received_ce_pending bitfield right above, or the
3-byte hole after u8 chrono_type in the tx group.

Would it be worth adding a CACHELINE_ASSERT_GROUP_SIZE() for struct
tcp_sock as well?  tcp_struct_check() only uses
CACHELINE_ASSERT_GROUP_MEMBER(), so an 8-byte growth of
tcp_sock_write_txrx is not caught at build time.  A pahole diff of
struct tcp_sock before and after would settle the "sizeof(struct
tcp_sock) is unchanged" claim either way.

> +				 * Can only be set if SOCK_NOSPACE is set.
> +				 */

[Severity: Low]

This comment says the mirror "Can only be set if SOCK_NOSPACE is set",
but subflow_ulp_init() in net/mptcp/subflow.c sets tp->tcp_nospace = 1
at ULP init time, when the shared struct socket has SOCK_NOSPACE clear,
and tcp_clear_nospace() then refuses to ever clear it again for MPTCP
subflows.

The direction the rest of the patch relies on is the reverse one
(SOCK_NOSPACE set => mirror set), which is what the comment block above
tcp_set_nospace() in include/net/tcp.h and the changelog describe.
Could this field comment be reworded to state that direction instead?
As written, the two headers document opposite contracts for the same
field, and a reader trusting this one might drop the SOCK_NOSPACE
re-test just added to __tcp_check_space(), or add a warning for
"mirror set but SOCK_NOSPACE clear", and break MPTCP.

>  	u64	tcp_clock_cache; /* cache last tcp_clock_ns() (see tcp_mstamp_refresh()) */
>  	u64	tcp_mstamp;	/* most recent packet received/sent */
>  	u32	rcv_nxt;	/* What we want to receive next		*/
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 5e5f5f9b89a38..1e1950dd184ec 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -783,14 +783,35 @@ void tcp_done_with_error(struct sock *sk, int err);
>  void tcp_reset(struct sock *sk, struct sk_buff *skb);
>  void tcp_fin(struct sock *sk);
>  void __tcp_check_space(struct sock *sk);
> +
> +/* Mirror of SOCK_NOSPACE in tcp_sock, maintained by sk_set_nospace()
> + * and sk_clear_nospace().
> + *
> + * MPTCP subflows share the parent socket, and thus its SOCK_NOSPACE bit.
> + * Keep their mirror always set (see subflow_ulp_init()) so that they
> + * always reach __tcp_check_space() and behave as before.
> + */
> +static inline void tcp_set_nospace(struct sock *sk)
> +{
> +	if (sk_is_tcp(sk))
> +		WRITE_ONCE(tcp_sk(sk)->tcp_nospace, 1);
> +}
> +
> +static inline void tcp_clear_nospace(struct sock *sk)
> +{
> +	if (sk_is_tcp(sk) && !sk_is_mptcp(sk))
> +		WRITE_ONCE(tcp_sk(sk)->tcp_nospace, 0);
> +}
> +
>  static inline void tcp_check_space(struct sock *sk)
>  {
>  	/* pairs with tcp_poll() */
>  	smp_mb();
>  
> -	if (sk->sk_socket && test_bit(SOCK_NOSPACE, &sk->sk_socket->flags))
> +	if (unlikely(READ_ONCE(tcp_sk(sk)->tcp_nospace)))
>  		__tcp_check_space(sk);
>  }
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 11a22aec7e414..dce4e8e4e2bbe 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2969,8 +2969,13 @@ void sk_set_nospace(struct sock *sk)
>  {
>  	struct socket *sock = sk->sk_socket;
>  
> -	if (sock)
> -		set_bit(SOCK_NOSPACE, &sock->flags);
> +	if (!sock)
> +		return;
> +	/* Mirror first: callers relying on the barrier implied by
> +	 * set_bit() + smp_mb__after_atomic() are then also covered.
> +	 */
> +	tcp_set_nospace(sk);
> +	set_bit(SOCK_NOSPACE, &sock->flags);
>  }
>  EXPORT_SYMBOL(sk_set_nospace);
>  
> @@ -2985,8 +2990,13 @@ void sk_clear_nospace(struct sock *sk)
>  {
>  	struct socket *sock = sk->sk_socket;
>  
> -	if (sock)
> -		clear_bit(SOCK_NOSPACE, &sock->flags);
> +	if (!sock)
> +		return;
> +	clear_bit(SOCK_NOSPACE, &sock->flags);
> +	/* Mirror last: a stale mirror only costs a slow path, while a
> +	 * stale SOCK_NOSPACE would cost a missed EPOLLOUT.
> +	 */
> +	tcp_clear_nospace(sk);
>  }
>  EXPORT_SYMBOL(sk_clear_nospace);

[Severity: Medium]

Can the invariant the changelog states actually hold?

    The two flags are now only changed from sk_set_nospace() and
    sk_clear_nospace(), which maintain this invariant:

            SOCK_NOSPACE set  =>  tp->tcp_nospace set

The set side stores the mirror first and then the bit, while the clear
side clears the bit first and then the mirror, so the two non-atomic
stores are issued in opposite order on the two sides.  sk_set_nospace()
runs without the socket lock from tcp_poll(), while sk_clear_nospace()
for TCP runs under the socket lock from
tcp_check_space()->__tcp_check_space()->tcp_new_space()->sk_stream_write_space().

CPU0, in tcp_poll(), no socket lock:
	tcp_set_nospace(sk);			/* mirror = 1 */
	/* delayed here */

CPU1, socket lock held, processing an ACK:
	sk_clear_nospace(sk)
		clear_bit(SOCK_NOSPACE, &sock->flags);
		tcp_clear_nospace(sk);		/* mirror = 0 */

CPU0 resumes:
	set_bit(SOCK_NOSPACE, &sock->flags);	/* bit = 1, mirror = 0 */

Doesn't that leave SOCK_NOSPACE set with tp->tcp_nospace clear, the
combination the changelog declares impossible?  It looks like a lost
update across two separate locations, so no barrier or WRITE_ONCE
placement fixes it, and reversing the store order in sk_clear_nospace()
just makes the symmetric interleaving possible instead.

While that state holds, tcp_check_space() short-circuits on every ACK
and every transmitted packet, so tcp_new_space() and
sk->sk_write_space() are never reached: no SOCK_NOSPACE clearing, no
EPOLLOUT/SIGIO wake, no tcp_sndbuf_expand() autotuning, and no
tcp_chrono_stop(TCP_CHRONO_SNDBUF_LIMITED).  Before this patch the same
race could only end up in the safe combination, since the bit being set
always took the slow path.

For poll()/epoll() this seems to self-heal: sk_clear_nospace() in
sk_stream_write_space() is immediately followed by
wake_up_interruptible_poll(EPOLLOUT ...) while the waiter is already
queued, and tcp_poll() re-arms the mirror whenever it finds the socket
not writeable.  sendmsg() callers take the socket lock and cannot race
at all.

The case that does not look self-healing is in-kernel users whose only
re-arm path is the suppressed ->sk_write_space() callback and whose
sk_set_nospace() is lock-free, for example send_to_sock() in
fs/dlm/lowcomms.c:

	sk_set_nospace(con->sock->sk);
	con->sock->sk->sk_write_pending++;

whose retry comes from lowcomms_write_space() requeueing con->swork, and
xs_nospace() in net/sunrpc/xprtsock.c, whose retry comes from
xs_write_space()->xprt_write_space().  Can a transmit stall there until
an unrelated event or an RPC timeout?

Would re-testing SOCK_NOSPACE after the clear (with a full barrier) and
re-arming the mirror, or only clearing the mirror together with a
re-validation by the socket owner, be enough to restore the stated
invariant?

[ ... ]

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index f0a6725d2c376..e297c88be503b 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -2001,6 +2001,11 @@ static int subflow_ulp_init(struct sock *sk)
>  	pr_debug("subflow=%p, family=%d\n", ctx, sk->sk_family);
>  
>  	tp->is_mptcp = 1;
> +	/* Subflows share the MPTCP socket, and thus its SOCK_NOSPACE bit,
> +	 * which tcp_check_space() can not mirror. Pin the mirror so that
> +	 * __tcp_check_space() always tests the shared bit.
> +	 */
> +	tp->tcp_nospace = 1;
>  	ctx->icsk_af_ops = icsk->icsk_af_ops;
>  	icsk->icsk_af_ops = subflow_default_af_ops(sk);
>  	ctx->tcp_state_change = sk->sk_state_change;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922122721.3568295-1-edumazet%40google.com

Re: [PATCH net-next 1/9] dlm: fix send buffer backpressure handling

[Eric Dumazet]

On Thu, Sep 24, 2026 at 2:27 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] fs/dlm/lowcomms.c:send_to_sock() — making the CF_APP_LIMITED
>   branch live for the first time in ten years introduces a permanent…
> - [Low] fs/dlm/lowcomms.c:send_to_sock(): after switching the guard to
>   SOCK_NOSPACE, the guard and the "Notify TCP" action now operate on the…
>
> --
>
> dlm: fix send buffer backpressure handling
>
> The send path in fs/dlm/lowcomms.c tested SOCKWQ_ASYNC_NOSPACE in
> con->sock->flags, a bit that has not lived there since commit ceb5d58b2170,
> so the test was always false and send_to_sock() always returned
> DLM_IO_RESCHED on -EAGAIN. The patch switches the test to SOCK_NOSPACE and
> drops the matching clear_bit() in lowcomms_write_space().
>
> > diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> > index 2aff1c7c17de4..abe9ae4c643fa 100644
> > --- a/fs/dlm/lowcomms.c
> > +++ b/fs/dlm/lowcomms.c
> > @@ -522,10 +522,8 @@ static void lowcomms_write_space(struct sock *sk)
> >       clear_bit(SOCK_NOSPACE, &con->sock->flags);
> >
> >       spin_lock_bh(&con->writequeue_lock);
> > -     if (test_and_clear_bit(CF_APP_LIMITED, &con->flags)) {
> > +     if (test_and_clear_bit(CF_APP_LIMITED, &con->flags))
> >               con->sock->sk->sk_write_pending--;
> > -             clear_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags);
> > -     }
> >
> >       lowcomms_queue_swork(con);
> >       spin_unlock_bh(&con->writequeue_lock);
> > @@ -1391,7 +1389,7 @@ static int send_to_sock(struct connection *con)
> >       if (ret == -EAGAIN || ret == 0) {
> >               lock_sock(con->sock->sk);
> >               spin_lock_bh(&con->writequeue_lock);
> > -             if (test_bit(SOCKWQ_ASYNC_NOSPACE, &con->sock->flags) &&
> > +             if (test_bit(SOCK_NOSPACE, &con->sock->flags) &&
> >                   !test_and_set_bit(CF_APP_LIMITED, &con->flags)) {
>
> [Severity: High]
> This makes the CF_APP_LIMITED branch reachable for the first time in ten
> years.  Once it is taken, is there anything left that can restart this
> connection if the TCP connection then dies?

I do not know. Maybe dlm folks can answer the question, and take care
of this patch.

Re: [PATCH net-next 9/9] tcp: add tp->tcp_nospace

[Eric Dumazet]

On Thu, Sep 24, 2026 at 2:27 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).

Ack, I will send a V2, thanks!

pw-bot: cr