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,
>
>> Have you adjusted the NUMA scanning parameters?
>
> No.  All NUMA balancing tunables are at their defaults
> (scan_delay_ms=1000, scan_period_min_ms=1000,
> scan_period_max_ms=60000, scan_size_mb=256).
>
> The scan is indeed slow: 58 passes of 256 MB in 55 s, 89 ms each,
> about 9% of wall time.  But the hint faults are not spread evenly
> between passes: those 9% carry 33% of all migrations.
>
> The faults come from the scan, just not from the pass that happens
> to be running at that instant.  Of the migrations that happen
> during a pass, 60% are for pages in the 256 MB chunk being scanned
> right now; nearly all the rest are for pages scanned one or two
> chunks earlier (lower addresses, within 512 MB).  None are from
> addresses the scanner has not reached yet.  So the scan makes the
> pages PROT_NONE and the workload touches them again within a second
> or two, while this or the next pass is running.  The collision is
> not a coincidence between two independent events.
>
> Here is the sequence, one pass on the unpatched kernel:
>
>   CPU A (task_numa_work)          CPU B..N (workload threads)
>   ----------------------          ---------------------------
>   tlb_gather_mmu()
>     tlb_flush_pending++
>   change_pte_range(table 1)
>     PTEs -> PROT_NONE
>                                   access page in table 1
>                                   do_numa_page()
>                                    migrate_misplaced_folio()
>                                     try_to_migrate_one()
>                                      PROT_NONE, but pending set:
>                                      pte_accessible() == true
>                                      tlb_flush_batched++
>                                     try_to_unmap_flush()
>                                      full flush, IPI to ~380 CPUs
>   change_pte_range(table 2)
>     flush_tlb_batched_pending()
>       pending != flushed:
>       flush_tlb_mm(), IPI to ~380 CPUs
>     PTEs -> PROT_NONE
>                                   (same for pages in table 2)
>   ... 128 PTE tables, ~0.7 ms each ...
>   tlb_finish_mmu()                    pass took ~89 ms, not ~7 ms
>     tlb_flush_pending--

This looks much more clear!  Thanks!

> Each table flush makes the pass longer, and a longer pass lets more
> faults happen while tlb_flush_pending is set.  With the patch the
> migration takes the direct path: it may still flush while the pass
> is running, but it does not bump tlb_flush_batched, so the scan does
> not pay per table and the pass stays at ~7 ms.
>
>> That appear too complex.
>
> Agreed.  I'll keep the change in migrate_folio_unmap() and put the
> sequence above into the v2 changelog.

Sounds good to me.

---
Best Regards,
Huang, Ying