Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH v3 1/2] alpha: do not clear remote MMU contexts in migrate_flush_tlb_page()
       [not found] <20261009212501.971158-1-linmag7@gmail.com>
@ 2026-10-09 21:19 ` Magnus Lindholm
  2026-10-10  1:34   ` Matt Turner
  2026-10-09 21:19 ` [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms Magnus Lindholm
  1 sibling, 1 reply; 5+ messages in thread
From: Magnus Lindholm @ 2026-10-09 21:19 UTC (permalink / raw)
  To: richard.henderson, mattst88, linux-kernel, linux-alpha; +Cc: linmag7, stable

After the on_each_cpu() rendezvous, migrate_flush_tlb_page() walks the
other CPUs and zeroes their mm->context[cpu] when mm_users is at most one,
described as mimicking flush_tlb_mm()'s mm_users<=1 optimization.

It is not one. flush_tlb_mm() tests mm_users before deciding whether to
send the IPIs; here every CPU has already been visited and waited for, so
nothing is saved. The callback has also just set each CPU's own slot
correctly, so the loop only overwrites it, and does so while that CPU may
still be running in the address space.

Remove it. The shootdown shortcuts in flush_tlb_mm(), flush_tlb_page()
and flush_icache_user_page() clear remote slots too; once the next patch
removes those as well, every runtime update of mm->context[cpu] is made by
CPU cpu itself. That patch relies on the invariant when it reads those
slots to decide whether a shootdown can be skipped, so this one is tagged
for stable as its prerequisite.

Fixes: dd5712f3379c ("alpha: fix user-space corruption during memory compaction")
Cc: stable@vger.kernel.org
Reviewed-by: Matt Turner <mattst88@gmail.com>
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
Message-ID: <20261008195608.965266-3-linmag7@gmail.com>
---
 arch/alpha/mm/tlbflush.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/arch/alpha/mm/tlbflush.c b/arch/alpha/mm/tlbflush.c
index ccbc317b9a34..239c72b8a741 100644
--- a/arch/alpha/mm/tlbflush.c
+++ b/arch/alpha/mm/tlbflush.c
@@ -90,22 +90,6 @@ void migrate_flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
 	 */
 	preempt_disable();
 	on_each_cpu(ipi_flush_mm_and_page, &d, 1);
-
-	/*
-	 * mimic flush_tlb_mm()'s mm_users<=1 optimization.
-	 */
-	if (atomic_read(&mm->mm_users) <= 1) {
-
-		int cpu, this_cpu;
-		this_cpu = smp_processor_id();
-
-		for (cpu = 0; cpu < NR_CPUS; cpu++) {
-			if (!cpu_online(cpu) || cpu == this_cpu)
-				continue;
-			if (READ_ONCE(mm->context[cpu]))
-				WRITE_ONCE(mm->context[cpu], 0);
-		}
-	}
 	preempt_enable();
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms
       [not found] <20261009212501.971158-1-linmag7@gmail.com>
  2026-10-09 21:19 ` [PATCH v3 1/2] alpha: do not clear remote MMU contexts in migrate_flush_tlb_page() Magnus Lindholm
@ 2026-10-09 21:19 ` Magnus Lindholm
  2026-10-10  1:35   ` Matt Turner
  1 sibling, 1 reply; 5+ messages in thread
From: Magnus Lindholm @ 2026-10-09 21:19 UTC (permalink / raw)
  To: richard.henderson, mattst88, linux-kernel, linux-alpha; +Cc: linmag7, stable

flush_tlb_mm(), flush_tlb_page() and flush_icache_user_page() skip the
shootdown IPI when mm_users <= 1, on the assumption that no other CPU can
be running the mm. A task that borrows an mm through kthread_use_mm()
takes mmgrab() rather than mmget(), so it never appears in mm_users, and
kthread_use_mm() may be handed an mm that is already the caller's
active_mm and loaded on that CPU. Such a CPU was not only left without
an IPI, its mm->context[cpu] was cleared from under it.

mm->context[cpu] is already the record of which CPUs hold an ASN for the
mm. Test it rather than clearing it: take the shortcut only when no
other CPU has one, and otherwise fall through to the IPI, which
invalidates those CPUs properly. A CPU that merely ran the mm in the
past also holds a context and now costs an IPI, which errs in the safe
direction.

Change ipi_flush_tlb_mm() and ipi_flush_icache_page() to test current->mm,
like ipi_flush_tlb_page(). A CPU that only retains the mm lazily then
clears its own slot through flush_tlb_other(), rather than publishing a
new ASN on every IPI. Once these historical slots are gone, the shortcut
is available again if mm_users remains at most one and no other CPU has
taken or kept a context. An explicit kthread_use_mm() borrower has
current->mm set and still receives the invalidate it needs.

Together with the preceding patch, dropping these clearing loops leaves
every runtime write to mm->context[] targeting the writing CPU's own slot,
so the array becomes a lockless publication of which CPUs may be using the
mm, with a single writer per slot. Mark the two stores that are now
observed from other CPUs; flush_tlb_other() already uses WRITE_ONCE().
The uniprocessor flush_icache_user_page() is left alone, as nothing reads
another CPU's slot there, and init_new_context() runs before the mm is
shared.

Order the slot reads after the page-table changes with smp_mb(). On the
publishing CPU, the store in ev5_switch_mm() precedes the scheduler's
post-switch barrier in finish_lock_switch(). Also issue smp_mb() after
publishing in __load_new_mm_context(), before loading and using the mm.
This covers every caller, including check_mmu_context(), which can
publish a replacement after finish_lock_switch() has already run.

This requires the preceding removal of remote slot clearing from
migrate_flush_tlb_page(). It also requires the stale-TLB series to set
need_new_asn on both the reused and newly allocated ASN paths. Otherwise
an IPI between finish_lock_switch() and check_mmu_context() can zero a
new slot while asn_lock is set, and the task can return to user space
with a live ASN that the shortcut cannot see.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
 arch/alpha/include/asm/mmu_context.h |  2 +-
 arch/alpha/kernel/smp.c              | 52 ++++++++++++++--------------
 arch/alpha/mm/fault.c                |  4 ++-
 3 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/arch/alpha/include/asm/mmu_context.h b/arch/alpha/include/asm/mmu_context.h
index e5a5737506db..f8a42f42a5dc 100644
--- a/arch/alpha/include/asm/mmu_context.h
+++ b/arch/alpha/include/asm/mmu_context.h
@@ -158,7 +158,7 @@ ev5_switch_mm(struct mm_struct *prev_mm, struct mm_struct *next_mm,
 	mmc = next_mm->context[cpu];
 	if ((mmc ^ asn) & ~HARDWARE_ASN_MASK) {
 		mmc = __get_new_mm_context(next_mm, cpu);
-		next_mm->context[cpu] = mmc;
+		WRITE_ONCE(next_mm->context[cpu], mmc);
 	}
 #ifdef CONFIG_SMP
 	/* A deferred shootdown can also invalidate a newly allocated ASN. */
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index e21bc3920bec..11768aa292a4 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -625,12 +625,30 @@ static void
 ipi_flush_tlb_mm(void *x)
 {
 	struct mm_struct *mm = x;
-	if (mm == current->active_mm && !asn_locked())
+	if (mm == current->mm && !asn_locked())
 		flush_tlb_current(mm);
 	else
 		flush_tlb_other(mm);
 }
 
+/* True if a CPU other than this one holds an ASN for MM.  */
+static bool
+mm_context_elsewhere(struct mm_struct *mm)
+{
+	int cpu, this_cpu = smp_processor_id();
+
+	/* Pairs with the barrier the publishing CPU issues before using MM.  */
+	smp_mb();
+
+	for_each_online_cpu(cpu) {
+		if (cpu == this_cpu)
+			continue;
+		if (READ_ONCE(mm->context[cpu]))
+			return true;
+	}
+	return false;
+}
+
 void
 flush_tlb_mm(struct mm_struct *mm)
 {
@@ -638,14 +656,8 @@ flush_tlb_mm(struct mm_struct *mm)
 
 	if (mm == current->active_mm) {
 		flush_tlb_current(mm);
-		if (atomic_read(&mm->mm_users) <= 1) {
-			int cpu, this_cpu = smp_processor_id();
-			for (cpu = 0; cpu < NR_CPUS; cpu++) {
-				if (!cpu_online(cpu) || cpu == this_cpu)
-					continue;
-				if (mm->context[cpu])
-					mm->context[cpu] = 0;
-			}
+		if (atomic_read(&mm->mm_users) <= 1 &&
+		    !mm_context_elsewhere(mm)) {
 			preempt_enable();
 			return;
 		}
@@ -690,14 +702,8 @@ flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
 	/* As in ipi_flush_tlb_page(): a targeted tbi() needs MM current.  */
 	if (mm == current->mm) {
 		flush_tlb_current_page(mm, vma, addr);
-		if (atomic_read(&mm->mm_users) <= 1) {
-			int cpu, this_cpu = smp_processor_id();
-			for (cpu = 0; cpu < NR_CPUS; cpu++) {
-				if (!cpu_online(cpu) || cpu == this_cpu)
-					continue;
-				if (mm->context[cpu])
-					mm->context[cpu] = 0;
-			}
+		if (atomic_read(&mm->mm_users) <= 1 &&
+		    !mm_context_elsewhere(mm)) {
 			preempt_enable();
 			return;
 		}
@@ -728,7 +734,7 @@ static void
 ipi_flush_icache_page(void *x)
 {
 	struct mm_struct *mm = (struct mm_struct *) x;
-	if (mm == current->active_mm && !asn_locked())
+	if (mm == current->mm && !asn_locked())
 		__load_new_mm_context(mm);
 	else
 		flush_tlb_other(mm);
@@ -747,14 +753,8 @@ flush_icache_user_page(struct vm_area_struct *vma, struct page *page,
 
 	if (mm == current->active_mm) {
 		__load_new_mm_context(mm);
-		if (atomic_read(&mm->mm_users) <= 1) {
-			int cpu, this_cpu = smp_processor_id();
-			for (cpu = 0; cpu < NR_CPUS; cpu++) {
-				if (!cpu_online(cpu) || cpu == this_cpu)
-					continue;
-				if (mm->context[cpu])
-					mm->context[cpu] = 0;
-			}
+		if (atomic_read(&mm->mm_users) <= 1 &&
+		    !mm_context_elsewhere(mm)) {
 			preempt_enable();
 			return;
 		}
diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index dfe427d93072..5b62bdeabe2a 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -69,7 +69,9 @@ __load_new_mm_context(struct mm_struct *next_mm)
 	struct pcb_struct *pcb;
 
 	mmc = __get_new_mm_context(next_mm, smp_processor_id());
-	next_mm->context[smp_processor_id()] = mmc;
+	WRITE_ONCE(next_mm->context[smp_processor_id()], mmc);
+	/* Publish the context before this CPU can access the mm. */
+	smp_mb();
 
 	pcb = &current_thread_info()->pcb;
 	pcb->asn = mmc & HARDWARE_ASN_MASK;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] alpha: do not clear remote MMU contexts in migrate_flush_tlb_page()
  2026-10-09 21:19 ` [PATCH v3 1/2] alpha: do not clear remote MMU contexts in migrate_flush_tlb_page() Magnus Lindholm
@ 2026-10-10  1:34   ` Matt Turner
  0 siblings, 0 replies; 5+ messages in thread
From: Matt Turner @ 2026-10-10  1:34 UTC (permalink / raw)
  To: Magnus Lindholm; +Cc: richard.henderson, linux-kernel, linux-alpha, stable

On Fri, Oct 9, 2026 at 11:19 PM Magnus Lindholm <linmag7@gmail.com> wrote:
> Fixes: dd5712f3379c ("alpha: fix user-space corruption during memory compaction")
> Cc: stable@vger.kernel.org
> Reviewed-by: Matt Turner <mattst88@gmail.com>
> Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
> Message-ID: <20261008195608.965266-3-linmag7@gmail.com>

The Message-ID trailer is the v2 posting's and should be dropped.

The patch is unchanged from v2, so my Reviewed-by stands.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms
  2026-10-09 21:19 ` [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms Magnus Lindholm
@ 2026-10-10  1:35   ` Matt Turner
  2026-10-10 14:45     ` Magnus Lindholm
  0 siblings, 1 reply; 5+ messages in thread
From: Matt Turner @ 2026-10-10  1:35 UTC (permalink / raw)
  To: Magnus Lindholm; +Cc: richard.henderson, linux-kernel, linux-alpha, stable

On Fri, Oct 9, 2026 at 11:19 PM Magnus Lindholm <linmag7@gmail.com> wrote:
> Change ipi_flush_tlb_mm() and ipi_flush_icache_page() to test current->mm,
> like ipi_flush_tlb_page(). A CPU that only retains the mm lazily then
> clears its own slot through flush_tlb_other(), rather than publishing a
> new ASN on every IPI. Once these historical slots are gone, the shortcut
> is available again if mm_users remains at most one and no other CPU has
> taken or kept a context.

ipi_flush_mm_and_page() in arch/alpha/mm/tlbflush.c still tests
current->active_mm, so a lazy CPU allocates and publishes a new ASN on
every migration rendezvous. The next flush then finds that slot, sends
IPIs, and the lazy CPU clears it again. Nothing breaks, but with
compaction running the shortcut is lost after each migrated page, which
is the case this paragraph says is gone.

I think the handler can take the same test as the other three:

diff --git a/arch/alpha/mm/tlbflush.c b/arch/alpha/mm/tlbflush.c
--- a/arch/alpha/mm/tlbflush.c
+++ b/arch/alpha/mm/tlbflush.c
@@ -66,7 +66,7 @@ static void ipi_flush_mm_and_page(void *x)
        struct tlb_mm_and_addr *d = x;

        /* Part 1: mm context side (Alpha uses ASN/context as a key
mechanism). */
-       if (d->mm == current->active_mm && !asn_locked())
+       if (d->mm == current->mm && !asn_locked())
                __load_new_mm_context(d->mm);
        else
                flush_tlb_other(d->mm);

A lazy CPU then clears its slot and still does the tbi() below. Any way
back to the mm from there gets a new ASN: ev5_switch_mm() sees version
0, and kthread_use_mm() goes through the next == current path. I have
not tested that change.

The callers have the same asymmetry. flush_tlb_mm() and
flush_icache_user_page() test current->active_mm, so a kernel thread
that flushes an mm it only holds lazily publishes its own slot. That
one also takes the shortcut in that branch, so I would leave it alone,
but the changelog could say so.

> Order the slot reads after the page-table changes with smp_mb(). On the
> publishing CPU, the store in ev5_switch_mm() precedes the scheduler's
> post-switch barrier in finish_lock_switch().

The barrier there is the mb() in alpha's arch_spin_unlock(). The
scheduler does not promise a full barrier when it drops the rq lock, so
this is an alpha property, and nothing at the store says it is being
relied on. Could the changelog name it, and the store get a comment?
Something like:

                mmc = __get_new_mm_context(next_mm, cpu);
+               /*
+                * Ordered before any use of the mm by the mb() in
+                * arch_spin_unlock(), when finish_lock_switch() drops
+                * the rq lock.
+                */
                WRITE_ONCE(next_mm->context[cpu], mmc);

The rest looks right to me. I went through the window between
finish_lock_switch() and check_mmu_context(): an IPI there clears the
slot under asn_lock, a flush on another CPU can then take the shortcut,
and check_mmu_context() sees the zero and loads a new context behind
the new smp_mb(). The three points from my v2 review are addressed.

The fork numbers in the cover letter are from before the handler
change. It would be good to have them again for this version, along
with an SMP run under compaction.

With ipi_flush_mm_and_page() changed, or the changelog reworded to
match what it does:

Reviewed-by: Matt Turner <mattst88@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms
  2026-10-10  1:35   ` Matt Turner
@ 2026-10-10 14:45     ` Magnus Lindholm
  0 siblings, 0 replies; 5+ messages in thread
From: Magnus Lindholm @ 2026-10-10 14:45 UTC (permalink / raw)
  To: Matt Turner; +Cc: richard.henderson, linux-kernel, linux-alpha, stable

Hi Matt,

On Sat, Oct 10, 2026 at 3:35 AM Matt Turner <mattst88@gmail.com> wrote:
>
> On Fri, Oct 9, 2026 at 11:19 PM Magnus Lindholm <linmag7@gmail.com> wrote:
> > Change ipi_flush_tlb_mm() and ipi_flush_icache_page() to test current->mm,
> > like ipi_flush_tlb_page(). A CPU that only retains the mm lazily then
> > clears its own slot through flush_tlb_other(), rather than publishing a
> > new ASN on every IPI. Once these historical slots are gone, the shortcut
> > is available again if mm_users remains at most one and no other CPU has
> > taken or kept a context.
>
> ipi_flush_mm_and_page() in arch/alpha/mm/tlbflush.c still tests
> current->active_mm, so a lazy CPU allocates and publishes a new ASN on
> every migration rendezvous. The next flush then finds that slot, sends
> IPIs, and the lazy CPU clears it again. Nothing breaks, but with
> compaction running the shortcut is lost after each migrated page, which
> is the case this paragraph says is gone.
>
> I think the handler can take the same test as the other three:
>
> diff --git a/arch/alpha/mm/tlbflush.c b/arch/alpha/mm/tlbflush.c
> --- a/arch/alpha/mm/tlbflush.c
> +++ b/arch/alpha/mm/tlbflush.c
> @@ -66,7 +66,7 @@ static void ipi_flush_mm_and_page(void *x)
>         struct tlb_mm_and_addr *d = x;
>
>         /* Part 1: mm context side (Alpha uses ASN/context as a key
> mechanism). */
> -       if (d->mm == current->active_mm && !asn_locked())
> +       if (d->mm == current->mm && !asn_locked())
>                 __load_new_mm_context(d->mm);
>         else
>                 flush_tlb_other(d->mm);
>

Well, spotted! thanks!

> A lazy CPU then clears its slot and still does the tbi() below. Any way
> back to the mm from there gets a new ASN: ev5_switch_mm() sees version
> 0, and kthread_use_mm() goes through the next == current path. I have
> not tested that change.
>
> The callers have the same asymmetry. flush_tlb_mm() and
> flush_icache_user_page() test current->active_mm, so a kernel thread
> that flushes an mm it only holds lazily publishes its own slot. That
> one also takes the shortcut in that branch, so I would leave it alone,
> but the changelog could say so.
>
> > Order the slot reads after the page-table changes with smp_mb(). On the
> > publishing CPU, the store in ev5_switch_mm() precedes the scheduler's
> > post-switch barrier in finish_lock_switch().
>
> The barrier there is the mb() in alpha's arch_spin_unlock(). The
> scheduler does not promise a full barrier when it drops the rq lock, so
> this is an alpha property, and nothing at the store says it is being
> relied on. Could the changelog name it, and the store get a comment?
> Something like:
>
>                 mmc = __get_new_mm_context(next_mm, cpu);
> +               /*
> +                * Ordered before any use of the mm by the mb() in
> +                * arch_spin_unlock(), when finish_lock_switch() drops
> +                * the rq lock.
> +                */
>                 WRITE_ONCE(next_mm->context[cpu], mmc);
>

Agree.

> The rest looks right to me. I went through the window between
> finish_lock_switch() and check_mmu_context(): an IPI there clears the
> slot under asn_lock, a flush on another CPU can then take the shortcut,
> and check_mmu_context() sees the zero and loads a new context behind
> the new smp_mb(). The three points from my v2 review are addressed.
>
> The fork numbers in the cover letter are from before the handler
> change. It would be good to have them again for this version, along
> with an SMP run under compaction.
>
> With ipi_flush_mm_and_page() changed, or the changelog reworded to
> match what it does:
>
> Reviewed-by: Matt Turner <mattst88@gmail.com>


Thanks

Magnus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-10 14:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20261009212501.971158-1-linmag7@gmail.com>
2026-10-09 21:19 ` [PATCH v3 1/2] alpha: do not clear remote MMU contexts in migrate_flush_tlb_page() Magnus Lindholm
2026-10-10  1:34   ` Matt Turner
2026-10-09 21:19 ` [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms Magnus Lindholm
2026-10-10  1:35   ` Matt Turner
2026-10-10 14:45     ` Magnus Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox