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

[Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>]

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