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

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

On 10/29/26 11:21, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] sched: sched_getaffinity() takes vCPU limit from target task, not caller
> - [Low] sched: sched_getaffinity() in CT can return an empty mask with no floor of 1
> --
> 
> commit 89d21190c29ff2e196f8d22db77072e4bd2c2544
> Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> 
> sched: Fix sched_getaffinity syscall
> 
> This patch changes sched_getaffinity() so that, inside a container, the
> reported mask is a prefix of set bits. The number of bits is the smaller of
> the task's vCPU limit and the weight of cpus_mask & cpu_active_mask. Host
> behavior is unchanged.
> 
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-141682
> 
>> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
>> --- a/kernel/sched/syscalls.c
>> +++ b/kernel/sched/syscalls.c
> 
> [ ... ]
> 
>> @@ -1316,15 +1316,23 @@ long sched_getaffinity(pid_t pid, struct cpumask *mask)
>>   	if (retval)
>>   		return retval;
>>   
>> +	scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
>> +		cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
>> +	}
>> +
>>   	if (!ve_is_super(get_exec_env())) {
>> +		/*
>> +		 * setaffinity is not available inside CT, so these bits
>> +		 * can only be used for counting.
>> +		 * We can't have more than vcpus and we can't run on CPUs
>> +		 * outside the mask. Reporting smallest will provide true
>> +		 * information about CT resources.
>> +		 */
>> +		unsigned int vcpus_avail = min(task_nr_cpus(p), cpumask_weight(mask));
> 
> [Severity: Low]
> Previously the vCPU limit came from num_online_vcpus(), which is
> task_nr_cpus(current), so it was based on the caller. Now it comes from
> task_nr_cpus(p), the target task, while the VE check still uses the
> caller's exec env.
> 
> Can a task in a CT that calls sched_getaffinity(pid) for another task
> in a different task group now get a different CPU count than it gets for
> itself?
> 
> For example, a task in a child cpu cgroup has nr_cpus of 0, which is
> clamped to num_online_cpus(). The CT root group may have a lower limit.
> 
> Callers that query themselves, such as nproc, see no change. The other
> users of num_online_vcpus() (/proc/cpuinfo, sysfs, net-procfs) still use
> the caller's limit.
> 
> Is this change intentional? The commit message only mentions that the VE
> check is done against the current exec env, and does not mention that the
> vCPU limit is now taken from the target task.

Yes, it is intentional. I think the current approach is correct.

> 
>>   		cpumask_clear(mask);
>> +		bitmap_set(cpumask_bits(mask), 0, vcpus_avail);
> 
> [Severity: Low]
> Can this return an empty mask inside a CT?

Already replied in v2. It is fine.

> 
> vcpus_avail depends on cpumask_weight(mask), which is the weight of
> p->cpus_mask & cpu_active_mask. If that intersection is empty, vcpus_avail
> is 0 and bitmap_set(..., 0, 0) leaves the mask all zero.
> 
> One way this can happen is during a CPU hotplug window. The task's
> cpus_mask may only hold CPUs that are already cleared from
> cpu_active_mask. select_fallback_rq() does not repair the mask until the
> task next wakes.
> 
> The old CT path used num_online_vcpus(), which is task_nr_cpus(current).
> That function is clamped to at least num_online_cpus(), so it never
> returned an empty mask. In this case nproc and similar tools would report
> 0.
> 
> Would a floor of 1 be appropriate here?
> 
>>   	}
>>   
>> -	guard(raw_spinlock_irqsave)(&p->pi_lock);
>> -	cpumask_and(mask, &p->cpus_mask, cpu_active_mask);
>> -
>>   	return 0;
>>   }
> 

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

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