From: Andy Lutomirski <luto@kernel.org>
To: x86@kernel.org, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
Nicholas Piggin <npiggin@gmail.com>,
Arnd Bergmann <arnd@arndb.de>, Anton Blanchard <anton@ozlabs.org>,
Andy Lutomirski <luto@kernel.org>
Subject: [PATCH 0/3] membarrier fixes
Date: Mon, 30 Nov 2020 09:50:32 -0800 [thread overview]
Message-ID: <cover.1606758530.git.luto@kernel.org> (raw)
Hi all-
x86's sync_core_before_usermode() was bogus. Without the other
patches applied, it would never be called in a problematic context,
but that's about to change. In any event, sync_core_before_usermode()
should be correct.
The second patch fixes a minor issue, but it also makes the third patch
nicer.
The third patch is the biggie. The old code looped over all CPUs
without disabling migration, and it skipped the current CPU. There
were comments about how the scheduler's barriers made this okay. This
may well be true, but it was a mess, and it's considerably simpler to
treat the current CPU just like all other CPUs.
The messy skip-the-current-CPU code masked what seems to be a couple
of much bigger issues: if the membarrier() syscall ran all the way
through _without_ being preempted, it completely failed to operate on
the calling thread. The smp_mb() calls sprinkled through the function
would mask this problem for the normal barrier mode, but they wouldn't
help for the core-serializing mode or rseq_preempt mode. In other
words, modifying some code, calling
membarrier(MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE, 0), and running
that modified code was not actually safe. This seems to rather defeat
the purpose.
Some or all of this might be -stable material.
The global_expedited code looks similarly nasty. Any volunteers to
clean it up?
Andy Lutomirski (3):
x86/membarrier: Get rid of a dubious optimization
membarrier: Add an actual barrier before rseq_preempt()
membarrier: Propagate SYNC_CORE and RSEQ actions more carefully
arch/x86/include/asm/sync_core.h | 9 +--
arch/x86/mm/tlb.c | 6 +-
kernel/sched/membarrier.c | 102 ++++++++++++++++++++++---------
3 files changed, 83 insertions(+), 34 deletions(-)
--
2.28.0
next reply other threads:[~2020-11-30 17:51 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-30 17:50 Andy Lutomirski [this message]
2020-11-30 17:50 ` [PATCH 1/3] x86/membarrier: Get rid of a dubious optimization Andy Lutomirski
2020-12-01 14:39 ` Mathieu Desnoyers
2020-12-01 17:47 ` Andy Lutomirski
2020-11-30 17:50 ` [PATCH 2/3] membarrier: Add an actual barrier before rseq_preempt() Andy Lutomirski
2020-12-01 10:06 ` Peter Zijlstra
2020-12-01 14:31 ` Mathieu Desnoyers
2020-12-01 17:55 ` Andy Lutomirski
2020-11-30 17:50 ` [PATCH 3/3] membarrier: Propagate SYNC_CORE and RSEQ actions more carefully Andy Lutomirski
2020-12-01 10:16 ` Peter Zijlstra
2020-12-01 14:28 ` Mathieu Desnoyers
2020-12-01 18:12 ` Andy Lutomirski
2020-12-01 18:29 ` Mathieu Desnoyers
2020-12-01 18:48 ` Andy Lutomirski
2020-12-01 20:51 ` Mathieu Desnoyers
2020-12-01 18:09 ` Andy Lutomirski
2020-12-01 18:53 ` Peter Zijlstra
2020-12-01 18:55 ` Peter Zijlstra
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=cover.1606758530.git.luto@kernel.org \
--to=luto@kernel.org \
--cc=anton@ozlabs.org \
--cc=arnd@arndb.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=npiggin@gmail.com \
--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).