All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	stable@vger.kernel.org
Subject: [PATCH v3 1/4] x86/membarrier: Get rid of a dubious optimization
Date: Thu,  3 Dec 2020 21:07:03 -0800	[thread overview]
Message-ID: <5afc7632be1422f91eaf7611aaaa1b5b8580a086.1607058304.git.luto@kernel.org> (raw)
In-Reply-To: <cover.1607058304.git.luto@kernel.org>

sync_core_before_usermode() had an incorrect optimization.  If we're
in an IRQ, we can get to usermode without IRET -- we just have to
schedule to a different task in the same mm and do SYSRET.
Fortunately, there were no callers of sync_core_before_usermode()
that could have had in_irq() or in_nmi() equal to true, because it's
only ever called from the scheduler.

While we're at it, clarify a related comment.

Cc: stable@vger.kernel.org
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/sync_core.h |  9 +++++----
 arch/x86/mm/tlb.c                | 10 ++++++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/sync_core.h b/arch/x86/include/asm/sync_core.h
index 0fd4a9dfb29c..ab7382f92aff 100644
--- a/arch/x86/include/asm/sync_core.h
+++ b/arch/x86/include/asm/sync_core.h
@@ -98,12 +98,13 @@ static inline void sync_core_before_usermode(void)
 	/* With PTI, we unconditionally serialize before running user code. */
 	if (static_cpu_has(X86_FEATURE_PTI))
 		return;
+
 	/*
-	 * Return from interrupt and NMI is done through iret, which is core
-	 * serializing.
+	 * Even if we're in an interrupt, we might reschedule before returning,
+	 * in which case we could switch to a different thread in the same mm
+	 * and return using SYSRET or SYSEXIT.  Instead of trying to keep
+	 * track of our need to sync the core, just sync right away.
 	 */
-	if (in_irq() || in_nmi())
-		return;
 	sync_core();
 }
 
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 11666ba19b62..569ac1d57f55 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -474,8 +474,14 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 	/*
 	 * The membarrier system call requires a full memory barrier and
 	 * core serialization before returning to user-space, after
-	 * storing to rq->curr. Writing to CR3 provides that full
-	 * memory barrier and core serializing instruction.
+	 * storing to rq->curr, when changing mm.  This is because
+	 * membarrier() sends IPIs to all CPUs that are in the target mm
+	 * to make them issue memory barriers.  However, if another CPU
+	 * switches to/from the target mm concurrently with
+	 * membarrier(), it can cause that CPU not to receive an IPI
+	 * when it really should issue a memory barrier.  Writing to CR3
+	 * provides that full memory barrier and core serializing
+	 * instruction.
 	 */
 	if (real_prev == next) {
 		VM_WARN_ON(this_cpu_read(cpu_tlbstate.ctxs[prev_asid].ctx_id) !=
-- 
2.28.0


  reply	other threads:[~2020-12-04  5:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-04  5:07 [PATCH v3 0/4] membarrier fixes Andy Lutomirski
2020-12-04  5:07 ` Andy Lutomirski [this message]
2020-12-09  4:10   ` [PATCH v3 1/4] x86/membarrier: Get rid of a dubious optimization Andy Lutomirski
2020-12-09  8:42   ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-12-04  5:07 ` [PATCH v3 2/4] membarrier: Add an actual barrier before rseq_preempt() Andy Lutomirski
2020-12-09  4:12   ` Andy Lutomirski
2020-12-09  8:42   ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-12-14 18:05     ` Andy Lutomirski
2020-12-04  5:07 ` [PATCH v3 3/4] membarrier: Explicitly sync remote cores when SYNC_CORE is requested Andy Lutomirski
2020-12-09  4:13   ` Andy Lutomirski
2020-12-09  8:42   ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-12-04  5:07 ` [PATCH v3 4/4] membarrier: Execute SYNC_CORE on the calling thread Andy Lutomirski
2020-12-04 19:35   ` Mathieu Desnoyers
2020-12-09  4:15   ` Andy Lutomirski
2020-12-09  8:42   ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski

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=5afc7632be1422f91eaf7611aaaa1b5b8580a086.1607058304.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=stable@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.