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

[no-reply@sashiko.virtuozzo.com]

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.

>  		cpumask_clear(mask);
> +		bitmap_set(cpumask_bits(mask), 0, vcpus_avail);

[Severity: Low]
Can this return an empty mask inside a CT?

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;
>  }

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029-ct-invalid-nrpoc-v3-0-89d4b0b7d520@virtuozzo.com?part=1

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