Re: [PATCH v3 2/2] alpha: do not skip TLB shootdown IPIs for kthread-borrowed mms

[Magnus Lindholm]

Hi Matt,

On Sat, Oct 10, 2026 at 3:35 AM Matt Turner <mattst88@xxxxxxxxx> wrote:
>
> On Fri, Oct 9, 2026 at 11:19 PM Magnus Lindholm <linmag7@xxxxxxxxx> 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@xxxxxxxxx>


Thanks

Magnus