* [PATCH v2 1/7] alpha: run check_mmu_context() from finish_arch_post_lock_switch()
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 2/7] alpha: only use a targeted tbi() when the target mm is really current Magnus Lindholm
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
check_mmu_context() clears asn_lock and acts on need_new_asn, but it
runs only as the tail of switch_to(), after alpha_switch_to() returns.
A newly forked task never gets there: its first context switch resumes
at ret_from_fork, which goes to schedule_tail() and then to user space
rather than returning to the code following alpha_switch_to(). A new
kernel thread reaches schedule_tail() the same way, through
ret_from_kernel_thread().
asn_lock is left set on that CPU, so the forked task runs user space
with it set and interrupts enabled. A TLB shootdown IPI arriving in
that window takes the deferred path, and the need_new_asn handshake
meant to cover that never runs.
finish_task_switch() calls finish_arch_post_lock_switch() with
preemption disabled, on the CPU that ran switch_mm(), so hooking
check_mmu_context() there completes the bookkeeping for both. The
existing call from switch_to() then becomes redundant, since
finish_task_switch() runs immediately afterwards and does the same
work, so drop it.
kthread_use_mm() and sched_force_init_mm() reach the same hook outside
the scheduler's preemption-disabled switch tail, where the CPU may have
changed since switch_mm(). check_mmu_context() acts on per-CPU state,
so testing preemptible() expresses the required condition directly
rather than naming particular callers.
One consequence is worth stating, because the TLB patches that follow
depend on it. After kthread_use_mm() the hook does nothing, so the
asn_lock that ev5_switch_mm() set stays on that CPU until the next real
context switch. A borrowed mm therefore has current->mm set while its
context is not loaded, and the shootdown handlers rely on asn_locked()
being true to take the conservative path for the whole of that window.
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 | 8 ++++++++
arch/alpha/include/asm/switch_to.h | 1 -
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/include/asm/mmu_context.h b/arch/alpha/include/asm/mmu_context.h
index eee8fe836a59..825d3b9605c9 100644
--- a/arch/alpha/include/asm/mmu_context.h
+++ b/arch/alpha/include/asm/mmu_context.h
@@ -181,6 +181,14 @@ do { \
#define check_mmu_context() do { } while(0)
#endif
+/* Per-CPU state: only safe while still on the switching CPU. */
+#define finish_arch_post_lock_switch finish_arch_post_lock_switch
+static inline void finish_arch_post_lock_switch(void)
+{
+ if (!preemptible())
+ check_mmu_context();
+}
+
__EXTERN_INLINE void
ev5_activate_mm(struct mm_struct *prev_mm, struct mm_struct *next_mm)
{
diff --git a/arch/alpha/include/asm/switch_to.h b/arch/alpha/include/asm/switch_to.h
index 762b7f975310..35c4b2c9d992 100644
--- a/arch/alpha/include/asm/switch_to.h
+++ b/arch/alpha/include/asm/switch_to.h
@@ -9,7 +9,6 @@ extern struct task_struct *alpha_switch_to(unsigned long, struct task_struct *);
#define switch_to(P,N,L) \
do { \
(L) = alpha_switch_to(virt_to_phys(&task_thread_info(N)->pcb), (P)); \
- check_mmu_context(); \
} while (0)
#endif /* __ALPHA_SWITCH_TO_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/7] alpha: only use a targeted tbi() when the target mm is really current
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 1/7] alpha: run check_mmu_context() from finish_arch_post_lock_switch() Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 3/7] alpha: fix the local TLB invalidate in flush_tlb_page() Magnus Lindholm
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
ipi_flush_tlb_page() gates a targeted tbi() on current->active_mm.
tbi() acts on the address space context currently loaded on the CPU, so
it reaches an mm's translations only when a thread of that mm is running
there. current->active_mm is not sufficient: under lazy TLB an idle or
kernel task keeps an mm as its active_mm while a different ASN is
loaded, so the invalidate hits the wrong context and the stale entry
survives. Nothing retires the old ASN afterwards either,
mm->context[cpu] still being valid, so the resuming thread can reuse it.
Test current->mm instead and otherwise fall back to flush_tlb_other(),
which clears mm->context[cpu] and forces a fresh ASN at the next switch
whatever is loaded now.
current->mm can be set while the mm's context is not loaded: a task that
borrowed an mm through kthread_use_mm() has current->mm set, but
ev5_switch_mm() only prepared the PCB and the asn_lock it set is still
held, as described in the previous patch. The !asn_locked() test keeps
this path conservative for the whole of that window.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
arch/alpha/kernel/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index ed06367ece57..1ad448105201 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -669,7 +669,8 @@ ipi_flush_tlb_page(void *x)
struct flush_tlb_page_struct *data = x;
struct mm_struct * mm = data->mm;
- if (mm == current->active_mm && !asn_locked())
+ /* A targeted tbi() needs a thread of MM to be current. */
+ if (mm == current->mm && !asn_locked())
flush_tlb_current_page(mm, data->vma, data->addr);
else
flush_tlb_other(mm);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 3/7] alpha: fix the local TLB invalidate in flush_tlb_page()
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 1/7] alpha: run check_mmu_context() from finish_arch_post_lock_switch() Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 2/7] alpha: only use a targeted tbi() when the target mm is really current Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 4/7] alpha: invalidate the local context " Magnus Lindholm
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
flush_tlb_page() invalidates the calling CPU itself before asking the
others, and gates that on current->active_mm. For a non-executable vma
that means a targeted tbi(2, addr), which acts on the context currently
loaded, so as in ipi_flush_tlb_page() it reaches nothing when only
active_mm names the mm, and nothing forces the old ASN to be retired
afterwards.
Test current->mm instead.
Reached in practice by folio_mkclean() from the writeback flusher kworker,
which has no mm of its own: about half the calls during writeback of a
shared mapping, and none at all on anonymous memory.
The caller-CPU omission in the same function is fixed in the next patch.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
arch/alpha/kernel/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index 1ad448105201..7856d23b3384 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -684,7 +684,8 @@ flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
preempt_disable();
- if (mm == current->active_mm) {
+ /* 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();
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/7] alpha: invalidate the local context in flush_tlb_page()
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
` (2 preceding siblings ...)
2026-08-10 19:34 ` [PATCH v2 3/7] alpha: fix the local TLB invalidate in flush_tlb_page() Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 5/7] alpha: fix the local TLB invalidate in the UP flush_tlb_page() Magnus Lindholm
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
When the target mm is not current, flush_tlb_page() does nothing locally,
and smp_call_function() reaches only the other CPUs. This CPU may still
hold translations for the mm and can later reuse the old ASN together with
them.
Add the missing else branch. flush_tlb_other() clears mm->context[cpu] so
a fresh ASN is taken at the next switch. With the previous patch this also
covers a lazily borrowed mm, which no longer takes the targeted path.
The uniprocessor implementations of flush_tlb_mm() and
flush_icache_user_page() in asm/tlbflush.h and asm/cacheflush.h already
contain exactly this branch.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
arch/alpha/kernel/smp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index 7856d23b3384..a5a42ae4a7d8 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -698,6 +698,9 @@ flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
preempt_enable();
return;
}
+ } else {
+ /* smp_call_function() does not call back into this CPU. */
+ flush_tlb_other(mm);
}
data.vma = vma;
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/7] alpha: fix the local TLB invalidate in the UP flush_tlb_page()
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
` (3 preceding siblings ...)
2026-08-10 19:34 ` [PATCH v2 4/7] alpha: invalidate the local context " Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 6/7] alpha: invalidate the local context in flush_tlb_mm() Magnus Lindholm
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
The uniprocessor flush_tlb_page() in asm/tlbflush.h has the same wrong
test as the SMP one: a targeted tbi() reaches an mm's translations only
when a thread of that mm is current, but the gate is
current->active_mm.
Test current->mm instead. arch/alpha/kernel/smp.c is not built with
CONFIG_SMP=n, so this is how the same defect reaches a uniprocessor,
where the flusher kworker necessarily shares the only CPU with the
writer.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
arch/alpha/include/asm/tlbflush.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/include/asm/tlbflush.h b/arch/alpha/include/asm/tlbflush.h
index 0c8529997f54..9ae1902faf8e 100644
--- a/arch/alpha/include/asm/tlbflush.h
+++ b/arch/alpha/include/asm/tlbflush.h
@@ -87,7 +87,8 @@ flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
{
struct mm_struct *mm = vma->vm_mm;
- if (mm == current->active_mm)
+ /* A targeted tbi() needs a thread of MM to be current. */
+ if (mm == current->mm)
flush_tlb_current_page(mm, vma, addr);
else
flush_tlb_other(mm);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 6/7] alpha: invalidate the local context in flush_tlb_mm()
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
` (4 preceding siblings ...)
2026-08-10 19:34 ` [PATCH v2 5/7] alpha: fix the local TLB invalidate in the UP flush_tlb_page() Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-08-10 19:34 ` [PATCH v2 7/7] alpha: invalidate the local context in flush_icache_user_page() Magnus Lindholm
2026-09-22 17:56 ` [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Matt Turner
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
flush_tlb_mm() has the same caller-CPU omission that was fixed in
flush_tlb_page(): when the target mm is not the calling CPU's active_mm
nothing happens locally, and smp_call_function() handles only the other
CPUs, so this CPU may later reuse the old ASN together with the
translations it still holds.
The active_mm test itself is left alone here. That path calls
flush_tlb_current(), which loads a fresh context through
__load_new_mm_context() rather than issuing a targeted tbi() against
whatever ASN happens to be loaded, so it does not depend on which context
is current.
The uniprocessor implementation in asm/tlbflush.h already has the missing
branch. Counted over a fork-heavy run, flush_tlb_mm() was entered with the
mm not this CPU's active_mm 2934 times, and 632 times while otherwise idle.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
arch/alpha/kernel/smp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index a5a42ae4a7d8..988e397b0b8a 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -649,6 +649,9 @@ flush_tlb_mm(struct mm_struct *mm)
preempt_enable();
return;
}
+ } else {
+ /* smp_call_function() does not call back into this CPU. */
+ flush_tlb_other(mm);
}
smp_call_function(ipi_flush_tlb_mm, mm, 1);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 7/7] alpha: invalidate the local context in flush_icache_user_page()
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
` (5 preceding siblings ...)
2026-08-10 19:34 ` [PATCH v2 6/7] alpha: invalidate the local context in flush_tlb_mm() Magnus Lindholm
@ 2026-08-10 19:34 ` Magnus Lindholm
2026-09-22 17:56 ` [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Matt Turner
7 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-08-10 19:34 UTC (permalink / raw)
To: richard.henderson, mattst88, linux-kernel, linux-alpha
Cc: glaubitz, mcree, ink, macro, Magnus Lindholm, stable
flush_icache_user_page() has the same caller-CPU omission that the
previous patch fixed in flush_tlb_mm(): when the target mm is not the
calling CPU's active_mm nothing happens locally, and smp_call_function()
handles only the other CPUs, so this CPU may later reuse the old ASN
together with the translations it still holds.
This matters here in particular because the function exists for
operating on another process's mappings: the comment above it describes
setting breakpoints through ptrace, and access_remote_vm() reaches it
through copy_to_user_page(). The calling CPU is therefore often running
something other than the target mm.
As in flush_tlb_mm(), the uniprocessor implementation in
asm/cacheflush.h already has the missing case.
No imb() is needed, here or in ipi_flush_icache_page(). Alpha's
user-space I-cache flush works by allocating a new ASN rather than by
invalidating the I-cache: the entries stay, but they are tagged with the
old ASN and can no longer match. An imb() is only required when the ASN
space wraps and numbers are reused, and __get_new_mm_context() already
does one in that case.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
arch/alpha/kernel/smp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/alpha/kernel/smp.c b/arch/alpha/kernel/smp.c
index 988e397b0b8a..e21bc3920bec 100644
--- a/arch/alpha/kernel/smp.c
+++ b/arch/alpha/kernel/smp.c
@@ -758,6 +758,9 @@ flush_icache_user_page(struct vm_area_struct *vma, struct page *page,
preempt_enable();
return;
}
+ } else {
+ /* smp_call_function() does not call back into this CPU. */
+ flush_tlb_other(mm);
}
smp_call_function(ipi_flush_icache_page, mm, 1);
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback
2026-08-10 19:34 [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Magnus Lindholm
` (6 preceding siblings ...)
2026-08-10 19:34 ` [PATCH v2 7/7] alpha: invalidate the local context in flush_icache_user_page() Magnus Lindholm
@ 2026-09-22 17:56 ` Matt Turner
2026-09-22 22:19 ` Magnus Lindholm
7 siblings, 1 reply; 10+ messages in thread
From: Matt Turner @ 2026-09-22 17:56 UTC (permalink / raw)
To: Magnus Lindholm; +Cc: linux-alpha
On Mon, Aug 10, 2026 at 09:34:51PM +0200, Magnus Lindholm wrote:
> mkclean4.c covers patches 3 and 5 (needs root), and fails on both SMP and
> UP. A single thread writes a small MAP_SHARED file while background
> writeback cleans it, and the file is then compared against the mapping;
> every round loses data.
I wrote something along those lines and could not get it to fail on EV7,
on v7.3-rc1 without the series, with and without CONFIG_COMPACTION, on
both an SMP-enabled and a CONFIG_SMP=n kernel. Only one CPU was online
throughout, so none of this exercises the multi-CPU paths. Writeback was
running underneath: 850 to 1700 pages written back per run.
The path is reached, though. With counters in flush_tlb_page() on the
unpatched kernel:
ftp: cur=77824 lazy=324 other=466
ftp lazy: loaded asn=0 mm asn=79 pid=12 comm=kworker/u4:0
ftp lazy: loaded asn=0 mm asn=138 pid=12 comm=kworker/u4:0
so the flusher does issue the targeted tbi() with ASN 0 loaded against an
mm whose ASN is 79, a few hundred times per run, exactly as you describe.
The writer keeps faulting afterwards and the page keeps being redirtied,
so nothing is lost.
The reading that fits is that the EV7 PALcode invalidates by VA without
matching the ASN, which would make this unobservable here and observable
on your EV68. If you have a way to distinguish that from the console I am
happy to run it. Either way the test is wrong by the architecture, so the
patches stand on their own.
One thing that confused me for a while, and may be worth a line in the
changelog: since dd5712f3379c ("alpha: fix user-space corruption during
memory compaction") alpha overrides ptep_clear_flush() to call
migrate_flush_tlb_page(), and that override is inside #ifdef
CONFIG_COMPACTION in asm/pgtable.h. With COMPACTION=y, folio_mkclean()
never reaches flush_tlb_page() at all, so mkclean4 should pass there
whether or not this series is applied. I take it your failing runs are the
COMPACTION=n ones. That also raises the question of why the correct
flushing in ptep_clear_flush() is conditional on COMPACTION in the first
place, but that is not this series.
On patch 1: alpha selects ARCH_NO_PREEMPT, so unless something drags in
PREEMPT_COUNT (DEBUG_ATOMIC_SLEEP, say) preemptible() is 0 and the hook
always runs, including after kthread_use_mm(). The changelog says the hook
does nothing there and that the later patches rely on asn_lock staying
set, which is then only true for PREEMPT_COUNT=y builds. Since patches 2
and 3 lean on that window, it would help to say so.
On the changes since v1: dropping the check_mmu_context() call from
switch_to() means the bookkeeping now runs after finish_lock_switch() has
re-enabled interrupts, rather than with them still off. I convinced myself
that window is harmless, since the CPU is in kernel code throughout and
the case that matters sets need_new_asn, but "no functional change from
v1" undersells it.
For what it is worth on EV7 with one CPU, with the series applied: no
regressions in a gdb breakpoint exerciser, a fork/COW check, or the
writeback test above, on either kernel.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback
2026-09-22 17:56 ` [PATCH v2 0/7] alpha: fix stale TLB translations breaking copy-on-write and writeback Matt Turner
@ 2026-09-22 22:19 ` Magnus Lindholm
0 siblings, 0 replies; 10+ messages in thread
From: Magnus Lindholm @ 2026-09-22 22:19 UTC (permalink / raw)
To: Matt Turner; +Cc: linux-alpha
Hi Matt,
On Tue, Sep 22, 2026 at 7:56 PM Matt Turner <mattst88@gmail.com> wrote:
>
> On Mon, Aug 10, 2026 at 09:34:51PM +0200, Magnus Lindholm wrote:
> > mkclean4.c covers patches 3 and 5 (needs root), and fails on both SMP and
> > UP. A single thread writes a small MAP_SHARED file while background
> > writeback cleans it, and the file is then compared against the mapping;
> > every round loses data.
>
> I wrote something along those lines and could not get it to fail on EV7,
> on v7.3-rc1 without the series, with and without CONFIG_COMPACTION, on
> both an SMP-enabled and a CONFIG_SMP=n kernel. Only one CPU was online
> throughout, so none of this exercises the multi-CPU paths. Writeback was
> running underneath: 850 to 1700 pages written back per run.
>
> The path is reached, though. With counters in flush_tlb_page() on the
> unpatched kernel:
>
> ftp: cur=77824 lazy=324 other=466
> ftp lazy: loaded asn=0 mm asn=79 pid=12 comm=kworker/u4:0
> ftp lazy: loaded asn=0 mm asn=138 pid=12 comm=kworker/u4:0
>
> so the flusher does issue the targeted tbi() with ASN 0 loaded against an
> mm whose ASN is 79, a few hundred times per run, exactly as you describe.
> The writer keeps faulting afterwards and the page keeps being redirtied,
> so nothing is lost.
>
> The reading that fits is that the EV7 PALcode invalidates by VA without
> matching the ASN, which would make this unobservable here and observable
> on your EV68. If you have a way to distinguish that from the console I am
> happy to run it. Either way the test is wrong by the architecture, so the
> patches stand on their own.
>
> One thing that confused me for a while, and may be worth a line in the
> changelog: since dd5712f3379c ("alpha: fix user-space corruption during
> memory compaction") alpha overrides ptep_clear_flush() to call
> migrate_flush_tlb_page(), and that override is inside #ifdef
> CONFIG_COMPACTION in asm/pgtable.h. With COMPACTION=y, folio_mkclean()
> never reaches flush_tlb_page() at all, so mkclean4 should pass there
> whether or not this series is applied. I take it your failing runs are the
> COMPACTION=n ones.
You have found something I owed the list a correction about, and this is
the explanation for it. I could not reproduce my own mkclean4 numbers when
I re-ran them - 0 failures in 103 rounds on unpatched kernels and I had
not worked out what differed. It was the config. My records say the box
had CONFIG_COMPACTION not set and CONFIG_MIGRATION absent on the day the
reproducer was written and the numbers taken, and every kernel I built
afterwards has COMPACTION=y. So yes: the failing runs are the COMPACTION=n
ones, and the re-runs that passed were COMPACTION=y, where as you say the
test cannot reach flush_tlb_page() at all.
> That also raises the question of why the correct
> flushing in ptep_clear_flush() is conditional on COMPACTION in the first
> place, but that is not this series.
Agreed, CONFIG_MIGRATION is the correct gate here, I should probably
send out a separate fix for that (fixing my own code there).
>
> On patch 1: alpha selects ARCH_NO_PREEMPT, so unless something drags in
> PREEMPT_COUNT (DEBUG_ATOMIC_SLEEP, say) preemptible() is 0 and the hook
> always runs, including after kthread_use_mm(). The changelog says the hook
> does nothing there and that the later patches rely on asn_lock staying
> set, which is then only true for PREEMPT_COUNT=y builds. Since patches 2
> and 3 lean on that window, it would help to say so.
>
Right, and the consequence is sharper than the changelog being incomplete.
In an ordinary alpha build the hook runs after kthread_use_mm() and clears
asn_lock there, so patch 2's mm == current->mm && !asn_locked() test is true
for a borrowed mm and the targeted tbi() goes out against the context that
is actually loaded, which is not the borrowed one. The claim that the
shootdown handlers can rely on asn_locked() across that window only holds
for PREEMPT_COUNT=y builds, and I will drop it.
It is not a regression, the pre-patch active_mm test behaves the same way
for a borrowed mm, and patch 2 still fixes the lazy-TLB case it is aimed at.
What actually closes the kthread window is loading the context on the direct
switch, which is patch 3 of the follow-up series. v3's changelog will say
that instead.
> On the changes since v1: dropping the check_mmu_context() call from
> switch_to() means the bookkeeping now runs after finish_lock_switch() has
> re-enabled interrupts, rather than with them still off. I convinced myself
> that window is harmless, since the CPU is in kernel code throughout and
> the case that matters sets need_new_asn, but "no functional change from
> v1" undersells it.
>
Fair, and I will say what moved. My reading of why the window is safe
matches yours: PAL_swpctx has already installed the new context, so an IPI
arriving there sees !asn_locked() only after the store and takes the direct
flush against the right context, and need_new_asn is only ever set while
asn_lock is 1.
> For what it is worth on EV7 with one CPU, with the series applied: no
> regressions in a gdb breakpoint exerciser, a fork/COW check, or the
> writeback test above, on either kernel.
Thanks! that covers patch 7's path, which I had only exercised for
regressions myself. I'll put together a v3 asap.
Magnus
^ permalink raw reply [flat|nested] 10+ messages in thread