Re: [PATCH 1/6] alpha: run check_mmu_context() from finish_arch_post_lock_switch()
Matt Turner <[email protected]>
| Newsgroups | gmane.linux.ports.alpha,gmane.linux.kernel |
|---|---|
| Message-ID | <CAEdQ38EHDfxJVsSOorF30TCsZAoCG1D==Bv6Z0Mz0uyNvRPftQ@mail.gmail.com> |
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?