[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <xhsmhmspu8zlj.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Mon, 15 Apr 2024 16:33:44 +0200
From: Valentin Schneider <vschneid@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: dccp@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org, "David S.
Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo
Abeni <pabeni@...hat.com>, mleitner@...hat.com, David Ahern
<dsahern@...nel.org>, Juri Lelli <juri.lelli@...hat.com>, Tomas Glozar
<tglozar@...hat.com>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v5 0/2] tcp/dcpp: Un-pin tw_timer
On 15/04/24 14:35, Eric Dumazet wrote:
> On Mon, Apr 15, 2024 at 1:34 PM Valentin Schneider <vschneid@...hat.com> wrote:
>>
>> Hi,
>>
>> This is v5 of the series where the tw_timer is un-pinned to get rid of
>> interferences in isolated CPUs setups.
>>
>> The first patch is a new one stemming from Jakub's bug reported. It's there
>> mainly to make the reviewing a bit easier, but as it changes behaviour it should
>> be squashed with the second one.
>>
>> Revisions
>> =========
>>
>> v4 -> v5
>> ++++++++
>>
>> o Rebased against latest Linus' tree
>> o Converted tw_timer into a delayed work following Jakub's bug report on v4
>> http://lore.kernel.org/r/20240411100536.224fa1e7@kernel.org
>
> What was the issue again ?
>
> Please explain precisely why it was fundamentally tied to the use of
> timers (and this was not possible to fix the issue without
> adding work queues and more dependencies to TCP stack)
In v4 I added the use of the ehash lock to serialize arming the timewait
timer vs destroying it (inet_twsk_schedule() vs inet_twsk_deschedule_put()).
Unfortunately, holding a lock both in a timer callback and in the context
in which it is destroyed is invalid. AIUI the issue is as follows:
CPUx CPUy
spin_lock(foo);
<timer fires>
call_timer_fn()
spin_lock(foo) // blocks
timer_shutdown_sync()
__timer_delete_sync()
__try_to_del_timer_sync() // looped as long as timer is running
<deadlock>
In our case, we had in v4:
inet_twsk_deschedule_put()
spin_lock(ehash_lock);
tw_timer_handler()
inet_twsk_kill()
spin_lock(ehash_lock);
__inet_twsk_kill();
timer_shutdown_sync(&tw->tw_timer);
The fix here is to move the timer deletion to a non-timer
context. Workqueues fit the bill, and as the tw_timer_handler() would just queue
a work item, I converted it to a delayed_work.
Powered by blists - more mailing lists