LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

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

To: Jakub Kicinski <kuba@xxxxxxxxxx>, Steven Rostedt <rostedt@xxxxxxxxxxx>
Subject: Re: [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before freeing timer
Cc: 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: Paolo Abeni <pabeni@xxxxxxxxxx>
Date: Sun, 30 Oct 2022 18:22:03 +0100
On Fri, 2022-10-28 at 15:46 -0700, Jakub Kicinski wrote:
> On Fri, 28 Oct 2022 18:31:49 -0400 Steven Rostedt wrote:
> > Could someone from networking confirm (or deny) that the timer being
> > removed in sk_stop_timer() will no longer be used even if del_timer()
> > returns false?
> > 
> > net/core/sock.c:
> > 
> > void sk_stop_timer(struct sock *sk, struct timer_list* timer)
> > {
> >     if (del_timer(timer))
> >             __sock_put(sk);
> > }
> > 
> > If this is the case, then I'll add the following interface:
> > 
> >    del_timer_sync_shutdown() // the common case which syncs
> > 
> >    del_timer_shutdown() // the uncommon case, that returns immediately
> >                         // used for those cases that add extra code to
> >                         // handle it, like sk_stop_timer()
> 
> Sorry too many bugs at once :)
> 
> FWIW Paolo was saying privately earlier today that he spotted some cases
> of reuse, he gave an example of ccid2_hc_tx_packet_recv()

For the records, there are other cases, e.g. after sk_stop_timer() in 
clear_3rdack_retransmission() (mptcp code) the timer can be-rearmed
without re-initializing. I *think* there are more of such use in the 
in ax25/rose code.

> So we can't convert all cases of sk_stop_timer() in one fell swoop :(

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

Cheers,

Paolo


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