[Devel] [PATCH hci-8.1 0/2] sched: Fix incorrect nproc report inside CT

[Devel] [PATCH hci-8.1 0/2] sched: Fix incorrect nproc report inside CT

[Vladimir Riabchun]

We observed weird nproc behavior - it reported 64 CPUs
no matter what the CT limits were or how many CPUs the host
actually had.

nproc code uses sched_getaffinity syscall under the hood[1].
It appears that this syscall returned complete nonsence when
called from VE - we ignored both num_online_vcpus and
cgroup cpuset limitations.

Now it is fixed and a new selftest is added.

[1]: https://github.com/coreutils/gnulib/blob/master/lib/nproc.c#L144
https://virtuozzo.atlassian.net/browse/VSTOR-141682

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
Vladimir Riabchun (2):
      sched: Fix sched_getaffinity syscall
      selftests: ve: Test sched_getaffinity in VE

 kernel/sched/syscalls.c                            |  13 +-
 tools/testing/selftests/ve/.gitignore              |   1 +
 tools/testing/selftests/ve/Makefile                |   1 +
 .../selftests/ve/ve_sched_getaffinity_test.c       | 147 +++++++++++++++++++++
 4 files changed, 156 insertions(+), 6 deletions(-)
---
base-commit: a1c0fd720976500f854fd5a4c946a54e65c82d78
change-id: 20261027-ct-invalid-nrpoc-f183b3ba691d

Best regards,
--  
Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

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

[Devel] [PATCH hci-8.1 1/2] sched: Fix sched_getaffinity syscall

[Vladimir Riabchun]

We observed incorrect cpu numbers inside containers:
[root@localhost ~]# nproc
64

even when there were much less physical/virtual CPUs.

The root cause is incorrect bitmap_fill usage - it sets whole
integers only, without handling bit tails, so the actual number of
set bits is 64 = bits in ULONG.
Anyway, this approach is incorrect - it doesn't respect
container cpuset.

Fix sched_getaffinity syscall by reusing p->cpus_mask, and bounding it
with num_online_vcpus().

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

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 kernel/sched/syscalls.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 6d921043e9e7..92051bace212 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -1305,6 +1305,7 @@ SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
 long sched_getaffinity(pid_t pid, struct cpumask *mask)
 {
 	struct task_struct *p;
+	unsigned int online_vcpus = num_online_vcpus();
 	int retval;
 
 	guard(rcu)();
@@ -1316,14 +1317,14 @@ long sched_getaffinity(pid_t pid, struct cpumask *mask)
 	if (retval)
 		return retval;
 
-	if (!ve_is_super(get_exec_env())) {
-		cpumask_clear(mask);
-		bitmap_fill(cpumask_bits(mask), num_online_vcpus());
-		return 0;
+	scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
+		cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
 	}
 
-	guard(raw_spinlock_irqsave)(&p->pi_lock);
-	cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
+	if (!ve_is_super(get_exec_env()) &&
+	    !WARN_ON_ONCE(online_vcpus > nr_cpumask_bits))
+		bitmap_clear(cpumask_bits(mask), online_vcpus,
+			     nr_cpumask_bits - online_vcpus);
 
 	return 0;
 }

-- 
2.52.0

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

[Devel] [PATCH hci-8.1 2/2] selftests: ve: Test sched_getaffinity in VE

[Vladimir Riabchun]

Add a new selftest that verifies that sched_getaffinity respects both
cgroup cpuset and nr_cpus limits.

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

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 tools/testing/selftests/ve/.gitignore              |   1 +
 tools/testing/selftests/ve/Makefile                |   1 +
 .../selftests/ve/ve_sched_getaffinity_test.c       | 147 +++++++++++++++++++++
 3 files changed, 149 insertions(+)

diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore
index 8ab1bfe6f5e4..178de0ee1822 100644
--- a/tools/testing/selftests/ve/.gitignore
+++ b/tools/testing/selftests/ve/.gitignore
@@ -3,3 +3,4 @@ ve_perms_test
 ve_mount_accounting_test
 ve_cpuid_override_test
 ve_vtty_stress
+ve_sched_getaffinity_test
diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
index 51f9316beb99..d53e724205f7 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_sched_getaffinity_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_sched_getaffinity_test.c b/tools/testing/selftests/ve/ve_sched_getaffinity_test.c
new file mode 100644
index 000000000000..a47cee832c3b
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_sched_getaffinity_test.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ve_mount_accounting selftests
+ *
+ * Tests to check cpuset virtualization.
+ */
+#define _GNU_SOURCE
+#include <asm/unistd.h>
+#include <linux/sched.h>
+#include <linux/limits.h>
+#include <sys/wait.h>
+#include <sys/syscall.h>
+#include <sys/stat.h>
+#include <sys/mount.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <time.h>
+
+#include "../kselftest_harness.h"
+#include "ve_selftest.h"
+
+FIXTURE(ve_getaff_test)
+{
+	int cgv2_fd;
+	int ctid;
+	struct namespace_ctx ns_ctx;
+};
+
+static int __getaffinity_ve(void *cpuset)
+{
+	cpu_set_t *dst = cpuset;
+
+	return sched_getaffinity(getpid(), sizeof(*dst), dst);
+}
+
+static int getaffinity_ve(FIXTURE_DATA(ve_getaff_test) * self, cpu_set_t *dst)
+{
+	int ret = -1;
+	cpu_set_t *tmp = mmap(NULL, sizeof(cpu_set_t),
+		PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	if (tmp == MAP_FAILED) {
+		fprintf(stderr, "Failed to mmap shared region: %s\n", strerror(errno));
+		return -1;
+	}
+	CPU_ZERO(tmp);
+	CPU_ZERO(dst);
+
+	ret = run_in_ve(self->cgv2_fd, self->ctid, &self->ns_ctx,
+		CLONE_NEWVE, __getaffinity_ve, tmp);
+
+	if (!ret)
+		memcpy(dst, tmp, sizeof(cpu_set_t));
+
+	munmap(tmp, sizeof(cpu_set_t));
+	return ret;
+}
+
+FIXTURE_SETUP(ve_getaff_test)
+{
+	cpu_set_t this, in_ve;
+
+	self->cgv2_fd = mount_cg2_fd();
+	ASSERT_GE(self->cgv2_fd, 0);
+
+	ASSERT_EQ(write_file_at(self->cgv2_fd, "cgroup.subtree_control",
+		  VE_CONTROLLERS), 0);
+
+	self->ctid = make_ve(self->cgv2_fd, CTID_MIN);
+	ASSERT_GE(self->ctid, 0);
+
+	ASSERT_EQ(prepare_namespaces(&self->ns_ctx, CLONE_NEWVE,
+				self->cgv2_fd, self->ctid), 0);
+
+
+	ASSERT_EQ(sched_getaffinity(getpid(), sizeof(this), &this), 0);
+	if (CPU_COUNT(&this) < 2)
+		SKIP(return, "Not enough CPUs available");
+
+	/* Chech that we are talking about the same CPUs */
+	ASSERT_EQ(getaffinity_ve(self, &in_ve), 0);
+	ASSERT_TRUE(CPU_EQUAL(&this, &in_ve));
+};
+
+FIXTURE_TEARDOWN(ve_getaff_test)
+{
+	close_namespace_fds(&self->ns_ctx);
+	destroy_ve(self->cgv2_fd, self->ctid);
+	close(self->cgv2_fd);
+}
+
+/* Test correct processing of cpuset.cpus updates */
+TEST_F(ve_getaff_test, update_cg_cpuset)
+{
+	char path[PATH_MAX];
+	int cpu_victim = 0;
+	cpu_set_t in_ve;
+
+	snprintf(path, sizeof(path), "%d/cpuset.cpus", self->ctid);
+	if (faccessat(self->cgv2_fd, path, F_OK, 0))
+		SKIP(return, "CT cgroup doesn't have cpuset.cpus entry");
+
+	ASSERT_EQ(getaffinity_ve(self, &in_ve), 0);
+
+	/* Find one CPU and leave it alone via cgroup interface */
+	while (!CPU_ISSET(cpu_victim, &in_ve))
+		cpu_victim++;
+
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path, cpu_victim), 0);
+
+	/* Verify sched_getaffinity sees updated value */
+	ASSERT_EQ(getaffinity_ve(self, &in_ve), 0);
+	ASSERT_EQ(CPU_COUNT(&in_ve), 1);
+	ASSERT_TRUE(CPU_ISSET(cpu_victim, &in_ve));
+}
+
+/* Test correct processing of cpu.nr_cpus updates */
+TEST_F(ve_getaff_test, nr_proc_bound)
+{
+	char path[PATH_MAX];
+	int cpu_victim = 0;
+	cpu_set_t in_ve;
+
+	snprintf(path, sizeof(path), "%d/cpu.nr_cpus", self->ctid);
+	if (faccessat(self->cgv2_fd, path, F_OK, 0))
+		SKIP(return, "CT cgroup doesn't have cpu.nr_cpus entry");
+
+	ASSERT_EQ(getaffinity_ve(self, &in_ve), 0);
+
+	/* Find one CPU and bound nr_cpus by its value */
+	while (!CPU_ISSET(cpu_victim, &in_ve))
+		cpu_victim++;
+
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path, cpu_victim + 1), 0);
+
+	/* Verify sched_getaffinity sees only one CPU */
+	ASSERT_EQ(getaffinity_ve(self, &in_ve), 0);
+	ASSERT_EQ(CPU_COUNT(&in_ve), 1);
+	ASSERT_TRUE(CPU_ISSET(cpu_victim, &in_ve));
+}
+
+TEST_HARNESS_MAIN

-- 
2.52.0

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

Re: [Devel] [PATCH hci-8.1 1/2] sched: Fix sched_getaffinity syscall

[Pavel Tikhomirov]

On 10/27/26 21:51, Vladimir Riabchun wrote:
> We observed incorrect cpu numbers inside containers:
> [root@localhost ~]# nproc
> 64
> 
> even when there were much less physical/virtual CPUs.
> 
> The root cause is incorrect bitmap_fill usage - it sets whole
> integers only, without handling bit tails, so the actual number of
> set bits is 64 = bits in ULONG.

Agreed, this is the real bug.

> Anyway, this approach is incorrect - it doesn't respect
> container cpuset.

Why vcpu should respect cpuset? Those are different things. cpu.nr_cpus
(num_online_vcpus()) is the number of virtual cpus the container sees, it
limits cpu time and is not bound to any physical cpu. Cpuset is a binding to
physical cpu ids. The old code returned the virtual view [0, nr_vcpus), the
same as we show in /sys/devices/system/cpu/{online,possible,present}
(show_cpus_attr()) and in /proc/cpuinfo.

> 
> Fix sched_getaffinity syscall by reusing p->cpus_mask, and bounding it
> with num_online_vcpus().
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-141682
> 
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>  kernel/sched/syscalls.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index 6d921043e9e7..92051bace212 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -1305,6 +1305,7 @@ SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
>  long sched_getaffinity(pid_t pid, struct cpumask *mask)
>  {
>  	struct task_struct *p;
> +	unsigned int online_vcpus = num_online_vcpus();
>  	int retval;
>  
>  	guard(rcu)();
> @@ -1316,14 +1317,14 @@ long sched_getaffinity(pid_t pid, struct cpumask *mask)
>  	if (retval)
>  		return retval;
>  
> -	if (!ve_is_super(get_exec_env())) {
> -		cpumask_clear(mask);
> -		bitmap_fill(cpumask_bits(mask), num_online_vcpus());
> -		return 0;
> +	scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
> +		cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
>  	}
>  
> -	guard(raw_spinlock_irqsave)(&p->pi_lock);
> -	cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
> +	if (!ve_is_super(get_exec_env()) &&
> +	    !WARN_ON_ONCE(online_vcpus > nr_cpumask_bits))
> +		bitmap_clear(cpumask_bits(mask), online_vcpus,
> +			     nr_cpumask_bits - online_vcpus);

This intersects a physical cpu mask with a virtual cpu range and if intersection
is empty I think it will look inconsistent.

One option is just to fix the bitmap_fill() part and keep the virtual view:

        if (!ve_is_super(get_exec_env())) {
                cpumask_clear(mask);
                bitmap_set(cpumask_bits(mask), 0, num_online_vcpus());
                return 0;
        }

Or if we want cpuset represented to follow mainstream more we should intersect those
smartly with mapping, e.g.:

        if (!ve_is_super(get_exec_env())) {
                unsigned int nr = num_online_vcpus(), cpu;
                cpumask_var_t phys;     /* or a local loop over a copy */
                ...
                cpumask_and(phys, &p->cpus_mask, cpu_active_mask);
                cpumask_clear(mask);
                for_each_cpu(cpu, phys)
                        __cpumask_set_cpu(cpu % nr, mask);
        }

same mapping as in task_vcpu_id.

>  
>  	return 0;
>  }
> 

-- 
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 1/2] sched: Fix sched_getaffinity syscall

[Vladimir Riabchun]

On 10/28/26 10:34, Pavel Tikhomirov wrote:
> 
> 
> On 10/27/26 21:51, Vladimir Riabchun wrote:
>> We observed incorrect cpu numbers inside containers:
>> [root@localhost ~]# nproc
>> 64
>>
>> even when there were much less physical/virtual CPUs.
>>
>> The root cause is incorrect bitmap_fill usage - it sets whole
>> integers only, without handling bit tails, so the actual number of
>> set bits is 64 = bits in ULONG.
> 
> Agreed, this is the real bug.
> 
>> Anyway, this approach is incorrect - it doesn't respect
>> container cpuset.
> 
> Why vcpu should respect cpuset? Those are different things. cpu.nr_cpus
> (num_online_vcpus()) is the number of virtual cpus the container sees, it
> limits cpu time and is not bound to any physical cpu. Cpuset is a binding to
> physical cpu ids. The old code returned the virtual view [0, nr_vcpus), the
> same as we show in /sys/devices/system/cpu/{online,possible,present}
> (show_cpus_attr()) and in /proc/cpuinfo.

As discussed, I'll change the logic so we return
[0; min(vr_vcpus, count(mask))) bits.

We can't use setaffinity from inside CT, so these bits
can be used only for counting, and min(vr_vcpus, count(mask))
is the closest to the actual resources CT has.

> 
>>
>> Fix sched_getaffinity syscall by reusing p->cpus_mask, and bounding it
>> with num_online_vcpus().
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-141682
>>
>> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>> ---
>>   kernel/sched/syscalls.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
>> index 6d921043e9e7..92051bace212 100644
>> --- a/kernel/sched/syscalls.c
>> +++ b/kernel/sched/syscalls.c
>> @@ -1305,6 +1305,7 @@ SYSCALL_DEFINE3(sched_setaffinity, pid_t, pid, unsigned int, len,
>>   long sched_getaffinity(pid_t pid, struct cpumask *mask)
>>   {
>>   	struct task_struct *p;
>> +	unsigned int online_vcpus = num_online_vcpus();
>>   	int retval;
>>   
>>   	guard(rcu)();
>> @@ -1316,14 +1317,14 @@ long sched_getaffinity(pid_t pid, struct cpumask *mask)
>>   	if (retval)
>>   		return retval;
>>   
>> -	if (!ve_is_super(get_exec_env())) {
>> -		cpumask_clear(mask);
>> -		bitmap_fill(cpumask_bits(mask), num_online_vcpus());
>> -		return 0;
>> +	scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
>> +		cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
>>   	}
>>   
>> -	guard(raw_spinlock_irqsave)(&p->pi_lock);
>> -	cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
>> +	if (!ve_is_super(get_exec_env()) &&
>> +	    !WARN_ON_ONCE(online_vcpus > nr_cpumask_bits))
>> +		bitmap_clear(cpumask_bits(mask), online_vcpus,
>> +			     nr_cpumask_bits - online_vcpus);
> 
> This intersects a physical cpu mask with a virtual cpu range and if intersection
> is empty I think it will look inconsistent.
> 
> One option is just to fix the bitmap_fill() part and keep the virtual view:
> 
>          if (!ve_is_super(get_exec_env())) {
>                  cpumask_clear(mask);
>                  bitmap_set(cpumask_bits(mask), 0, num_online_vcpus());
>                  return 0;
>          }
> 
> Or if we want cpuset represented to follow mainstream more we should intersect those
> smartly with mapping, e.g.:
> 
>          if (!ve_is_super(get_exec_env())) {
>                  unsigned int nr = num_online_vcpus(), cpu;
>                  cpumask_var_t phys;     /* or a local loop over a copy */
>                  ...
>                  cpumask_and(phys, &p->cpus_mask, cpu_active_mask);
>                  cpumask_clear(mask);
>                  for_each_cpu(cpu, phys)
>                          __cpumask_set_cpu(cpu % nr, mask);
>          }
> 
> same mapping as in task_vcpu_id.
> 
>>   
>>   	return 0;
>>   }
>>
> 

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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