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

[Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>]

> 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, checks that something was migrated and reads the file back
> against the written pattern. 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.
> 
> 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
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
>
> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
> index 51f9316beb99..e96320a18384 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 000000000000..5fbf4af49f7f
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_numa_migrate_test.c
> @@ -0,0 +1,304 @@
> +// 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.
> + */
> +#define _GNU_SOURCE
> +#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 <linux/sched.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)
> +
> +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;
> +}
> +
> +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: evicting the file would hit "
> +		       "BUG_ON(nrpages) in clear_inode()", 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];
> +	unsigned long off;
> +	int fd;
> +
> +	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);
> +
> +	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);
> +
> +	ASSERT_EQ(file_bytes_on_node(self->cg_fd, self->node_to,
> +				     &on_to_after), 0);
> +	TH_LOG("file bytes on node %d: %llu -> %llu", self->node_to,
> +	       on_to_before, on_to_after);

Non blocker: in hosts with many CPUs we could read unflushed stats.
Maybe do the memcmp loop first and retry numa_stat read for a few
seconds.

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel