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

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

[Zhiteng Qiu]

Since commit 7e12beb8ca2a ("migrate_pages: batch flushing TLB"), every
MIGRATE_ASYNC migration unmaps with TTU_BATCH_FLUSH.  That includes NUMA
balancing and memory-tiering promotion, which migrate one folio per call
through migrate_misplaced_folio().  For an order-0 folio with a single
mapping the batch holds one PTE and there is nothing to merge, but the
flush still changes: on x86, arch_tlbbatch_flush() sends a full TLB
flush to every CPU in mm_cpumask() instead of invalidating that one page.

On a 2-socket, 384-CPU x86 server without INVLPGB, a 4 KiB-page,
memory-heavy multithreaded workload takes about 200 TLB shootdown
interrupts per page migrated by NUMA balancing (v5.15 on the same machine
takes about 11), and balancing costs it 13-16% of its throughput.

Skip the batched flush only when the batch cannot merge anything: the
folio is the only one in this migrate_pages_batch() call, it is not a
large folio, and it is mapped only once.  The rmap walk then meets at
most one PTE, and ptep_clear_flush() invalidates just that page.
Everything else keeps the batched flush, including PTE-mapped large
folios, folios mapped more than once, and the batch of sub-folios
produced when a partially mapped large folio is split during migration.

PMD-mapped THPs already invalidate their mappings through
set_pmd_migration_entry(), independently of TTU_BATCH_FLUSH, so their
flush behaviour is unchanged.  On arm64 the batched path only defers the
final DSB, so a one-PTE batch should cost about the same either way.

migrate_folio_unmap() cannot see how many folios its caller is
migrating, so migrate_pages_batch() passes that down.

Measured on v7.3: 120 GiB anonymous working set, 384 threads, 180 s per
run, three A/B pairs (numa_balancing on vs. off, same boot) per kernel.

                                    v7.3            patched
  throughput loss from balancing   13.0-16.5%      0.5-6.3%
  kernel cycles per hinting PTE    1.18-1.22M      0.28-0.31M
  TLB interrupts / migrated page   202-210         12.7-13.6

Signed-off-by: Zhiteng Qiu <qiuzhiteng@hygon.cn>
---
Notes:

An earlier version decided on reason == MR_NUMA_MISPLACED.  That also
dropped batching for the split sub-folio batch mentioned above.  In a
test that splits partially mapped 2 MiB THPs during NUMA migration, the
earlier version raised TLB interrupts per migrated page from 0.13 to
16.1; this version keeps them at 0.11.  For 64 KiB mTHP and for a
MAP_SHARED mapping used by two processes, this version behaves like the
base kernel (TLB interrupts per migrated page: mTHP 2.2-2.7 patched vs.
2.6-3.3 base; shared 31.3-31.5 vs. 31.2-31.7).

The earlier version behaves the same as this one for order-0, private,
single-mapped memory.  These results were measured with it:
 - 192 CPUs (SMT off, 192 threads, 4 pairs): balancing loss
   3.1-14.7% patched vs. 7.4-54.3% base.
 - Memory first-touched on the remote socket, threads on the local
   socket (6 runs per kernel): no regression in time to convergence or
   in throughput after convergence.
 - 2 MiB THP (2 pairs): no difference, as expected for PMD-mapped
   folios.
 - A 30-minute run with compaction forced every 30 s: no warnings.

With this version, tools/testing/selftests/mm/migration passes 6/6
(including the THP and hugetlb cases), the LTP move_pages and
migrate_pages cases pass 14/14, and the scenario tests above saw no data
mismatches.

There is no Fixes: tag.  This changes a performance trade-off in code
that has been there since v6.4, and nobody has reported it as a
regression.  Please add one if you see it that way.
---
 mm/migrate.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 953af633ec..0f3404c3d7 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1222,7 +1222,7 @@ static void migrate_folio_done(struct folio *src,
 static int migrate_folio_unmap(new_folio_t get_new_folio,
 		free_folio_t put_new_folio, unsigned long private,
 		struct folio *src, struct folio **dstp, enum migrate_mode mode,
-		struct list_head *ret)
+		bool single_folio, struct list_head *ret)
 {
 	struct folio *dst;
 	int rc = -EAGAIN;
@@ -1344,7 +1344,18 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
 		/* Establish migration ptes */
 		VM_BUG_ON_FOLIO(folio_test_anon(src) &&
 			       !folio_test_ksm(src) && !anon_vma, src);
-		try_to_migrate(src, mode == MIGRATE_ASYNC ? TTU_BATCH_FLUSH : 0);
+		/*
+		 * Batching only pays off when the rmap walk can meet more than
+		 * one PTE.  A lone order-0 folio with a single mapping has
+		 * exactly one, and on x86 the batched flush turns that one-page
+		 * invalidation into a full flush of mm_cpumask().
+		 */
+		bool batch_flush = mode == MIGRATE_ASYNC;
+
+		if (single_folio && !folio_test_large(src) &&
+		    !folio_maybe_mapped_shared(src))
+			batch_flush = false;
+		try_to_migrate(src, batch_flush ? TTU_BATCH_FLUSH : 0);
 		old_folio_state |= FOLIO_WAS_MAPPED;
 	}
 
@@ -1835,6 +1846,7 @@ static int migrate_pages_batch(struct list_head *from,
 	LIST_HEAD(unmap_folios);
 	LIST_HEAD(dst_folios);
 	bool nosplit = (reason == MR_NUMA_MISPLACED);
+	bool single_folio = list_is_singular(from);
 
 	VM_WARN_ON_ONCE(mode != MIGRATE_ASYNC &&
 			!list_empty(from) && !list_is_singular(from));
@@ -1925,7 +1937,8 @@ static int migrate_pages_batch(struct list_head *from,
 			}
 
 			rc = migrate_folio_unmap(get_new_folio, put_new_folio,
-					private, folio, &dst, mode, ret_folios);
+					private, folio, &dst, mode, single_folio,
+					ret_folios);
 			/*
 			 * The rules are:
 			 *	0: folio will be put on unmap_folios list,

base-commit: dba8aba0a9a00ff781c03d82ae534fc6f8c2e263
-- 
2.51.0

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

[Huang, Ying]

Zhiteng Qiu <qiuzhiteng@hygon.cn> writes:

> Since commit 7e12beb8ca2a ("migrate_pages: batch flushing TLB"), every
> MIGRATE_ASYNC migration unmaps with TTU_BATCH_FLUSH.  That includes NUMA
> balancing and memory-tiering promotion, which migrate one folio per call
> through migrate_misplaced_folio().  For an order-0 folio with a single
> mapping the batch holds one PTE and there is nothing to merge, but the
> flush still changes: on x86, arch_tlbbatch_flush() sends a full TLB
> flush to every CPU in mm_cpumask() instead of invalidating that one page.
>
> On a 2-socket, 384-CPU x86 server without INVLPGB, a 4 KiB-page,
> memory-heavy multithreaded workload takes about 200 TLB shootdown
> interrupts per page migrated by NUMA balancing (v5.15 on the same machine
> takes about 11), and balancing costs it 13-16% of its throughput.
>
> Skip the batched flush only when the batch cannot merge anything: the
> folio is the only one in this migrate_pages_batch() call, it is not a
> large folio, and it is mapped only once.  The rmap walk then meets at
> most one PTE, and ptep_clear_flush() invalidates just that page.
> Everything else keeps the batched flush, including PTE-mapped large
> folios, folios mapped more than once, and the batch of sub-folios
> produced when a partially mapped large folio is split during migration.
>
> PMD-mapped THPs already invalidate their mappings through
> set_pmd_migration_entry(), independently of TTU_BATCH_FLUSH, so their
> flush behaviour is unchanged.  On arm64 the batched path only defers the
> final DSB, so a one-PTE batch should cost about the same either way.
>
> migrate_folio_unmap() cannot see how many folios its caller is
> migrating, so migrate_pages_batch() passes that down.
>
> Measured on v7.3: 120 GiB anonymous working set, 384 threads, 180 s per
> run, three A/B pairs (numa_balancing on vs. off, same boot) per kernel.
>
>                                     v7.3            patched
>   throughput loss from balancing   13.0-16.5%      0.5-6.3%
>   kernel cycles per hinting PTE    1.18-1.22M      0.28-0.31M
>   TLB interrupts / migrated page   202-210         12.7-13.6

IIRC, after commit b99a342d4f11 ("NUMA balancing: reduce TLB flush via
delaying mapping on hint page fault"), we can avoid most TLB flushing
caused by do_numa_page().  Could you check why it does not work?

> Signed-off-by: Zhiteng Qiu <qiuzhiteng@hygon.cn>

[snip]

---
Best Regards,
Huang, Ying

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

[Zhiteng Qiu]

Hi Ying,

"Huang, Ying" <ying.huang@linux.alibaba.com> writes:

> IIRC, after commit b99a342d4f11 ("NUMA balancing: reduce TLB flush via
> delaying mapping on hint page fault"), we can avoid most TLB flushing
> caused by do_numa_page().  Could you check why it does not work?

Thanks for looking.  b99a342d4f11 is in effect here: on success
do_numa_page() returns without rebuilding the PTE, so the PTE is still
PROT_NONE when try_to_migrate() runs, and both the batched and the
direct unmap path should skip the flush because pte_accessible() is
false.

The numbers disagree for the batched path.  On the same workload and
setup there are about 200 TLB shootdown interrupts per migrated page
with TTU_BATCH_FLUSH, and about 13 with this patch applied.  So
something makes the batched path flush where the direct path does not,
and I don't have the answer yet.  I'm instrumenting the unmap path and
will follow up with data.

Thanks,
Zhiteng

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

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

[Huang, Ying]

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