lkml.org 
[lkml]   [2023]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC PATCH] tcp/dcpp: Un-pin tw_timer
    Date
    On 16/10/23 17:40, Eric Dumazet wrote:
    > On Mon, Oct 16, 2023 at 3:00 PM Valentin Schneider <vschneid@redhat.com> wrote:
    >>
    >> The TCP timewait timer is proving to be problematic for setups where scheduler
    >> CPU isolation is achieved at runtime via cpusets (as opposed to statically via
    >> isolcpus=domains).
    >>
    >> What happens there is a CPU goes through tcp_time_wait(), arming the time_wait
    >> timer, then gets isolated. TCP_TIMEWAIT_LEN later, the timer fires, causing
    >> interference for the now-isolated CPU. This is conceptually similar to the issue
    >> described in
    >> e02b93124855 ("workqueue: Unbind kworkers before sending them to exit()")
    >>
    >> Making the timer un-pinned would resolve this, as it would be queued onto
    >> HK_FLAG_TIMER CPUs. It would Unfortunately go against
    >> ed2e92394589 ("tcp/dccp: fix timewait races in timer handling")
    >> as we'd need to arm the timer after the *hashdance() to not have it fire before
    >> we've finished setting up the timewait_socket.
    >>
    >> However, looking into this, I cannot grok what race is fixed by having the timer
    >> *armed* before the hashdance.
    >
    > That was because :
    >
    > 1) the timer could expire before we had a chance to set refcnt to
    > a non zero value. I guess this is fine if we use an extra atomic decrement.
    >
    > OR
    >
    > 2) another cpu could find the TW and delete it (trying to cancel the
    > tw_timer) before
    > we could arm the timer. ( inet_twsk_deschedule_put() is using
    > del_timer_sync() followed by inet_twsk_kill())
    >
    > Thus the tw timer would be armed for 60 seconds, then we would have to
    > wait for the timer to really
    > get rid of the tw structure.
    >
    > I think you also need to change inet_twsk_deschedule_put() logic ?
    >

    Gotcha, thank you for pointing it out.

    >> Keep softirqs disabled, but make the timer un-pinned and arm it after the
    >> hashdance. Remote CPUs may start using the timewait socket before the timer is
    >> armed, but their execution of __inet_lookup_established() won't prevent the
    >> arming of the timer.
    >
    > OK, I guess we can live with the following race :
    >
    > CPU0
    >
    > allocates a tw, insert it in hash table
    >
    > CPU1: finds the TW and removes it (timer
    > cancel does nothing)
    >
    > CPU0
    > arms a TW timer, lasting
    >

    Looks reasonable to me, I'll go write v2.

    Thanks for the help!

    \
     
     \ /
      Last update: 2023-10-18 16:58    [W:4.751 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site