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

To: Eric Dumazet <edumazet@xxxxxxxxxx>
Subject: Re: [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before freeing timer
Cc: Paolo Abeni <pabeni@xxxxxxxxxx>, 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>, 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: Fri, 4 Nov 2022 01:51:39 -0400
On Thu, 3 Nov 2022 17:00:20 -0700
Eric Dumazet <edumazet@xxxxxxxxxx> wrote:

>  inet_csk_clear_xmit_timers() can be called multiple times during TCP
> socket lifetime.
> (See tcp_disconnect(), which can be followed by another connect() ... and 
> loop)
> Maybe add a second parameter, or add a new
> inet_csk_shutdown_xmit_timers() only called from tcp_v4_destroy_sock() ?

I guess.

> >
> >  void inet_csk_delete_keepalive_timer(struct sock *sk)
> >  {
> > -       sk_stop_timer(sk, &sk->sk_timer);
> > +       sk_shutdown_timer(sk, &sk->sk_timer);  
> SO_KEEPALIVE can be called multiple times in a TCP socket lifetime,
> on/off/on/off/...
> I suggest leaving sk_stop_timer() here.
> Eventually  inet_csk_clear_xmit_timers( sk, destroy=true) (or
> inet_csk_shutdown_xmit_timers(())
>    will  be called before the socket is destroyed.



I posted a new series, but did not include this change. If you want to
test that other series, I would suggest to at least add the first part
of this patch, otherwise it will trigger. But we want to see if there's
other locations of issue that we should care about.

-- Steve

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