On Thu, 27 Oct 2022 13:15:23 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Oct 27, 2022 at 12:55 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> > I think we need to update this code to squeeze in a del_timer_shutdown() to
> > make sure that the timers are never restarted.
>
> So the reason the networking code does this is that it can't just do
> the old 'sync()' thing, the timers are deleted in contexts where that
> isn't valid.
>
> Which is also afaik why the networking code does that whole "timer
> implies a refcount to the socket" and then does the
>
> if (del_timer(timer))
> sock_put()
>
> thing (ie if the del_timer failed - possibly because it was already
> running - you leave the refcount alone).
OK, so the above is assuming that the timer is always active, and
del_timer() returns if it successfully removed it (where it can call
sock_put()), but if del_timer() returns 0, that means the timer is
currently running (or about to be), so it doesn't call sock_put().
>
> So the networking code cannot do the del_timer_shutdown() for the same
> reason it cannot do the del_timer_sync(): it can't afford to wait for
> the timer to stop running.
>
> I suspect it needs something like a new "del_timer_shutdown_async()"
> that isn't synchronous, but does that
>
> - acts as del_timer in that it doesn't wait, and returns a success if
> it could just remove the pending case
>
> - does that "mark timer for shutdown" in that success case
>
> or something similar.
>
What about del_timer_try_shutdown(), that if it removes the timer, it sets
the function to NULL (making it equivalent to a successful shutdown),
otherwise it does nothing. Allowing the the timer to be rearmed.
I think this would work in this case.
-- Steve
|