Re: [Devel] [PATCH hci-8.1 v2 0/2] mm/memcontrol: numa_migrate destinations of the source folio order

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

Looks good. (except that sashiko reveiw should be handled)

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

On 10/28/26 15:17, Konstantin Khorenko wrote:
> memory.numa_migrate, our memcg file that vcmmd uses to move a cgroup's
> memory between NUMA nodes, allocates a 2 MB destination for every large
> folio it migrates. Page cache folios of a smaller order get a
> destination of the wrong size, and the page cache falls apart around
> it. Patch 1 is the fix, patch 2 a selftest that catches it without
> crashing the node.
> 
> What the customer hit
> =====================
> 
> A two-node VMware host runs VMs with vcmmd's NUMA policy enabled, so
> every few minutes vcmmd writes memory.numa_migrate for a VM's cgroup.
> qemu writes its backup scratch file on NFS, and NFS sizes each page
> cache folio to the write, so that file is full of order-1 to order-5
> folios. Closing the scratch file dies in nfs4_evict_inode():
> 
>   kernel BUG at fs/inode.c:737!
>   RIP: 0010:clear_inode+0x75/0x80
> 
> mapping->nrpages of the inode is negative, and the vmcore holds
> order-9 folios of that file with an index not aligned to 512, the
> refcount of a small folio, and whole xarray ranges of live folios that
> are no longer in the tree. Reads of the migrated ranges return garbage,
> so the backups taken on that host while the policy was active should
> be verified. The same crash was filed as VSTOR-151676.
> 
> Where the 2 MB comes from
> =========================
> 
> memcg_numa_migrate_new_folio() is the migrate_pages() allocation
> callback. Its large folio branch dates from vz8 and copies the 4.18
> new_page() in mm/mempolicy.c:
> 
>   if (thp_migration_supported() && PageTransHuge(page))
>           alloc ... HPAGE_PMD_ORDER
> 
> A compound page on the LRU could only be a PMD-sized THP then, so the
> test implied the order. Upstream replaced that with folio_order() of
> the source when its allocators moved to folios, ffe06786b540
> ("mm/migrate: Use a folio in alloc_migration_target()"). Our copy kept
> the shape, only PageTransHuge() became folio_test_large(), which is
> folio_test_head() and is true for any order from 1 up. NFS and XFS
> large folios, shmem and anonymous mTHP now all reach it.
> 
> Why an oversized destination breaks the page cache
> ==================================================
> 
> The two sides of the page cache size the folio differently.
> 
> On insertion __folio_migrate_mapping() takes everything from the
> source: nr = folio_nr_pages(old), the xarray state at the old index
> with no xas_set_order(), newfolio->index = old->index,
> folio_ref_add(newfolio, nr), and xas_store() replaces exactly the slots
> the source occupied. folio_copy() copies the source pages only. So a
> 2 MB folio enters the cache as, say, a 4-page one: 4 xarray slots, 4
> extra references, nrpages unchanged, 4 pages of valid data.
> 
> Everything afterwards asks the folio itself, and it answers order 9:
> 
> - page_cache_delete(): xas_set_order(index, 9) and nrpages -= 512.
>   512 slots wiped instead of 4, with the index rounded down to 512 so
>   that other live folios lose their slots, and 512 subtracted instead
>   of 4. That is the negative nrpages and the orphaned folios.
> - __remove_mapping(): expects 1 + 512 references and finds 1 + 4.
>   Reclaim can never free the folio.
> - filemap_free_folio(): drops 512 references from a handful at
>   eviction. The refcount goes negative and the folio leaks.
> - reads: folio_file_page() picks the page by its offset inside the
>   folio, but only the first pages were copied.
> 
> While the folio sits in the cache and is read through its own slots
> nothing is visible. It breaks the first time something applies
> folio_nr_pages() or folio_order() to it: invalidation, truncate,
> reclaim, inode eviction. On NFS cache invalidation is routine (an
> attribute change), so the bug shows up there instead of staying a
> silent 2 MB leak per folio.
> 
> In short: inserted by the source size, removed by the destination
> size. That only works when the two match, which for a real THP they
> always did. A debug kernel says the same on the first migration, in
> mem_cgroup_migrate():
> 
>   VM_BUG_ON_FOLIO(folio_nr_pages(old) != folio_nr_pages(new))
> 
> The patches
> ===========
> 
> Patch 1 allocates the destination with folio_order() of the source,
> GFP_TRANSHUGE_LIGHT for large folios as before. The
> thp_migration_supported() test goes: migrate_pages() splits PMD-mappable
> folios itself when THP migration is unsupported, and every other large
> folio still needs a matching destination.
> 
> Patch 2 adds tools/testing/selftests/ve/ve_numa_migrate_test.c. It
> binds a cgroup to one NUMA node, writes a file on a loop-mounted XFS
> image in 64K chunks so that the page cache holds large folios, migrates
> the cgroup to the other node through memory.numa_migrate and compares
> what reads back. memory.current cannot serve as the detector:
> mem_cgroup_migrate() only transfers the charge, whatever the size
> mismatch. On an unfixed kernel the comparison fails and the test leaves
> the image mounted on purpose, since evicting the inode would hit the
> BUG_ON.
> 
> The folio sizes are not taken on faith: before the migration the test
> reads the compound flags of the file's pages through /proc/self/pagemap
> and /proc/kpageflags, and a file without a large folio fails the test.
> That is deliberate: a skip would hide that the test no longer reaches
> the bug, a failure says the setup needs another way to get large
> folios. After the comparison the test polls memory.numa_stat for a few
> seconds to see that something was migrated, as memcg flushes its
> per-cpu counters lazily. It needs root, two NUMA nodes with memory
> (numa=fake=2 is enough), cgroup2 with memory and cpuset, and mkfs.xfs;
> it skips otherwise.
> 
> Testing
> =======
> 
> On a VM booted with numa=fake=2:
> 
> - the reproducer (64K and 8K writes on XFS from a node-bound cgroup,
>   memory.numa_migrate to the other node, unlink) kills the unfixed
>   debug kernel on the VM_BUG_ON above and the release kernel in
>   clear_inode(); with patch 1 migration succeeds and unlink and
>   eviction complete;
> - the selftest passes on the fixed kernel and fails on the comparison
>   on 22.11 without taking the node down.
> 
> Other branches
> ==============
> 
> The same patch applies as is to the 20.x branch (the customer's
> kernel) and is on pr/khorenko-VSTOR-151607-numa-migrate-order-20.x;
> the function is a plain callback, so it is a ReadyKernel candidate.
> vz9 carries the same PageTransHuge() code, but 5.14 has no non-PMD
> large folios on the LRU, so it is not reachable there.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-151607
> 
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
> Changes in v2:
> - Patch 1: the commit message says why the thp_migration_supported()
>   check goes (Pavel).
> - Patch 2: the folio sizes are checked through /proc/self/pagemap and
>   /proc/kpageflags before the migration; a file without a large folio
>   fails the test on purpose (Pavel).
> - Patch 2: the read-back comparison runs first and the memory.numa_stat
>   check after it, polled for a few seconds, since memory_numa_stat_show()
>   flushes the per-cpu counters only past a per-CPU threshold (Vasileios).
> - Reviewed-by from Pavel and Vasileios on both patches.
> - Link to v1: http://lore.virtuozzo.com/20261028-b4-prep-khorenko-vstor-151607-numa-migrate-order-v1-0-268336526677@virtuozzo.com
> 
> ---
> Konstantin Khorenko (2):
>       mm/memcontrol: allocate numa_migrate targets of the source folio order
>       selftests/ve: check memory.numa_migrate keeps the folio order
> 
>  mm/memcontrol.c                                   |  11 +-
>  tools/testing/selftests/ve/Makefile               |   1 +
>  tools/testing/selftests/ve/ve_numa_migrate_test.c | 384 ++++++++++++++++++++++
>  3 files changed, 391 insertions(+), 5 deletions(-)
> ---
> base-commit: b499efa17aa9fb8f20512d1831d1cb61b22e60d3
> change-id: 20261028-b4-prep-khorenko-vstor-151607-numa-migrate-order-4e3a664a89d4
> 
> Best regards,
> --  
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel