linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Andy Lutomirski <luto@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Anton Blanchard <anton@ozlabs.org>, Arnd Bergmann <arnd@arndb.de>,
	linux-arch <linux-arch@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Peter Zijlstra <peterz@infradead.org>, x86 <x86@kernel.org>
Subject: Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode
Date: Tue, 14 Jul 2020 02:37:52 +1000	[thread overview]
Message-ID: <1594657848.8og86nopq6.astroid@bobo.none> (raw)
In-Reply-To: <CALCETrUHsYp0oGAiy3N-yAauPyx2nKqp1AiETgSJWc77GwO-Sg@mail.gmail.com>

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@efficios.com> wrote:
>>
>> ----- On Jul 13, 2020, at 9:47 AM, Nicholas Piggin npiggin@gmail.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


  reply	other threads:[~2020-07-13 16:38 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-10  1:56 [RFC PATCH 0/7] mmu context cleanup, lazy tlb cleanup, Nicholas Piggin
2020-07-10  1:56 ` [RFC PATCH 1/7] asm-generic: add generic MMU versions of mmu context functions Nicholas Piggin
2020-07-10  1:56 ` [RFC PATCH 2/7] arch: use asm-generic mmu context for no-op implementations Nicholas Piggin
2020-07-10  1:56 ` [RFC PATCH 3/7] mm: introduce exit_lazy_tlb Nicholas Piggin
2020-07-10  1:56 ` [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode Nicholas Piggin
2020-07-10  9:42   ` Peter Zijlstra
2020-07-10 14:02   ` Mathieu Desnoyers
2020-07-10 17:04   ` Andy Lutomirski
2020-07-13  4:45     ` Nicholas Piggin
2020-07-13 13:47       ` Nicholas Piggin
2020-07-13 14:13         ` Mathieu Desnoyers
2020-07-13 15:48           ` Andy Lutomirski
2020-07-13 16:37             ` Nicholas Piggin [this message]
2020-07-16  4:15           ` Nicholas Piggin
2020-07-16  4:42             ` Nicholas Piggin
2020-07-16 15:46               ` Mathieu Desnoyers
2020-07-16 16:03                 ` Mathieu Desnoyers
2020-07-16 18:58                   ` Mathieu Desnoyers
2020-07-16 21:24                     ` Alan Stern
2020-07-17 13:39                       ` Mathieu Desnoyers
2020-07-17 14:51                         ` Alan Stern
2020-07-17 15:39                           ` Mathieu Desnoyers
2020-07-17 16:11                             ` Alan Stern
2020-07-17 16:22                               ` Mathieu Desnoyers
2020-07-17 17:44                                 ` Alan Stern
2020-07-17 17:52                                   ` Mathieu Desnoyers
2020-07-17  0:00                     ` Nicholas Piggin
2020-07-16  5:18             ` Andy Lutomirski
2020-07-16  6:06               ` Nicholas Piggin
2020-07-16  8:50               ` Peter Zijlstra
2020-07-16 10:03                 ` Nicholas Piggin
2020-07-16 11:00                   ` peterz
2020-07-16 15:34                     ` Mathieu Desnoyers
2020-07-16 23:26                     ` Nicholas Piggin
2020-07-17 13:42                       ` Mathieu Desnoyers
2020-07-20  3:03                         ` Nicholas Piggin
2020-07-20 16:46                           ` Mathieu Desnoyers
2020-07-21 10:04                             ` Nicholas Piggin
2020-07-21 13:11                               ` Mathieu Desnoyers
2020-07-21 14:30                                 ` Nicholas Piggin
2020-07-21 15:06                               ` peterz
2020-07-21 15:15                                 ` Mathieu Desnoyers
2020-07-21 15:19                                   ` Peter Zijlstra
2020-07-21 15:22                                     ` Mathieu Desnoyers
2020-07-10  1:56 ` [RFC PATCH 5/7] lazy tlb: introduce lazy mm refcount helper functions Nicholas Piggin
2020-07-10  9:48   ` Peter Zijlstra
2020-07-10  1:56 ` [RFC PATCH 6/7] lazy tlb: allow lazy tlb mm switching to be configurable Nicholas Piggin
2020-07-10  1:56 ` [RFC PATCH 7/7] lazy tlb: shoot lazies, a non-refcounting lazy tlb option Nicholas Piggin
2020-07-10  9:35   ` Peter Zijlstra
2020-07-13  4:58     ` Nicholas Piggin
2020-07-13 15:59   ` Andy Lutomirski
2020-07-13 16:48     ` Nicholas Piggin
2020-07-13 18:18       ` Andy Lutomirski
2020-07-14  5:04         ` Nicholas Piggin
2020-07-14  6:31           ` Nicholas Piggin
2020-07-14 12:46             ` Andy Lutomirski
2020-07-14 13:23               ` Peter Zijlstra
2020-07-16  2:26               ` Nicholas Piggin
2020-07-16  2:35               ` Nicholas Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1594657848.8og86nopq6.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=anton@ozlabs.org \
    --cc=arnd@arndb.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=peterz@infradead.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).