All of lore.kernel.org
 help / color / mirror / Atom feed
From: "tip-bot2 for Andy Lutomirski" <tip-bot2@linutronix.de>
To: linux-tip-commits@vger.kernel.org
Cc: Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	stable@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org
Subject: [tip: x86/urgent] x86/membarrier: Get rid of a dubious optimization
Date: Wed, 09 Dec 2020 08:42:47 -0000	[thread overview]
Message-ID: <160750336786.3364.17691454567808013905.tip-bot2@tip-bot2> (raw)
In-Reply-To: <5afc7632be1422f91eaf7611aaaa1b5b8580a086.1607058304.git.luto@kernel.org>

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     a493d1ca1a03b532871f1da27f8dbda2b28b04c4
Gitweb:        https://git.kernel.org/tip/a493d1ca1a03b532871f1da27f8dbda2b28b04c4
Author:        Andy Lutomirski <luto@kernel.org>
AuthorDate:    Thu, 03 Dec 2020 21:07:03 -08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 09 Dec 2020 09:37:42 +01:00

x86/membarrier: Get rid of a dubious optimization

sync_core_before_usermode() had an incorrect optimization.  If the kernel
returns from an interrupt, it can get to usermode without IRET. It just has
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 at it, clarify a related comment.

Fixes: 70216e18e519 ("membarrier: Provide core serializing command, *_SYNC_CORE")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/5afc7632be1422f91eaf7611aaaa1b5b8580a086.1607058304.git.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 0fd4a9d..ab7382f 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 11666ba..569ac1d 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) !=

  parent reply	other threads:[~2020-12-09  8:44 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 ` [PATCH v3 1/4] x86/membarrier: Get rid of a dubious optimization Andy Lutomirski
2020-12-09  4:10   ` Andy Lutomirski
2020-12-09  8:42   ` tip-bot2 for Andy Lutomirski [this message]
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=160750336786.3364.17691454567808013905.tip-bot2@tip-bot2 \
    --to=tip-bot2@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --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.