dlm: fix send buffer backpressure handling

[PATCH] dlm: fix send buffer backpressure handling

Posted by Eric Dumazet 1 week, 3 days ago

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
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 2aff1c7c17de..abe9ae4c643f 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.1007.g17ff1f9808-goog

Re: [PATCH] dlm: fix send buffer backpressure handling

Posted by Eric Dumazet 3 days, 7 hours ago

On Mon, Sep 14, 2026 at 6:44 PM 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
> 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 2aff1c7c17de..abe9ae4c643f 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.1007.g17ff1f9808-goog
>

Gentle ping ?

I have an ongoing series for net-next removing one cache line miss in
TCP sendmsg() and ACK processing,
blocked by this patch.

If you prefer, I can insert this patch into my net-next series.

Thanks.

Re: [PATCH] dlm: fix send buffer backpressure handling

Posted by Alexander Aring 2 days, 19 hours ago

Hi,

On Mon, Sep 21, 2026 at 9:10 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Sep 14, 2026 at 6:44 PM 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
> > 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 2aff1c7c17de..abe9ae4c643f 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.1007.g17ff1f9808-goog
> >
>
> Gentle ping ?
>
> I have an ongoing series for net-next removing one cache line miss in
> TCP sendmsg() and ACK processing,
> blocked by this patch.
>
> If you prefer, I can insert this patch into my net-next series.

I acked the patch in your patch series.

Again thanks for looking into that!

- Alex