Re: [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before f

To: Paolo Abeni <pabeni@xxxxxxxxxx>
Subject: Re: [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before freeing timer
Cc: Jakub Kicinski <kuba@xxxxxxxxxx>, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Stephen Boyd <sboyd@xxxxxxxxxx>, Guenter Roeck <linux@xxxxxxxxxxxx>, Jesse Brandeburg <jesse.brandeburg@xxxxxxxxx>, Tony Nguyen <anthony.l.nguyen@xxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Eric Dumazet <edumazet@xxxxxxxxxx>, Mirko Lindner <mlindner@xxxxxxxxxxx>, Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>, Martin KaFai Lau <martin.lau@xxxxxxxxxx>, Alexei Starovoitov <ast@xxxxxxxxxx>, Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>, Pavel Begunkov <asml.silence@xxxxxxxxx>, Menglong Dong <imagedong@xxxxxxxxxxx>, linux-usb@xxxxxxxxxxxxxxx, linux-wireless@xxxxxxxxxxxxxxx, bridge@xxxxxxxxxxxxxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, coreteam@xxxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, linux-afs@xxxxxxxxxxxxxxxxxxx, linux-nfs@xxxxxxxxxxxxxxx, tipc-discussion@xxxxxxxxxxxxxxxxxxxxx
From: Steven Rostedt <rostedt@xxxxxxxxxxx>
Date: Thu, 3 Nov 2022 17:51:23 -0400
On Sun, 30 Oct 2022 18:22:03 +0100
Paolo Abeni <pabeni@xxxxxxxxxx> wrote:

> On the positive side, I think converting the sk_stop_timer in 
> inet_csk_clear_xmit_timers() should be safe and should cover the issue
> reported by Guenter

Would something like this be OK? 

[ Note, talking with Thomas Gleixner, we agreed that we are changing the
  name to: time_shutdown_sync() and timer_shutdown() (no wait version).
  I'll be posting new patches soon. ]

-- Steve

diff --git a/include/net/sock.h b/include/net/sock.h
index 22f8bab583dd..0ef58697d4e5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2439,6 +2439,8 @@ void sk_stop_timer(struct sock *sk, struct timer_list 
 void sk_stop_timer_sync(struct sock *sk, struct timer_list *timer);
+void sk_shutdown_timer(struct sock *sk, struct timer_list *timer);
 int __sk_queue_drop_skb(struct sock *sk, struct sk_buff_head *sk_queue,
                        struct sk_buff *skb, unsigned int flags,
                        void (*destructor)(struct sock *sk,
diff --git a/net/core/sock.c b/net/core/sock.c
index a3ba0358c77c..82124862b594 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3357,6 +3357,13 @@ void sk_stop_timer_sync(struct sock *sk, struct 
timer_list *timer)
+void sk_shutdown_timer(struct sock *sk, struct timer_list* timer)
+       if (timer_shutdown(timer))
+               __sock_put(sk);
 void sock_init_data(struct socket *sock, struct sock *sk)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 5e70228c5ae9..71f398f51958 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -722,15 +722,15 @@ void inet_csk_clear_xmit_timers(struct sock *sk)
        icsk->icsk_pending = icsk->icsk_ack.pending = 0;
-       sk_stop_timer(sk, &icsk->icsk_retransmit_timer);
-       sk_stop_timer(sk, &icsk->icsk_delack_timer);
-       sk_stop_timer(sk, &sk->sk_timer);
+       sk_shutdown_timer(sk, &icsk->icsk_retransmit_timer);
+       sk_shutdown_timer(sk, &icsk->icsk_delack_timer);
+       sk_shutdown_timer(sk, &sk->sk_timer);
 void inet_csk_delete_keepalive_timer(struct sock *sk)
-       sk_stop_timer(sk, &sk->sk_timer);
+       sk_shutdown_timer(sk, &sk->sk_timer);

<Prev in Thread] Current Thread [Next in Thread>