Netdev List
 help / color / mirror / Atom feed
* [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset()
@ 2026-08-25  2:36 Eric Dumazet
  2026-08-25 11:27 ` Jason Xing
  2026-08-27  9:09 ` Paolo Abeni
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2026-08-25  2:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Neal Cardwell, Kuniyuki Iwashima, netdev,
	eric.dumazet, Eric Dumazet

tcp_send_active_reset() can be called from contexts where gfp_any()
(in tcp_disconnect()) or sk->sk_allocation (in __tcp_close() and
mptcp_do_fastclose()) evaluates to GFP_KERNEL, which includes
__GFP_FS and __GFP_DIRECT_RECLAIM.

Allocating with GFP_KERNEL while holding the socket lock (sk_lock) creates
a lockdep dependency:
  sk_lock -> fs_reclaim

This causes false-positive lockdep circular locking warnings with storage
subsystems (such as nvme-tcp) that acquire socket locks in block I/O paths
and invoke tcp_disconnect() or close sockets upon teardown:
  set->srcu -> sk_lock -> fs_reclaim -> elevator_lock -> set->srcu

Active resets are small RST packet headers that should never
enter direct reclaim or block while holding socket locks.

Hardcode GFP_ATOMIC inside tcp_send_active_reset() and remove its
priority argument since all callers now use GFP_ATOMIC.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h     |  3 +--
 net/ipv4/tcp.c        | 14 ++++++--------
 net/ipv4/tcp_output.c |  7 +++----
 net/ipv4/tcp_timer.c  |  6 +++---
 net/mptcp/protocol.c  |  3 +--
 net/mptcp/protocol.h  |  2 +-
 6 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 670c20876f265c14504c26f45b87763ae47d3127..436495ff2271de047423dbe33036b7c6d1556584 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -765,8 +765,7 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
 void tcp_send_probe0(struct sock *);
 int tcp_write_wakeup(struct sock *, int mib);
 void tcp_send_fin(struct sock *sk);
-void tcp_send_active_reset(struct sock *sk, gfp_t priority,
-			   enum sk_rst_reason reason);
+void tcp_send_active_reset(struct sock *sk, enum sk_rst_reason reason);
 int tcp_send_synack(struct sock *);
 void tcp_push_one(struct sock *, unsigned int mss_now);
 void __tcp_send_ack(struct sock *sk, u32 rcv_nxt, u16 flags);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4237d0e994d6f9d754d2167023e3981a40b58f4..93d723d8c1098e421cb7e3596318acd20fd80233 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3182,8 +3182,7 @@ void __tcp_close(struct sock *sk, long timeout)
 		/* Unread data was tossed, zap the connection. */
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONCLOSE);
 		tcp_set_state(sk, TCP_CLOSE);
-		tcp_send_active_reset(sk, sk->sk_allocation,
-				      SK_RST_REASON_TCP_ABORT_ON_CLOSE);
+		tcp_send_active_reset(sk, SK_RST_REASON_TCP_ABORT_ON_CLOSE);
 	} else if (sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime) {
 		/* Check zero linger _after_ checking for unread data. */
 		sk->sk_prot->disconnect(sk, 0);
@@ -3257,7 +3256,7 @@ void __tcp_close(struct sock *sk, long timeout)
 		struct tcp_sock *tp = tcp_sk(sk);
 		if (READ_ONCE(tp->linger2) < 0) {
 			tcp_set_state(sk, TCP_CLOSE);
-			tcp_send_active_reset(sk, GFP_ATOMIC,
+			tcp_send_active_reset(sk,
 					      SK_RST_REASON_TCP_ABORT_ON_LINGER);
 			__NET_INC_STATS(sock_net(sk),
 					LINUX_MIB_TCPABORTONLINGER);
@@ -3276,7 +3275,7 @@ void __tcp_close(struct sock *sk, long timeout)
 	if (sk->sk_state != TCP_CLOSE) {
 		if (tcp_check_oom(sk, 0)) {
 			tcp_set_state(sk, TCP_CLOSE);
-			tcp_send_active_reset(sk, GFP_ATOMIC,
+			tcp_send_active_reset(sk,
 					      SK_RST_REASON_TCP_ABORT_ON_MEMORY);
 			__NET_INC_STATS(sock_net(sk),
 					LINUX_MIB_TCPABORTONMEMORY);
@@ -3377,14 +3376,14 @@ int tcp_disconnect(struct sock *sk, int flags)
 	} else if (unlikely(tp->repair)) {
 		WRITE_ONCE(sk->sk_err, ECONNABORTED);
 	} else if (tcp_need_reset(old_state)) {
-		tcp_send_active_reset(sk, gfp_any(), SK_RST_REASON_TCP_STATE);
+		tcp_send_active_reset(sk, SK_RST_REASON_TCP_STATE);
 		WRITE_ONCE(sk->sk_err, ECONNRESET);
 	} else if (tp->snd_nxt != tp->write_seq &&
 		   (1 << old_state) & (TCPF_CLOSING | TCPF_LAST_ACK)) {
 		/* The last check adjusts for discrepancy of Linux wrt. RFC
 		 * states
 		 */
-		tcp_send_active_reset(sk, gfp_any(),
+		tcp_send_active_reset(sk,
 				      SK_RST_REASON_TCP_DISCONNECT_WITH_DATA);
 		WRITE_ONCE(sk->sk_err, ECONNRESET);
 	} else if (old_state == TCP_SYN_SENT)
@@ -5147,8 +5146,7 @@ int tcp_abort(struct sock *sk, int err)
 	bh_lock_sock(sk);
 
 	if (tcp_need_reset(sk->sk_state))
-		tcp_send_active_reset(sk, GFP_ATOMIC,
-				      SK_RST_REASON_TCP_STATE);
+		tcp_send_active_reset(sk, SK_RST_REASON_TCP_STATE);
 	tcp_done_with_error(sk, err);
 
 	bh_unlock_sock(sk);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f2709d585edbd9d9fef97953920edfcb7c45e61c..19a799e5d5ce3b55c57e2557d21a8d44982db714 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3849,15 +3849,14 @@ void tcp_send_fin(struct sock *sk)
  * was unread data in the receive queue.  This behavior is recommended
  * by RFC 2525, section 2.17.  -DaveM
  */
-void tcp_send_active_reset(struct sock *sk, gfp_t priority,
-			   enum sk_rst_reason reason)
+void tcp_send_active_reset(struct sock *sk, enum sk_rst_reason reason)
 {
 	struct sk_buff *skb;
 
 	TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
 
 	/* NOTE: No TCP options attached and we never retransmit this. */
-	skb = alloc_skb(MAX_TCP_HEADER, priority);
+	skb = alloc_skb(MAX_TCP_HEADER, GFP_ATOMIC);
 	if (!skb) {
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
 		return;
@@ -3869,7 +3868,7 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority,
 			     TCPHDR_ACK | TCPHDR_RST);
 	tcp_mstamp_refresh(tcp_sk(sk));
 	/* Send it off. */
-	if (tcp_transmit_skb(sk, skb, 0, priority))
+	if (tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC))
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
 
 	/* skb of trace_tcp_send_reset() keeps the skb that caused RST,
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 1038e7ba9c2eb19279b431249b56f8a8e4ffaf74..e56eae4bc341e94880bf0d6d1435094dfaaf879f 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -126,7 +126,7 @@ static int tcp_out_of_resources(struct sock *sk, bool do_reset)
 		    (!tp->snd_wnd && !tp->packets_out))
 			do_reset = true;
 		if (do_reset)
-			tcp_send_active_reset(sk, GFP_ATOMIC,
+			tcp_send_active_reset(sk,
 					      SK_RST_REASON_TCP_ABORT_ON_MEMORY);
 		tcp_done(sk);
 		__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONMEMORY);
@@ -809,7 +809,7 @@ static void tcp_keepalive_timer(struct timer_list *t)
 				goto out;
 			}
 		}
-		tcp_send_active_reset(sk, GFP_ATOMIC, SK_RST_REASON_TCP_STATE);
+		tcp_send_active_reset(sk, SK_RST_REASON_TCP_STATE);
 		goto death;
 	}
 
@@ -836,7 +836,7 @@ static void tcp_keepalive_timer(struct timer_list *t)
 		    icsk->icsk_probes_out > 0) ||
 		    (user_timeout == 0 &&
 		    icsk->icsk_probes_out >= keepalive_probes(tp))) {
-			tcp_send_active_reset(sk, GFP_ATOMIC,
+			tcp_send_active_reset(sk,
 					      SK_RST_REASON_TCP_KEEPALIVE_TIMEOUT);
 			tcp_write_err(sk);
 			goto out;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b474d03620a75d3df26fcae1a84901b965c6954e..e1f08f71cdb16b2bbecd5630bca895d651c17b4c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3109,8 +3109,7 @@ static void mptcp_do_fastclose(struct sock *sk)
 		 */
 		inet_csk(ssk)->icsk_ack.rcv_mss = TCP_MIN_MSS;
 
-		tcp_send_active_reset(ssk, ssk->sk_allocation,
-				      SK_RST_REASON_TCP_ABORT_ON_CLOSE);
+		tcp_send_active_reset(ssk, SK_RST_REASON_TCP_ABORT_ON_CLOSE);
 unlock:
 		release_sock(ssk);
 	}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 06a107d4e8392269b42f3af7ac531764619107c1..87ccb84e9927ccb23b242c11d34eb69427362702 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -690,7 +690,7 @@ mptcp_send_active_reset_reason(struct sock *sk)
 	enum sk_rst_reason reason;
 
 	reason = sk_rst_convert_mptcp_reason(subflow->reset_reason);
-	tcp_send_active_reset(sk, GFP_ATOMIC, reason);
+	tcp_send_active_reset(sk, reason);
 }
 
 /* Made the fwd mem carried by the given skb available to the msk,
-- 
2.55.0.860.g4b6b3295ed-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset()
  2026-08-25  2:36 [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset() Eric Dumazet
@ 2026-08-25 11:27 ` Jason Xing
  2026-08-27  9:09 ` Paolo Abeni
  1 sibling, 0 replies; 6+ messages in thread
From: Jason Xing @ 2026-08-25 11:27 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Neal Cardwell, Kuniyuki Iwashima, netdev, eric.dumazet

On Tue, Aug 25, 2026 at 10:36 AM Eric Dumazet <edumazet@google.com> wrote:
>
> tcp_send_active_reset() can be called from contexts where gfp_any()
> (in tcp_disconnect()) or sk->sk_allocation (in __tcp_close() and
> mptcp_do_fastclose()) evaluates to GFP_KERNEL, which includes
> __GFP_FS and __GFP_DIRECT_RECLAIM.
>
> Allocating with GFP_KERNEL while holding the socket lock (sk_lock) creates
> a lockdep dependency:
>   sk_lock -> fs_reclaim
>
> This causes false-positive lockdep circular locking warnings with storage
> subsystems (such as nvme-tcp) that acquire socket locks in block I/O paths
> and invoke tcp_disconnect() or close sockets upon teardown:
>   set->srcu -> sk_lock -> fs_reclaim -> elevator_lock -> set->srcu
>
> Active resets are small RST packet headers that should never
> enter direct reclaim or block while holding socket locks.
>
> Hardcode GFP_ATOMIC inside tcp_send_active_reset() and remove its
> priority argument since all callers now use GFP_ATOMIC.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset()
  2026-08-25  2:36 [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset() Eric Dumazet
  2026-08-25 11:27 ` Jason Xing
@ 2026-08-27  9:09 ` Paolo Abeni
  2026-08-27  9:33   ` Eric Dumazet
  1 sibling, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-08-27  9:09 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller, Jakub Kicinski
  Cc: Simon Horman, Neal Cardwell, Kuniyuki Iwashima, netdev,
	eric.dumazet

On 8/25/26 4:36 AM, Eric Dumazet wrote:
> tcp_send_active_reset() can be called from contexts where gfp_any()
> (in tcp_disconnect()) or sk->sk_allocation (in __tcp_close() and
> mptcp_do_fastclose()) evaluates to GFP_KERNEL, which includes
> __GFP_FS and __GFP_DIRECT_RECLAIM.
> 
> Allocating with GFP_KERNEL while holding the socket lock (sk_lock) creates
> a lockdep dependency:
>   sk_lock -> fs_reclaim
> 
> This causes false-positive lockdep circular locking warnings with storage
> subsystems (such as nvme-tcp) that acquire socket locks in block I/O paths
> and invoke tcp_disconnect() or close sockets upon teardown:
>   set->srcu -> sk_lock -> fs_reclaim -> elevator_lock -> set->srcu
> 
> Active resets are small RST packet headers that should never
> enter direct reclaim or block while holding socket locks.
> 
> Hardcode GFP_ATOMIC inside tcp_send_active_reset() and remove its
> priority argument since all callers now use GFP_ATOMIC.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Sashiko has a bit to say:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825023614.1228551-1-edumazet%40google.com

A couple of issues (the same lockdep chain still be present for
regular transmissions, and SOCK_MEMALLOC being ignored) looks
real to me. WDYT?

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset()
  2026-08-27  9:09 ` Paolo Abeni
@ 2026-08-27  9:33   ` Eric Dumazet
  2026-08-27 10:04     ` Paolo Abeni
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-08-27  9:33 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: David S . Miller, Jakub Kicinski, Simon Horman, Neal Cardwell,
	Kuniyuki Iwashima, netdev, eric.dumazet

On Thu, Aug 27, 2026 at 11:09 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/25/26 4:36 AM, Eric Dumazet wrote:
> > tcp_send_active_reset() can be called from contexts where gfp_any()
> > (in tcp_disconnect()) or sk->sk_allocation (in __tcp_close() and
> > mptcp_do_fastclose()) evaluates to GFP_KERNEL, which includes
> > __GFP_FS and __GFP_DIRECT_RECLAIM.
> >
> > Allocating with GFP_KERNEL while holding the socket lock (sk_lock) creates
> > a lockdep dependency:
> >   sk_lock -> fs_reclaim
> >
> > This causes false-positive lockdep circular locking warnings with storage
> > subsystems (such as nvme-tcp) that acquire socket locks in block I/O paths
> > and invoke tcp_disconnect() or close sockets upon teardown:
> >   set->srcu -> sk_lock -> fs_reclaim -> elevator_lock -> set->srcu
> >
> > Active resets are small RST packet headers that should never
> > enter direct reclaim or block while holding socket locks.
> >
> > Hardcode GFP_ATOMIC inside tcp_send_active_reset() and remove its
> > priority argument since all callers now use GFP_ATOMIC.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> Sashiko has a bit to say:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825023614.1228551-1-edumazet%40google.com
>
> A couple of issues (the same lockdep chain still be present for
> regular transmissions, and SOCK_MEMALLOC being ignored) looks
> real to me. WDYT?

Thanks for the pointer; my local Sashiko couldn't spot this.

Regarding SOCK_MEMALLOC, that's a good point.
Using bare GFP_ATOMICdrops __GFP_MEMALLOC and misses __GFP_NOWARN.
We should use:
    sk_gfp_mask(sk, GFP_ATOMIC | __GFP_NOWARN)

I will squash this into V2.

All the remarks against nvme will be taken care of once this patch is merged.

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 19a799e5d5ce3b55c57e2557d21a8d44982db714..3581384d097feea4cddc3c1f30404367aa93aaa8
100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3851,12 +3851,13 @@ void tcp_send_fin(struct sock *sk)
  */
 void tcp_send_active_reset(struct sock *sk, enum sk_rst_reason reason)
 {
+       gfp_t priority = sk_gfp_mask(sk, GFP_ATOMIC | __GFP_NOWARN);
        struct sk_buff *skb;

        TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);

        /* NOTE: No TCP options attached and we never retransmit this. */
-       skb = alloc_skb(MAX_TCP_HEADER, GFP_ATOMIC);
+       skb = alloc_skb(MAX_TCP_HEADER, priority);
        if (!skb) {
                NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
                return;
@@ -3868,7 +3869,7 @@ void tcp_send_active_reset(struct sock *sk, enum
sk_rst_reason reason)
                             TCPHDR_ACK | TCPHDR_RST);
        tcp_mstamp_refresh(tcp_sk(sk));
        /* Send it off. */
-       if (tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC))
+       if (tcp_transmit_skb(sk, skb, 0, priority))
                NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);

        /* skb of trace_tcp_send_reset() keeps the skb that caused RST,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset()
  2026-08-27  9:33   ` Eric Dumazet
@ 2026-08-27 10:04     ` Paolo Abeni
  2026-08-27 10:56       ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-08-27 10:04 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Simon Horman, Neal Cardwell,
	Kuniyuki Iwashima, netdev, eric.dumazet

On 8/27/26 11:33 AM, Eric Dumazet wrote:
> On Thu, Aug 27, 2026 at 11:09 AM Paolo Abeni <pabeni@redhat.com> wrote:
>>
>> On 8/25/26 4:36 AM, Eric Dumazet wrote:
>>> tcp_send_active_reset() can be called from contexts where gfp_any()
>>> (in tcp_disconnect()) or sk->sk_allocation (in __tcp_close() and
>>> mptcp_do_fastclose()) evaluates to GFP_KERNEL, which includes
>>> __GFP_FS and __GFP_DIRECT_RECLAIM.
>>>
>>> Allocating with GFP_KERNEL while holding the socket lock (sk_lock) creates
>>> a lockdep dependency:
>>>   sk_lock -> fs_reclaim
>>>
>>> This causes false-positive lockdep circular locking warnings with storage
>>> subsystems (such as nvme-tcp) that acquire socket locks in block I/O paths
>>> and invoke tcp_disconnect() or close sockets upon teardown:
>>>   set->srcu -> sk_lock -> fs_reclaim -> elevator_lock -> set->srcu
>>>
>>> Active resets are small RST packet headers that should never
>>> enter direct reclaim or block while holding socket locks.
>>>
>>> Hardcode GFP_ATOMIC inside tcp_send_active_reset() and remove its
>>> priority argument since all callers now use GFP_ATOMIC.
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Sashiko has a bit to say:
>>
>> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825023614.1228551-1-edumazet%40google.com
>>
>> A couple of issues (the same lockdep chain still be present for
>> regular transmissions, and SOCK_MEMALLOC being ignored) looks
>> real to me. WDYT?
> 
> Thanks for the pointer; my local Sashiko couldn't spot this.
> 
> Regarding SOCK_MEMALLOC, that's a good point.
> Using bare GFP_ATOMICdrops __GFP_MEMALLOC and misses __GFP_NOWARN.
> We should use:
>     sk_gfp_mask(sk, GFP_ATOMIC | __GFP_NOWARN)
> 
> I will squash this into V2.

Thanks!
> All the remarks against nvme will be taken care of once this patch is merged.
So the addressed lockdep splat is against nvme, right? Given the
reclassification and memalloc_noio_save() dance performed by nvme
noted by sashiko, I'm wondering if something alike the following
(completely untested!) could be a viable alternative?
---
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4237d0e994d..755feda4e399 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3377,14 +3377,15 @@ int tcp_disconnect(struct sock *sk, int flags)
 	} else if (unlikely(tp->repair)) {
 		WRITE_ONCE(sk->sk_err, ECONNABORTED);
 	} else if (tcp_need_reset(old_state)) {
-		tcp_send_active_reset(sk, gfp_any(), SK_RST_REASON_TCP_STATE);
+		tcp_send_active_reset(sk, sk->sk_allocation,
+				      SK_RST_REASON_TCP_STATE);
 		WRITE_ONCE(sk->sk_err, ECONNRESET);
 	} else if (tp->snd_nxt != tp->write_seq &&
 		   (1 << old_state) & (TCPF_CLOSING | TCPF_LAST_ACK)) {
 		/* The last check adjusts for discrepancy of Linux wrt. RFC
 		 * states
 		 */
-		tcp_send_active_reset(sk, gfp_any(),
+		tcp_send_active_reset(sk, sk->sk_alloction,
 				      SK_RST_REASON_TCP_DISCONNECT_WITH_DATA);
 		WRITE_ONCE(sk->sk_err, ECONNRESET);
 	} else if (old_state == TCP_SYN_SENT)


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset()
  2026-08-27 10:04     ` Paolo Abeni
@ 2026-08-27 10:56       ` Eric Dumazet
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2026-08-27 10:56 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: David S . Miller, Jakub Kicinski, Simon Horman, Neal Cardwell,
	Kuniyuki Iwashima, netdev, eric.dumazet

On Thu, Aug 27, 2026 at 12:04 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/27/26 11:33 AM, Eric Dumazet wrote:
> > On Thu, Aug 27, 2026 at 11:09 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >>
> >> On 8/25/26 4:36 AM, Eric Dumazet wrote:
> >>> tcp_send_active_reset() can be called from contexts where gfp_any()
> >>> (in tcp_disconnect()) or sk->sk_allocation (in __tcp_close() and
> >>> mptcp_do_fastclose()) evaluates to GFP_KERNEL, which includes
> >>> __GFP_FS and __GFP_DIRECT_RECLAIM.
> >>>
> >>> Allocating with GFP_KERNEL while holding the socket lock (sk_lock) creates
> >>> a lockdep dependency:
> >>>   sk_lock -> fs_reclaim
> >>>
> >>> This causes false-positive lockdep circular locking warnings with storage
> >>> subsystems (such as nvme-tcp) that acquire socket locks in block I/O paths
> >>> and invoke tcp_disconnect() or close sockets upon teardown:
> >>>   set->srcu -> sk_lock -> fs_reclaim -> elevator_lock -> set->srcu
> >>>
> >>> Active resets are small RST packet headers that should never
> >>> enter direct reclaim or block while holding socket locks.
> >>>
> >>> Hardcode GFP_ATOMIC inside tcp_send_active_reset() and remove its
> >>> priority argument since all callers now use GFP_ATOMIC.
> >>>
> >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >>> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >> Sashiko has a bit to say:
> >>
> >> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825023614.1228551-1-edumazet%40google.com
> >>
> >> A couple of issues (the same lockdep chain still be present for
> >> regular transmissions, and SOCK_MEMALLOC being ignored) looks
> >> real to me. WDYT?
> >
> > Thanks for the pointer; my local Sashiko couldn't spot this.
> >
> > Regarding SOCK_MEMALLOC, that's a good point.
> > Using bare GFP_ATOMICdrops __GFP_MEMALLOC and misses __GFP_NOWARN.
> > We should use:
> >     sk_gfp_mask(sk, GFP_ATOMIC | __GFP_NOWARN)
> >
> > I will squash this into V2.
>
> Thanks!
> > All the remarks against nvme will be taken care of once this patch is merged.
> So the addressed lockdep splat is against nvme, right? Given the
> reclassification and memalloc_noio_save() dance performed by nvme
> noted by sashiko, I'm wondering if something alike the following
> (completely untested!) could be a viable alternative?

I considered that, but having tcp_send_active_reset() use
sk_gfp_mask(sk, GFP_ATOMIC | __GFP_NOWARN) internally is  cleaner
and safer:

 Active resets are small control packets sent during
 abort/teardown. There is no reason to ever enter direct reclaim or
 __GFP_FS while holding socket locks, just as we never do for FINs,
 ACKs, or window probes in tcp_output.c
 Note that the current thread is likely to free memory sooner;
it's better not to block it because of GFP_KERNEL.

tcp_disconnect() was not the only caller passing GFP_KERNEL; __tcp_close()
 (when tossing unread data) and mptcp_do_fastclose() also passed
 sk->sk_allocation (GFP_KERNEL for normal sockets).

7 out of the 10 call sites of tcp_send_active_reset() were already passing
 GFP_ATOMIC (tcp_abort, timers, linger2, OOM, mptcp reset reason). Removing
 the priority argument simplifies the API and guarantees that all callers
 use a safe, non-reclaiming mask.

Other in-kernel users (or future storage drivers) that do not explicitly set
 sk->sk_allocation to GFP_ATOMIC would still suffer from the same issue if
 we keep GFP_KERNEL under socket locks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-27 10:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-25  2:36 [PATCH net] tcp: use GFP_ATOMIC in tcp_send_active_reset() Eric Dumazet
2026-08-25 11:27 ` Jason Xing
2026-08-27  9:09 ` Paolo Abeni
2026-08-27  9:33   ` Eric Dumazet
2026-08-27 10:04     ` Paolo Abeni
2026-08-27 10:56       ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox