Re: [PATCH] mm/migrate: don't batch the TLB flush when there is nothing to batch

[Zhiteng Qiu]

Hi Ying,

I instrumented the unmap path on both kernels (kprobes, 55 s windows,
same workload).

b99a342d4f11 does what it should.  On the patched kernel, where the
old PTE can be read from ptep_clear_flush(), about 96% of the PTEs are
still PROT_NONE when try_to_migrate_one() clears them.  The flushes
come from pte_accessible() treating a PROT_NONE PTE as accessible while
mm->tlb_flush_pending is raised.  On the unpatched kernel that is the
case for about 42% of the migrations (3.5% with the patch), and each of
those does a full arch_tlbbatch_flush() to about 380 CPUs.

tlb_flush_pending is raised that often because of a loop.  Each batched
unmap bumps mm->tlb_flush_batched, and only flush_tlb_batched_pending()
marks it flushed.  So while migrations keep coming, change_pte_range()
in the NUMA scanner does a flush_tlb_mm() for every PTE table it walks:
about 11,600 full flushes in the window, one per 512 pages scanned,
and 99.9% of them come from that call stack.  The average mmu_gather
in task_numa_work() takes 91 ms on the unpatched kernel and 6.7 ms with
the patch.  A longer scan catches more migrations with
tlb_flush_pending set, and more migrations make the scan pay again.
With the patch, a lone order-0 folio is unmapped with
ptep_clear_flush() and never touches tlb_flush_batched, so the loop
does not start.  The remaining ~13 interrupts per page with the patch
are the same effect at a much smaller scale: the 3.5% of migrations
that still overlap a scan.

So the patch helps by not leaving a pending batch behind, not by making
a single flush cheaper.  I'll rewrite the changelog to say that in v2.

The loop could also be broken on the flush side, by letting
try_to_unmap_flush() mark the batch as flushed.  But tlb_flush_batched
is per mm while the batch is per task, so I don't see a simple and safe
way to do that.  Would you prefer that direction, or is the change in
migrate_folio_unmap() fine?

Thanks,
Zhiteng