Re: [PATCH 1/6] alpha: run check_mmu_context() from finish_arch_post_lock_switch()

On Sun, Aug 9, 2026 at 10:37 PM Matt Turner <[email protected]> wrote:
>
> On Sun, Aug 9, 2026 at 4:55 AM Magnus Lindholm <[email protected]> wrote:
> >
> > 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 deferred-ASN bookkeeping for both. Running it when
> > switch_to() has already done the work is harmless: check_mmu_context()
> > clears need_new_asn as it goes.
> >
> > The same hook is also called from kthread_use_mm() and
> > sched_force_init_mm(), outside the scheduler's preemption-disabled switch
> > tail. check_mmu_context() acts on per-CPU state, so it can only complete
> > this bookkeeping while still on the CPU that ran switch_mm(). Testing
> > preemptible() expresses that condition directly rather than naming callers:
> > where those paths leave preemption enabled the CPU may already have changed
> > and nothing is done, and where preemption is disabled across the switch, or
> > not configured at all, no migration is possible and running it is correct.
> >
> > Signed-off-by: Magnus Lindholm <[email protected]>
> > ---
> >  arch/alpha/include/asm/mmu_context.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/arch/alpha/include/asm/mmu_context.h b/arch/alpha/include/asm/mmu_context.h
> > index eee8fe836a59..f7afc64ca8dd 100644
> > --- a/arch/alpha/include/asm/mmu_context.h
> > +++ b/arch/alpha/include/asm/mmu_context.h
> > @@ -181,6 +181,34 @@ do {                                                               \
> >  #define check_mmu_context()  do { } while(0)
> >  #endif
> >
> > +/*
> > + * check_mmu_context() clears asn_lock and acts on need_new_asn, but it runs
> > + * only as the tail of switch_to(), which a newly forked task never reaches:
> > + * it resumes at ret_from_fork instead.  asn_lock is left set and the task
> > + * goes on to run user space with it set and interrupts enabled, so a
> > + * shootdown IPI arriving in that window takes the deferred path and the
> > + * need_new_asn handshake meant to cover it never runs.
> > + *
> > + * finish_task_switch() calls this hook with preemption disabled on the CPU
> > + * that ran switch_mm(), which covers that case.  Running it when switch_to()
> > + * has already done the work is harmless: check_mmu_context() clears
> > + * need_new_asn as it goes.
> > + *
> > + * kthread_use_mm() and sched_force_init_mm() also call this hook, outside
> > + * the scheduler's preemption-disabled switch tail.  check_mmu_context() acts
> > + * on per-CPU state, so it can only complete this bookkeeping while still on
> > + * the CPU that ran switch_mm(); preemptible() tests that directly.  Where
> > + * those paths leave preemption enabled the CPU may already have changed and
> > + * nothing is done; where preemption is disabled across the switch, or not
> > + * configured at all, no migration is possible and running it is correct.
> > + */
> > +#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)
> >  {
> > --
> > 2.53.0
> >
>
> This seems to make the check_mmu_context() in switch_to() redundant.
>
> arch/alpha/include/asm/switch_to.h:12 calls it, then finish_task_switch() calls
> the new hook a few lines later. Between them the rq lock is held and IRQs are
> off (finish_lock_switch() → raw_spin_rq_unlock_irq), so no shootdown IPI can
> land in that window.
>
> I think we should drop it from switch_to.h?

Another thing. Pre-existing issue, I believe:

preemptible() is true in kthread_use_mm(), so the hook does nothing,
and ev5_switch_mm()'s asn_lock = 1 stays set on that CPU until the
next real switch_to.

Benign, but patch 2 depends on this. patch 2's own commit message says
current->mm can be true with the mm's context not loaded
(kthread_use_mm), and the only thing saving that case is the leaked
asn_lock forcing asn_locked() → flush_tlb_other(). Should probably
document that in one of these patches.