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

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

[Konstantin Khorenko]

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

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

[Devel] [PATCH hci-8.1 v2 1/2] mm/memcontrol: allocate numa_migrate targets of the source folio order

[Konstantin Khorenko]

memcg_numa_migrate_new_folio() treats every large folio as a PMD-sized
THP and allocates an HPAGE_PMD_ORDER destination for it. The branch is
the 4.18 pattern of new_page() in mm/mempolicy.c: a compound page on
the LRU could only be a THP then, so PageTransHuge() implied
HPAGE_PMD_ORDER.

Today the page cache holds large folios of any order (NFS sizes a folio
to the write, XFS and shmem have them too) and anonymous memory has
mTHP, while folio_test_large() is still just folio_test_head() and is
true for any order from 1 up. migrate_pages() never checks that the
destination order matches the source.

For an order-1 or order-4 file folio the destination is an order-9
folio that inherits the source index and page cache reference count,
so every later page cache operation that sizes the folio by its order
goes wrong: nrpages underflows, neighbouring xarray slots get wiped,
the folio can never be freed.

On a two-node host a VM migration to another NUMA node along with a
backup to NFS ends up with a negative nrpages, and closing the backup
file dies in nfs4_evict_inode():

  kernel BUG at fs/inode.c:737!
  RIP: 0010:clear_inode+0x75/0x80
  Call Trace:
   nfs4_evict_inode+0x21/0x80 [nfsv4]
   evict+0x115/0x2a0
   nfs4_delegreturn_release+0x55/0xa0 [nfsv4]
   rpc_free_task+0x39/0x60 [sunrpc]
   __rpc_execute+0x3d1/0x3e0 [sunrpc]

A debug build dies earlier, on the first migration of a large folio of
any file system, in mem_cgroup_migrate():

  VM_BUG_ON_FOLIO(folio_nr_pages(old) != folio_nr_pages(new))

Make memcg_numa_migrate_new_folio() handle large folios properly.
The thp_migration_supported() check goes: migrate_pages() splits PMD
THPs itself when unsupported, other large folios migrate whole anyway.

Fixes: 9f8a754ad6c2 ("mm: memcontrol: add memory.numa_migrate file")
https://virtuozzo.atlassian.net/browse/VSTOR-151607

Feature: mm: interface to migrate memory between NUMA nodes upon userspace request
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 mm/memcontrol.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0734c0fa21d05..d8a87913ace37 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3480,6 +3480,7 @@ static struct folio *memcg_numa_migrate_new_folio(struct folio *old_folio,
 {
 	struct memcg_numa_migrate_struct *ms = (void *)private;
 	gfp_t gfp_mask = GFP_HIGHUSER_MOVABLE | __GFP_NORETRY | __GFP_NOWARN;
+	unsigned int order = 0;
 
 	ms->current_node = next_node(ms->current_node, *ms->target_nodes);
 	if (ms->current_node >= MAX_NUMNODES) {
@@ -3487,13 +3488,13 @@ static struct folio *memcg_numa_migrate_new_folio(struct folio *old_folio,
 		VM_BUG_ON(ms->current_node >= MAX_NUMNODES);
 	}
 
-	if (thp_migration_supported() && folio_test_large(old_folio)) {
-		return __folio_alloc(GFP_TRANSHUGE_LIGHT | __GFP_THISNODE,
-				HPAGE_PMD_ORDER, ms->current_node,
-				ms->target_nodes);
+	if (folio_test_large(old_folio)) {
+		gfp_mask = GFP_TRANSHUGE_LIGHT | __GFP_THISNODE;
+		order = folio_order(old_folio);
 	}
 
-	return __folio_alloc(gfp_mask, 0, ms->current_node, ms->target_nodes);
+	return __folio_alloc(gfp_mask, order, ms->current_node,
+			     ms->target_nodes);
 }
 
 /*

-- 
2.43.0

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

[Devel] [PATCH hci-8.1 v2 2/2] selftests/ve: check memory.numa_migrate keeps the folio order

[Konstantin Khorenko]

memcg_numa_migrate_new_folio() used to allocate a PMD-sized destination
for every large folio. A page cache folio of a smaller order then ended
up with a 512-page destination carrying its index and reference count.
Only the source pages were copied into it, so a read of the folios that
followed in the same 2 MB range was served from the uninitialised part
of the destination, and the inode died in clear_inode() on eviction.

The test writes a file on an XFS image, where 64K writes produce large
folios, from a cgroup bound to one NUMA node, migrates the cgroup to a
second node, reads the file back against the written pattern and checks
through memory.numa_stat that something was migrated. The stat read is
retried for a while, as memcg flushes its per-cpu counters lazily. On an
unfixed kernel the comparison fails; the test then leaves the image
mounted, because unlinking or unmounting would evict the inode and
crash the node.

The folio sizes are not taken on faith: before the migration the test
maps the file and reads the compound flags of its pages through
/proc/self/pagemap and /proc/kpageflags. A file without a large folio
fails the test with a message, because it would pass without touching
the bug.

It needs root, two NUMA nodes with memory (numa=fake=2 is enough),
cgroup2 with the memory and cpuset controllers, and mkfs.xfs, and skips
otherwise.

https://virtuozzo.atlassian.net/browse/VSTOR-151607

Feature: ve selftests
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/ve/Makefile               |   1 +
 tools/testing/selftests/ve/ve_numa_migrate_test.c | 384 ++++++++++++++++++++++
 2 files changed, 385 insertions(+)

diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
index 51f9316beb99b..e96320a183844 100644
--- a/tools/testing/selftests/ve/Makefile
+++ b/tools/testing/selftests/ve/Makefile
@@ -6,6 +6,7 @@ TEST_GEN_PROGS += ve_ns_owner_test
 TEST_GEN_PROGS += ve_perms_test
 TEST_GEN_PROGS += ve_mount_accounting_test
 TEST_GEN_PROGS += ve_vtty_stress
+TEST_GEN_PROGS += ve_numa_migrate_test
 
 # CPUID faulting is x86-64 only. ARCH says "x86" for 32-bit too and lib.mk
 # picks the clang target by its emptiness, so decide by the compiler.
diff --git a/tools/testing/selftests/ve/ve_numa_migrate_test.c b/tools/testing/selftests/ve/ve_numa_migrate_test.c
new file mode 100644
index 0000000000000..b4a60dba8063d
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_numa_migrate_test.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * memory.numa_migrate must give a large page cache folio a destination of
+ * the same order. An oversized destination inherits the index and the
+ * reference count of the source, only the source pages are copied into it,
+ * reads of the neighbouring folios land in its uninitialised part, and the
+ * inode dies in clear_inode() on eviction.
+ *
+ * Write a file on an XFS image (XFS allocates large folios for 64K writes)
+ * from a cgroup bound to one NUMA node, migrate the cgroup to another node
+ * and check that the data reads back intact.
+ *
+ * Needs root, two NUMA nodes with memory (numa=fake=2 will do), cgroup2
+ * with the memory and cpuset controllers, and mkfs.xfs. The folio sizes
+ * are read through /proc/self/pagemap and /proc/kpageflags before the
+ * migration: a file without large folios fails the test, as it would
+ * pass without touching the bug.
+ */
+#define _GNU_SOURCE
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sched.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <sys/mount.h>
+#include <sys/mman.h>
+#include <linux/sched.h>
+#include <linux/kernel-page-flags.h>
+
+#include "../kselftest_harness.h"
+#include "ve_selftest.h"
+
+#define CG_NAME		"ve_numa_migrate_test"
+#define DIR_TEMPLATE	"/var/tmp/ve_numa_migrate.XXXXXX"
+/* mkfs.xfs refuses anything smaller; the image is sparse. */
+#define IMG_SIZE	(320UL << 20)
+#define FILE_SIZE	(4UL << 20)
+#define CHUNK_SIZE	(64UL << 10)
+#define PAGE_SIZE	4096UL
+#define PM_PFN_MASK	((1ULL << 55) - 1)
+#define PM_PRESENT	(1ULL << 63)
+/* The periodic memcg stats flush runs every two seconds. */
+#define STAT_RETRIES	50
+#define STAT_RETRY_US	100000
+
+FIXTURE(numa_migrate)
+{
+	int cgv2_fd;
+	int cg_fd;
+	int node_from;
+	int node_to;
+	char dir[64];
+	char img[80];
+	char mnt[80];
+	int mounted;
+	int leave_mounted;
+	int data_fd;
+};
+
+/* First two node ids of a sysfs node list such as "0-1" or "0,2-3". */
+static int two_nodes_with_memory(int *from, int *to)
+{
+	char buf[256], *tok, *save;
+	int found = 0;
+
+	if (read_file_at(AT_FDCWD, "/sys/devices/system/node/has_memory",
+			 buf, sizeof(buf)) <= 0)
+		return -1;
+
+	for (tok = strtok_r(buf, ",\n", &save); tok && found < 2;
+	     tok = strtok_r(NULL, ",\n", &save)) {
+		int a, b, n;
+
+		if (sscanf(tok, "%d-%d", &a, &b) != 2) {
+			if (sscanf(tok, "%d", &a) != 1)
+				return -1;
+			b = a;
+		}
+		for (n = a; n <= b && found < 2; n++) {
+			if (found == 0)
+				*from = n;
+			else
+				*to = n;
+			found++;
+		}
+	}
+	return found == 2 ? 0 : -1;
+}
+
+static int run_cmd(const char *fmt, ...)
+{
+	char cmd[256];
+	va_list ap;
+	int status;
+
+	va_start(ap, fmt);
+	vsnprintf(cmd, sizeof(cmd), fmt, ap);
+	va_end(ap);
+
+	status = system(cmd);
+	if (status < 0 || !WIFEXITED(status))
+		return -1;
+	return WEXITSTATUS(status);
+}
+
+/* Bytes of file pages the cgroup has on @node, from memory.numa_stat. */
+static int file_bytes_on_node(int cg_fd, int node, unsigned long long *out)
+{
+	static char buf[32768];
+	char key[16], *p;
+
+	if (read_file_at(cg_fd, "memory.numa_stat", buf, sizeof(buf)) <= 0)
+		return -1;
+	p = strstr(buf, "\nfile ");
+	if (!p)
+		return -1;
+	snprintf(key, sizeof(key), " N%d=", node);
+	p = strstr(p, key);
+	if (!p)
+		return -1;
+	*out = strtoull(p + strlen(key), NULL, 10);
+	return 0;
+}
+
+/*
+ * Pages of @fd that sit in large folios, found through /proc/self/pagemap
+ * and /proc/kpageflags. Returns -1 when the interfaces are not usable.
+ */
+static long large_folio_pages(int fd, unsigned long size, long *folios)
+{
+	unsigned long nr = size / PAGE_SIZE, i;
+	long large = 0, heads = 0;
+	int pagemap, kpageflags;
+	unsigned char *map;
+	uint64_t *pm;
+
+	pagemap = open("/proc/self/pagemap", O_RDONLY);
+	kpageflags = open("/proc/kpageflags", O_RDONLY);
+	pm = malloc(nr * sizeof(*pm));
+	map = mmap(NULL, size, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
+	if (pagemap < 0 || kpageflags < 0 || !pm || map == MAP_FAILED) {
+		large = -1;
+		goto out;
+	}
+	if (pread(pagemap, pm, nr * sizeof(*pm),
+		  (unsigned long)map / PAGE_SIZE * sizeof(*pm)) !=
+	    (ssize_t)(nr * sizeof(*pm))) {
+		large = -1;
+		goto out;
+	}
+	for (i = 0; i < nr; i++) {
+		uint64_t flags, pfn = pm[i] & PM_PFN_MASK;
+
+		if (!(pm[i] & PM_PRESENT) || !pfn ||
+		    pread(kpageflags, &flags, sizeof(flags),
+			  pfn * sizeof(flags)) != sizeof(flags)) {
+			large = -1;
+			goto out;
+		}
+		if (flags & (1ULL << KPF_COMPOUND_HEAD))
+			heads++;
+		if (flags & ((1ULL << KPF_COMPOUND_HEAD) |
+			     (1ULL << KPF_COMPOUND_TAIL)))
+			large++;
+	}
+	*folios = heads;
+out:
+	if (map != MAP_FAILED)
+		munmap(map, size);
+	free(pm);
+	if (pagemap >= 0)
+		close(pagemap);
+	if (kpageflags >= 0)
+		close(kpageflags);
+	return large;
+}
+
+static void cleanup(FIXTURE_DATA(numa_migrate) *self)
+{
+	char path[PATH_MAX];
+
+	if (self->data_fd >= 0) {
+		close(self->data_fd);
+		self->data_fd = -1;
+	}
+	if (self->dir[0]) {
+		snprintf(path, sizeof(path), "%s/data", self->mnt);
+		unlink(path);
+		if (self->mounted)
+			umount(self->mnt);
+		unlink(self->img);
+		rmdir(self->mnt);
+		rmdir(self->dir);
+		self->dir[0] = '\0';
+	}
+	if (self->cg_fd >= 0) {
+		close(self->cg_fd);
+		self->cg_fd = -1;
+	}
+	if (self->cgv2_fd >= 0) {
+		/* The test task may still sit in the cgroup after a failure. */
+		write_u64_at(self->cgv2_fd, "cgroup.procs", getpid());
+		unlinkat(self->cgv2_fd, CG_NAME, AT_REMOVEDIR);
+		close(self->cgv2_fd);
+		self->cgv2_fd = -1;
+	}
+}
+
+/*
+ * Returns 0, or 1 with everything undone when the host cannot run the test.
+ * The harness runs no teardown after a failed setup, so the undo is here.
+ */
+static int prepare(FIXTURE_DATA(numa_migrate) *self, const char **why)
+{
+	int fd;
+
+	self->cgv2_fd = mount_cg2_fd();
+	if (self->cgv2_fd < 0) {
+		*why = "cannot mount cgroup2";
+		goto fail;
+	}
+	if (write_file_at(self->cgv2_fd, "cgroup.subtree_control",
+			  "+memory +cpuset")) {
+		*why = "cannot enable the memory and cpuset controllers";
+		goto fail;
+	}
+	if (mkdirat(self->cgv2_fd, CG_NAME, 0755) && errno != EEXIST) {
+		*why = "cannot create the test cgroup";
+		goto fail;
+	}
+	self->cg_fd = openat(self->cgv2_fd, CG_NAME, O_PATH | O_DIRECTORY);
+	if (self->cg_fd < 0) {
+		*why = "cannot open the test cgroup";
+		goto fail;
+	}
+	if (faccessat(self->cg_fd, "memory.numa_migrate", W_OK, 0)) {
+		*why = "no memory.numa_migrate";
+		goto fail;
+	}
+	if (write_u64_at(self->cg_fd, "cpuset.mems", self->node_from)) {
+		*why = "cannot bind the cgroup to a node";
+		goto fail;
+	}
+
+	snprintf(self->dir, sizeof(self->dir), DIR_TEMPLATE);
+	if (!mkdtemp(self->dir)) {
+		self->dir[0] = '\0';
+		*why = "cannot create a temporary directory";
+		goto fail;
+	}
+	snprintf(self->img, sizeof(self->img), "%s/img", self->dir);
+	snprintf(self->mnt, sizeof(self->mnt), "%s/mnt", self->dir);
+	if (mkdir(self->mnt, 0755)) {
+		*why = "cannot create the mount point";
+		goto fail;
+	}
+	fd = open(self->img, O_CREAT | O_RDWR, 0600);
+	if (fd < 0) {
+		*why = "cannot create the image file";
+		goto fail;
+	}
+	if (ftruncate(fd, IMG_SIZE)) {
+		close(fd);
+		*why = "cannot size the image file";
+		goto fail;
+	}
+	close(fd);
+	if (run_cmd("mkfs.xfs -q -f %s", self->img)) {
+		*why = "mkfs.xfs failed";
+		goto fail;
+	}
+	if (run_cmd("mount -o loop %s %s", self->img, self->mnt)) {
+		*why = "cannot loop-mount the XFS image";
+		goto fail;
+	}
+	self->mounted = 1;
+	return 0;
+fail:
+	cleanup(self);
+	return 1;
+}
+
+FIXTURE_SETUP(numa_migrate)
+{
+	const char *why;
+
+	self->cgv2_fd = -1;
+	self->cg_fd = -1;
+	self->data_fd = -1;
+	self->dir[0] = '\0';
+
+	if (geteuid())
+		SKIP(return, "needs root");
+	if (two_nodes_with_memory(&self->node_from, &self->node_to))
+		SKIP(return, "needs two NUMA nodes with memory");
+	if (access("/usr/sbin/mkfs.xfs", X_OK) && access("/sbin/mkfs.xfs", X_OK))
+		SKIP(return, "needs mkfs.xfs");
+	if (prepare(self, &why))
+		SKIP(return, "%s", why);
+}
+
+FIXTURE_TEARDOWN(numa_migrate)
+{
+	if (self->leave_mounted) {
+		TH_LOG("leaving %s mounted, eviction would hit BUG_ON(nrpages)",
+		       self->mnt);
+		self->dir[0] = '\0';
+	}
+	cleanup(self);
+}
+
+TEST_F(numa_migrate, large_folio_order)
+{
+	unsigned long long on_to_before, on_to_after;
+	unsigned char *buf, *check;
+	char path[PATH_MAX], list[64];
+	long large, folios;
+	unsigned long off;
+	int fd, i;
+
+	ASSERT_EQ(write_u64_at(self->cg_fd, "cgroup.procs", getpid()), 0);
+
+	buf = malloc(CHUNK_SIZE);
+	check = malloc(CHUNK_SIZE);
+	ASSERT_NE(buf, NULL);
+	ASSERT_NE(check, NULL);
+
+	snprintf(path, sizeof(path), "%s/data", self->mnt);
+	fd = open(path, O_CREAT | O_RDWR | O_TRUNC, 0600);
+	ASSERT_GE(fd, 0);
+	self->data_fd = fd;
+	for (off = 0; off < FILE_SIZE; off += CHUNK_SIZE) {
+		memset(buf, (off / CHUNK_SIZE) & 0xff, CHUNK_SIZE);
+		ASSERT_EQ(pwrite(fd, buf, CHUNK_SIZE, off), (ssize_t)CHUNK_SIZE);
+	}
+	ASSERT_EQ(fsync(fd), 0);
+
+	large = large_folio_pages(fd, FILE_SIZE, &folios);
+	ASSERT_GE(large, 0) TH_LOG("cannot read the page cache folio sizes");
+	TH_LOG("%ld of %lu file pages are in %ld large folios",
+	       large, FILE_SIZE / PAGE_SIZE, folios);
+	ASSERT_GT(large, 0)
+		TH_LOG("no large folio in the page cache, the bug is out of reach");
+
+	ASSERT_EQ(file_bytes_on_node(self->cg_fd, self->node_to,
+				     &on_to_before), 0);
+
+	/* From here on a failure means eviction would crash the kernel. */
+	self->leave_mounted = 1;
+	snprintf(list, sizeof(list), "%d,%d", self->node_from, self->node_to);
+	ASSERT_EQ(write_file_at(self->cg_fd, "cpuset.mems", list), 0);
+	ASSERT_EQ(write_u64_at(self->cg_fd, "memory.numa_migrate",
+			       self->node_to), 0);
+
+	for (off = 0; off < FILE_SIZE; off += CHUNK_SIZE) {
+		memset(buf, (off / CHUNK_SIZE) & 0xff, CHUNK_SIZE);
+		ASSERT_EQ(pread(fd, check, CHUNK_SIZE, off), (ssize_t)CHUNK_SIZE);
+		ASSERT_EQ(memcmp(buf, check, CHUNK_SIZE), 0);
+	}
+	self->leave_mounted = 0;
+
+	/* memory.numa_stat flushes per-cpu counters lazily, give it time. */
+	for (i = 0; i < STAT_RETRIES; i++) {
+		ASSERT_EQ(file_bytes_on_node(self->cg_fd, self->node_to,
+					     &on_to_after), 0);
+		if (on_to_after >= on_to_before + FILE_SIZE)
+			break;
+		usleep(STAT_RETRY_US);
+	}
+	TH_LOG("file bytes on node %d: %llu -> %llu", self->node_to,
+	       on_to_before, on_to_after);
+	ASSERT_GE(on_to_after, on_to_before + FILE_SIZE)
+		TH_LOG("memory.numa_migrate moved nothing to node %d", self->node_to);
+
+	free(buf);
+	free(check);
+}
+
+TEST_HARNESS_MAIN

-- 
2.43.0

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

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

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: memcg numa_migrate uses no-reclaim THISNODE gfp for all large folio orders
--

commit dc423622ee7bf287039eede1edeca15b40cb3108
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

mm/memcontrol: allocate numa_migrate targets of the source folio order

This patch makes memcg_numa_migrate_new_folio() allocate the destination
folio with the same order as the source large folio, instead of always
using HPAGE_PMD_ORDER. It also drops the thp_migration_supported() check.

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 0734c0fa21d05..d8a87913ace37 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c

[ ... ]

> @@ -3487,13 +3488,13 @@ static struct folio *memcg_numa_migrate_new_folio(struct folio *old_folio,
>  		VM_BUG_ON(ms->current_node >= MAX_NUMNODES);
>  	}
>  
> -	if (thp_migration_supported() && folio_test_large(old_folio)) {
> -		return __folio_alloc(GFP_TRANSHUGE_LIGHT | __GFP_THISNODE,
> -				HPAGE_PMD_ORDER, ms->current_node,
> -				ms->target_nodes);
> +	if (folio_test_large(old_folio)) {
> +		gfp_mask = GFP_TRANSHUGE_LIGHT | __GFP_THISNODE;
> +		order = folio_order(old_folio);
>  	}

[Severity: Medium]
The order fix looks right, but the no-reclaim, single-node policy is now
applied to every large folio, not only PMD-sized THPs.

GFP_TRANSHUGE_LIGHT has no __GFP_DIRECT_RECLAIM, and __GFP_THISNODE pins
the allocation to ms->current_node. For a small-order file folio or an
mTHP, can the allocation now fail on a fragmented node, with no fallback
to the other nodes in target_nodes? Before this patch, such folios got an
order-0 destination allocated with GFP_HIGHUSER_MOVABLE | __GFP_NORETRY,
which does allow direct reclaim.

If the allocation fails, migrate_folio_unmap() returns -ENOMEM. In
migrate_pages_batch() the -ENOMEM case then does this:

    case -ENOMEM:
        ...
        if (is_large && !nosplit) {
            int ret = try_split_folio(folio, split_folios, mode);
            ...
        }
        ...
        rc_saved = rc;
        if (list_empty(&unmap_folios))
            goto out;

When the split also fails (for example -EAGAIN from an elevated refcount,
or a mapping whose minimum folio order is above 0), migrate_pages()
returns a negative value. Then in __memcg_numa_migrate_pages():

    ret = migrate_pages(&pages, memcg_numa_migrate_new_folio, ...);
    putback_movable_pages(&pages);
    if (ret < 0)
        return ret;

Does this abort the whole memory.numa_migrate write with -ENOMEM and
leave the remaining folios of the memcg unmigrated?

The old PMD-THP path had the same failure semantics. This patch widens
it to all large folio orders, and the commit message doesn't mention the
change in allocation policy.

Would it be reasonable to fall back to order 0 or to a reclaim-capable
gfp mask when the large allocation fails?

>  
> -	return __folio_alloc(gfp_mask, 0, ms->current_node, ms->target_nodes);
> +	return __folio_alloc(gfp_mask, order, ms->current_node,
> +			     ms->target_nodes);
>  }

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261028-b4-prep-khorenko-vstor-151607-numa-migrate-order-v2-0-b973a82a5b0c@virtuozzo.com?part=1

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

Re: [Devel] [PATCH hci-8.1 v2 2/2] selftests/ve: check memory.numa_migrate keeps the folio order

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] selftests/ve: large-folio guard accepts PMD-sized folios, can pass without hitting bug
- [Medium] selftests/ve: numa_migrate test needs 100% migration but large-folio dest alloc is no-retry
- [Low] selftests/ve: PAGE_SIZE hard-coded to 4096 in pagemap index arithmetic
--

commit bed50173f19f827d74b11f238428d4fcbb79ebf0
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

selftests/ve: check memory.numa_migrate keeps the folio order

This adds ve_numa_migrate_test. It writes a file on an XFS loop image
from a cgroup bound to one NUMA node, migrates the cgroup to a second
node with memory.numa_migrate, and reads the data back against the
written pattern. It also checks through memory.numa_stat that the file
pages moved, and checks through pagemap and kpageflags that the page
cache holds large folios.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-151607

> diff --git a/tools/testing/selftests/ve/ve_numa_migrate_test.c b/tools/testing/selftests/ve/ve_numa_migrate_test.c
> new file mode 100644
> index 0000000000000..b4a60dba8063d
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_numa_migrate_test.c

[ ... ]

> +#define IMG_SIZE	(320UL << 20)
> +#define FILE_SIZE	(4UL << 20)
> +#define CHUNK_SIZE	(64UL << 10)
> +#define PAGE_SIZE	4096UL

[Severity: Low]
Is hard-coding PAGE_SIZE to 4096UL safe here?

large_folio_pages() uses it for the page count and for the pagemap offset:

    unsigned long nr = size / PAGE_SIZE, i;
    ...
    (unsigned long)map / PAGE_SIZE * sizeof(*pm)

On a host with a base page size other than 4K (arm64 or ppc64 with 64K
pages), both values are wrong. The folio check can then give a bogus
failure or a bogus result. The "FILE_SIZE / PAGE_SIZE" value in the
TH_LOG message uses the same constant.

Would sysconf(_SC_PAGESIZE) be a better choice? A libc definition of
PAGE_SIZE could also make this define a redefinition.

[ ... ]

> +TEST_F(numa_migrate, large_folio_order)
> +{

[ ... ]

> +	large = large_folio_pages(fd, FILE_SIZE, &folios);
> +	ASSERT_GE(large, 0) TH_LOG("cannot read the page cache folio sizes");
> +	TH_LOG("%ld of %lu file pages are in %ld large folios",
> +	       large, FILE_SIZE / PAGE_SIZE, folios);
> +	ASSERT_GT(large, 0)
> +		TH_LOG("no large folio in the page cache, the bug is out of reach");

[Severity: Medium]
Does this check fully back the commit message claim that a file without
a suitable large folio "would pass without touching the bug"?

large_folio_pages() counts every page with KPF_COMPOUND_HEAD or
KPF_COMPOUND_TAIL, and the test only asserts large > 0. The bug needs a
source folio smaller than PMD order, because the old
memcg_numa_migrate_new_folio() always allocated a PMD-sized destination.

If the page cache held only PMD-order folios, the old and new code would
allocate identical destinations. The test would then pass on an unfixed
kernel without hitting the bug.

The test already computes "heads" and "large". Could it also require that
the folios are smaller than 512 pages, for example by checking that
large / folios is below 512?

[ ... ]

> +	/* memory.numa_stat flushes per-cpu counters lazily, give it time. */
> +	for (i = 0; i < STAT_RETRIES; i++) {
> +		ASSERT_EQ(file_bytes_on_node(self->cg_fd, self->node_to,
> +					     &on_to_after), 0);
> +		if (on_to_after >= on_to_before + FILE_SIZE)
> +			break;
> +		usleep(STAT_RETRY_US);
> +	}
> +	TH_LOG("file bytes on node %d: %llu -> %llu", self->node_to,
> +	       on_to_before, on_to_after);
> +	ASSERT_GE(on_to_after, on_to_before + FILE_SIZE)
> +		TH_LOG("memory.numa_migrate moved nothing to node %d", self->node_to);

[Severity: Medium]
Can this assertion fail on a healthy kernel?

It requires every one of the 4 MiB of file pages to land on node_to. The
migration path is:

memcg_numa_migrate_pages()
  __memcg_numa_migrate_pages()
    migrate_pages(&pages, memcg_numa_migrate_new_folio, NULL,
                  (unsigned long)&ms, MIGRATE_ASYNC, MR_SYSCALL, NULL)

For a large source folio, memcg_numa_migrate_new_folio() allocates the
destination with:

    gfp_mask = GFP_TRANSHUGE_LIGHT | __GFP_THISNODE;
    order = folio_order(old_folio);

GFP_TRANSHUGE_LIGHT has no direct reclaim and no compaction. Under
fragmentation or memory pressure on the target node this allocation can
fail, and migrate_pages() then leaves that folio on node_from.

__memcg_numa_migrate_pages() only propagates ret < 0:

    putback_movable_pages(&pages);
    if (ret < 0)
        return ret;

The write to memory.numa_migrate therefore still succeeds after a partial
migration. memcg_numa_isolate_pages() also skips folios silently when
folio_try_get() fails, the folio is not on the LRU, it is under writeback,
or folio_trylock() fails.

Any single skipped or failed folio leaves the node_to count below
on_to_before + FILE_SIZE. The data comparison has already passed at that
point, so the test would report a failure although the fix works. The
retry loop only waits for the lazy memcg flush and does not tolerate a
shortfall.

Should the final check allow some slack, or otherwise be made robust
against partial migration?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261028-b4-prep-khorenko-vstor-151607-numa-migrate-order-v2-0-b973a82a5b0c@virtuozzo.com?part=2

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

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

[Pavel Tikhomirov]

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

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

[Konstantin Khorenko]

On 10/28/26 15:22, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] mm: memcg numa_migrate uses no-reclaim THISNODE gfp for all large folio orders
> --
> 
> commit dc423622ee7bf287039eede1edeca15b40cb3108
> Author: Konstantin Khorenko <khorenko@virtuozzo.com>
> 
> mm/memcontrol: allocate numa_migrate targets of the source folio order
> 
> This patch makes memcg_numa_migrate_new_folio() allocate the destination
> folio with the same order as the source large folio, instead of always
> using HPAGE_PMD_ORDER. It also drops the thp_migration_supported() check.
> 
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 0734c0fa21d05..d8a87913ace37 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
> 
> [ ... ]
> 
>> @@ -3487,13 +3488,13 @@ static struct folio *memcg_numa_migrate_new_folio(struct folio *old_folio,
>>  		VM_BUG_ON(ms->current_node >= MAX_NUMNODES);
>>  	}
>>  
>> -	if (thp_migration_supported() && folio_test_large(old_folio)) {
>> -		return __folio_alloc(GFP_TRANSHUGE_LIGHT | __GFP_THISNODE,
>> -				HPAGE_PMD_ORDER, ms->current_node,
>> -				ms->target_nodes);
>> +	if (folio_test_large(old_folio)) {
>> +		gfp_mask = GFP_TRANSHUGE_LIGHT | __GFP_THISNODE;
>> +		order = folio_order(old_folio);
>>  	}
> 
> [Severity: Medium]
> The order fix looks right, but the no-reclaim, single-node policy is now
> applied to every large folio, not only PMD-sized THPs.
> 
> GFP_TRANSHUGE_LIGHT has no __GFP_DIRECT_RECLAIM, and __GFP_THISNODE pins
> the allocation to ms->current_node. For a small-order file folio or an
> mTHP, can the allocation now fail on a fragmented node, with no fallback
> to the other nodes in target_nodes? Before this patch, such folios got an
> order-0 destination allocated with GFP_HIGHUSER_MOVABLE | __GFP_NORETRY,
> which does allow direct reclaim.
> 
> If the allocation fails, migrate_folio_unmap() returns -ENOMEM. In
> migrate_pages_batch() the -ENOMEM case then does this:
> 
>     case -ENOMEM:
>         ...
>         if (is_large && !nosplit) {
>             int ret = try_split_folio(folio, split_folios, mode);
>             ...
>         }
>         ...
>         rc_saved = rc;
>         if (list_empty(&unmap_folios))
>             goto out;
> 
> When the split also fails (for example -EAGAIN from an elevated refcount,
> or a mapping whose minimum folio order is above 0), migrate_pages()
> returns a negative value. Then in __memcg_numa_migrate_pages():
> 
>     ret = migrate_pages(&pages, memcg_numa_migrate_new_folio, ...);
>     putback_movable_pages(&pages);
>     if (ret < 0)
>         return ret;
> 
> Does this abort the whole memory.numa_migrate write with -ENOMEM and
> leave the remaining folios of the memcg unmigrated?
> 
> The old PMD-THP path had the same failure semantics. This patch widens
> it to all large folio orders, and the commit message doesn't mention the
> change in allocation policy.
> 
> Would it be reasonable to fall back to order 0 or to a reclaim-capable
> gfp mask when the large allocation fails?

The allocation policy for large folios does not change here. Before
the patch every folio_test_large() folio already took the
GFP_TRANSHUGE_LIGHT | __GFP_THISNODE branch, only with HPAGE_PMD_ORDER.
The order-0 path with reclaim was reached for a large folio only when
thp_migration_supported() was false, which it never is on x86_64. So a
fragmented node is now asked for an order-4 folio where it used to be
asked for an order-9 one.

The fallback you suggest is in migrate_pages() already. On -ENOMEM
from the allocator migrate_pages_batch() calls try_split_folio() and
goes on with the batch, and migrate_pages() then runs the split pieces
through the same allocator as order-0 folios: GFP_HIGHUSER_MOVABLE |
__GFP_NORETRY, the whole target nodemask, no __GFP_THISNODE. Only when
the split fails too does the write return -ENOMEM with the rest
unmigrated, which is what a PMD THP did before the patch.

Whether large destinations should drop __GFP_THISNODE and allow direct
reclaim, as upstream alloc_migration_target() does with GFP_TRANSHUGE
and the nodemask, is a separate change, not part of this fix.


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

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

[Konstantin Khorenko]

On 10/28/26 16:30, Konstantin Khorenko wrote:
> On 10/28/26 15:22, no-reply@sashiko.virtuozzo.com wrote:
...
>> [Severity: Medium]
>> The order fix looks right, but the no-reclaim, single-node policy is now
>> applied to every large folio, not only PMD-sized THPs.
>>
>> GFP_TRANSHUGE_LIGHT has no __GFP_DIRECT_RECLAIM, and __GFP_THISNODE pins
>> the allocation to ms->current_node. For a small-order file folio or an
>> mTHP, can the allocation now fail on a fragmented node, with no fallback
>> to the other nodes in target_nodes? Before this patch, such folios got an
>> order-0 destination allocated with GFP_HIGHUSER_MOVABLE | __GFP_NORETRY,
>> which does allow direct reclaim.
>>
>> If the allocation fails, migrate_folio_unmap() returns -ENOMEM. In
>> migrate_pages_batch() the -ENOMEM case then does this:
>>
>>     case -ENOMEM:
>>         ...
>>         if (is_large && !nosplit) {
>>             int ret = try_split_folio(folio, split_folios, mode);
>>             ...
>>         }
>>         ...
>>         rc_saved = rc;
>>         if (list_empty(&unmap_folios))
>>             goto out;
>>
>> When the split also fails (for example -EAGAIN from an elevated refcount,
>> or a mapping whose minimum folio order is above 0), migrate_pages()
>> returns a negative value. Then in __memcg_numa_migrate_pages():
>>
>>     ret = migrate_pages(&pages, memcg_numa_migrate_new_folio, ...);
>>     putback_movable_pages(&pages);
>>     if (ret < 0)
>>         return ret;
>>
>> Does this abort the whole memory.numa_migrate write with -ENOMEM and
>> leave the remaining folios of the memcg unmigrated?
>>
>> The old PMD-THP path had the same failure semantics. This patch widens
>> it to all large folio orders, and the commit message doesn't mention the
>> change in allocation policy.
>>
>> Would it be reasonable to fall back to order 0 or to a reclaim-capable
>> gfp mask when the large allocation fails?
> 
> The allocation policy for large folios does not change here. Before
> the patch every folio_test_large() folio already took the
> GFP_TRANSHUGE_LIGHT | __GFP_THISNODE branch, only with HPAGE_PMD_ORDER.
> The order-0 path with reclaim was reached for a large folio only when
> thp_migration_supported() was false, which it never is on x86_64. So a
> fragmented node is now asked for an order-4 folio where it used to be
> asked for an order-9 one.
> 
> The fallback you suggest is in migrate_pages() already. On -ENOMEM
> from the allocator migrate_pages_batch() calls try_split_folio() and
> goes on with the batch, and migrate_pages() then runs the split pieces
> through the same allocator as order-0 folios: GFP_HIGHUSER_MOVABLE |
> __GFP_NORETRY, the whole target nodemask, no __GFP_THISNODE. Only when
> the split fails too does the write return -ENOMEM with the rest
> unmigrated, which is what a PMD THP did before the patch.
> 
> Whether large destinations should drop __GFP_THISNODE and allow direct
> reclaim, as upstream alloc_migration_target() does with GFP_TRANSHUGE
> and the nodemask, is a separate change, not part of this fix.
> 

On the gfp mask for large destinations.

The patch does not change it: large folios took GFP_TRANSHUGE_LIGHT |
__GFP_THISNODE before, only with order 9. Compared with GFP_TRANSHUGE
in upstream alloc_migration_target() it has no direct reclaim (the
allocation is served from the free lists or fails at once, no
compaction either) and no fallback to the other target nodes.

The cost: on a fragmented node the large allocation fails,
migrate_pages() splits the folio and moves the pieces as order-0 pages,
with reclaim and the whole nodemask. The data arrives intact, but as 4K
pages, which the page cache does not grow back. GFP_TRANSHUGE would
keep the folio size more often at the price of a reclaim/compaction
stall in vcmmd, the writer of memory.numa_migrate.

Upstream makes the same choice for the same job: NUMA balancing's
alloc_misplaced_dst_folio() gives large folios exactly
GFP_TRANSHUGE_LIGHT | __GFP_THISNODE. Placement is an optimisation and
should neither reclaim nor land on the wrong node. So I would leave it
unless a two-node host shows NFS or XFS throughput dropping after a VM
migration; the selftest can measure the split rate through
/proc/kpageflags, and then a reclaiming mask is a separate change.



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