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

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/ve: numa_migrate test leaves +memory +cpuset enabled in root cgroup
--

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

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

This adds a selftest that writes a file on an XFS image from a cgroup
bound to one NUMA node, migrates the cgroup to a second node with
memory.numa_migrate, and checks that the data reads back intact and that
large folios kept their order. It tracks the folios through
/proc/self/pagemap, /proc/kpageflags and move_pages().

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
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_numa_migrate_test.c

[ ... ]

> +static void cleanup(FIXTURE_DATA(numa_migrate) *self)
> +{
> +	char path[PATH_MAX];
> +
[ ... ]
> +	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;
> +	}
> +}
> +
[ ... ]
> +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")) {

[Severity: Low]
This isn't a functional defect in the checked logic, but prepare() writes
"+memory +cpuset" to the root cgroup.subtree_control, and cleanup() only
removes the CG_NAME cgroup with unlinkat(AT_REMOVEDIR).

Is it intended that cleanup() never writes "-cpuset -memory" back?

On a host where the root had neither controller enabled in
subtree_control, every run changes the host-wide cgroup2 controller state,
since the mount from mount_cg2_fd() is a view of the single cgroup2
hierarchy. Later tests may then see different controller delegation.

If the test is killed or times out, the harness skips teardown, so the
loop mount, the image file and the /var/tmp/ve_numa_migrate.* directory
can also be left behind.

Could prepare() record whether the controllers were already enabled, and
could cleanup() restore the previous state?

> +		*why = "cannot enable the memory and cpuset controllers";
> +		goto fail;
> +	}

[ ... ]

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

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