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

["Huang, Ying" <ying.huang@linux.alibaba.com>]

Zhiteng Qiu <qiuzhiteng@hygon.cn> writes:

> 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.

I found that it's not easy to understand the text above.  A figure like
the one in the below commit may help:

https://github.com/torvalds/linux/commit/5a6c9cf117fca3c38a98fb2a2d5549795c8b0b73

Have you adjusted the NUMA scanning parameters?  I originally thought
that the scanning is slow, so it wouldn't be common for scanning and
NUMA hint faulting to race.

> 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?

That appear too complex.

---
Best Regards,
Huang, Ying