[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1594657848.8og86nopq6.astroid@bobo.none>
Date: Tue, 14 Jul 2020 02:37:52 +1000
From: Nicholas Piggin <npiggin@...il.com>
To: Andy Lutomirski <luto@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: Anton Blanchard <anton@...abs.org>, Arnd Bergmann <arnd@...db.de>,
linux-arch <linux-arch@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-mm <linux-mm@...ck.org>,
linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
Peter Zijlstra <peterz@...radead.org>, x86 <x86@...nel.org>
Subject: Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than
membarrier_mm_sync_core_before_usermode
Excerpts from Andy Lutomirski's message of July 14, 2020 1:48 am:
> On Mon, Jul 13, 2020 at 7:13 AM Mathieu Desnoyers
> <mathieu.desnoyers@...icios.com> wrote:
>>
>> ----- On Jul 13, 2020, at 9:47 AM, Nicholas Piggin npiggin@...il.com wrote:
>>
>> > Excerpts from Nicholas Piggin's message of July 13, 2020 2:45 pm:
>> >> Excerpts from Andy Lutomirski's message of July 11, 2020 3:04 am:
>> >>> Also, as it stands, I can easily see in_irq() ceasing to promise to
>> >>> serialize. There are older kernels for which it does not promise to
>> >>> serialize. And I have plans to make it stop serializing in the
>> >>> nearish future.
>> >>
>> >> You mean x86's return from interrupt? Sounds fun... you'll konw where to
>> >> update the membarrier sync code, at least :)
>> >
>> > Oh, I should actually say Mathieu recently clarified a return from
>> > interrupt doesn't fundamentally need to serialize in order to support
>> > membarrier sync core.
>>
>> Clarification to your statement:
>>
>> Return from interrupt to kernel code does not need to be context serializing
>> as long as kernel serializes before returning to user-space.
>>
>> However, return from interrupt to user-space needs to be context serializing.
>>
>
> Indeed, and I figured this out on the first read through because I'm
> quite familiar with the x86 entry code. But Nick somehow missed this,
> and Nick is the one who wrote the patch.
>
> Nick, I think this helps prove my point. The code you're submitting
> may well be correct, but it's unmaintainable.
It's not. The patch I wrote for x86 is a no-op, it just moves existing
x86 hook and code that's already there to a different name.
Actually it's not quite a no-op, it't changes it to use hooks that are
actually called in the right places. Because previously it was
unmaintainable from point of view of generic mm -- it was not clear at
all that the old one should have been called in other places where the
mm goes non-lazy. Now with the exit_lazy_tlb hook, it can quite easily
be spotted where it is missing.
And x86 keeps their membarrier code in x86, and uses nice well defined
lazy tlb mm hooks.
> At the very least, this
> needs a comment explaining, from the perspective of x86, *exactly*
> what exit_lazy_tlb() is promising, why it's promising it, how it
> achieves that promise, and what code cares about it. Or we could do
> something with TIF flags and make this all less magical, although that
> will probably end up very slightly slower.
It's all documented there in existing comments plus the asm-generic
exit_lazy_tlb specification added AFAIKS.
Is the membarrier comment in finish_task_switch plus these ones not
enough?
Thanks,
Nick
Powered by blists - more mailing lists