Re: [PATCH 02/12] sched: Make NOHZ CFS bandwidth checks follow proxy donor

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fails to remove the `rq->nr_running != 1` restriction from `sched_fair_update_stop_tick()`, leaving proxy execution bandwidth enforcement broken during context switches.
--

--- Patch [2]: [PATCH 02/12] sched: Make NOHZ CFS bandwidth checks follow proxy donor ---
commit 4af18b2dbe3d9116ee4049aa70ca4c8662c0104c
Author: Andrea Righi <[email protected]>

sched: Make NOHZ CFS bandwidth checks follow proxy donor

Proxy execution separates the scheduling context in rq->donor from the
physical execution context in rq->curr. sched_can_stop_tick() checks the
latter for CFS bandwidth constraints and only does so when nr_running is
one. A retained proxy donor keeps both the donor and mutex owner queued,
so this patch checks the selected donor instead and removes the
nr_running restriction.

Link: https://lore.kernel.org/r/[email protected]

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f788b72d1bfff..35d41d45e365e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1409,11 +1409,8 @@ static void nohz_csd_func(void *info)
>  #endif /* CONFIG_NO_HZ_COMMON */
>  
>  #ifdef CONFIG_NO_HZ_FULL
> -static inline bool __need_bw_check(struct rq *rq, struct task_struct *p)
> +static inline bool __need_bw_check(struct task_struct *p)
>  {
> -	if (rq->nr_running != 1)
> -		return false;
> -

[Severity: High]
This is a pre-existing issue, but does removing this restriction here fully
resolve the proxy execution bandwidth enforcement bug?

Looking at sched_fair_update_stop_tick() in kernel/sched/fair.c, there is an
identical check that prevents tick updates when nr_running != 1:

kernel/sched/fair.c:sched_fair_update_stop_tick() {
	if (rq->nr_running != 1)
		return;
	...
}

If a constrained FAIR task donates its scheduling context to a lower-priority
task (like an SCX task) via proxy execution, nr_running becomes >= 2 while
h_nr_queued remains 1.

During enqueue, sched_can_stop_tick() evaluates against the old rq->donor
(e.g., the idle task) and may clear the tick dependency. When the scheduler
picks the FAIR donor, sched_fair_update_stop_tick() returns early because
rq->nr_running != 1.

Can this result in the tick remaining cleared, allowing a constrained FAIR
task to bypass its CFS bandwidth quota?

Should the nr_running restriction in sched_fair_update_stop_tick() also be
removed or updated to properly account for proxy execution?

>  	if (p->sched_class != &fair_sched_class)
>  		return false;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2