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

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

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