All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Andrea Righi" <arighi@nvidia.com>
Cc: sched-ext@lists.linux.dev
Subject: Re: [PATCH 02/14] sched: Make NOHZ CFS bandwidth checks follow proxy donor
Date: Sat, 25 Jul 2026 16:26:39 +0000	[thread overview]
Message-ID: <20260725162640.36CE51F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260725160513.57477-3-arighi@nvidia.com>

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

Pre-existing issues:
- [High] The newly added donor bandwidth check in `sched_can_stop_tick()` is bypassed by early returns when a FAIR donor delegates execution to a throttled RT task.
--

--- Patch [2]: [PATCH 02/14] sched: Make NOHZ CFS bandwidth checks follow proxy donor ---
commit 18e332090abc5adb7229eaf795a740846b882ae9
Author: Andrea Righi <arighi@nvidia.com>

sched: Make NOHZ CFS bandwidth checks follow proxy donor

This commit modifies sched_can_stop_tick() and sched_fair_update_stop_tick()
to check the CFS bandwidth constraints of the selected donor scheduling context
rather than the physical execution context. This prevents constrained FAIR
donors from evading bandwidth enforcement during proxy execution.

Link: https://lore.kernel.org/r/20260713164807.E5ED21F00A3A@smtp.kernel.org

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f38332274f8fe..ef0563970d412 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c

[ ... ]

> @@ -1462,14 +1459,17 @@ bool sched_can_stop_tick(struct rq *rq)
>  		return false;
>  
>  	/*
> -	 * If there is one task and it has CFS runtime bandwidth constraints
> -	 * and it's on the cpu now we don't want to stop the tick.
> +	 * If the selected scheduling context has CFS runtime bandwidth
> +	 * constraints, don't stop the tick. With proxy execution, rq->donor is
> +	 * the selected scheduling context while rq->curr is the task physically
> +	 * executing on its behalf.
> +	 *
>  	 * This check prevents clearing the bit if a newly enqueued task here is
> -	 * dequeued by migrating while the constrained task continues to run.
> +	 * dequeued by migrating while the constrained donor continues to run.
>  	 * E.g. going from 2->1 without going through pick_next_task().
>  	 */
> -	if (__need_bw_check(rq, rq->curr)) {
> -		if (cfs_task_bw_constrained(rq->curr))
> +	if (__need_bw_check(rq->donor)) {
> +		if (cfs_task_bw_constrained(rq->donor))
>  			return false;
>  	}

[Severity: High]
This is a pre-existing issue, but does this placement still allow a CFS
bandwidth-constrained FAIR task to evade its quota if it donates its context
to a throttled RT task?

If a FAIR task with CFS bandwidth constraints blocks on a mutex held by an
RT task, and the RT task exceeds its global RT runtime (becoming throttled),
the RT task's own context becomes ineligible. Proxy execution would then
select the FAIR donor's scheduling context (rq->donor), while the RT task
serves as the execution context (rq->curr).

During global RT throttling, dequeue_top_rt_rq() unsets rt_queued but does
not decrement rt_nr_running, leaving rq->rt.rt_nr_running > 0.

Because sched_can_stop_tick() has early checks for RT tasks (which can
return true when fifo_nr_running > 0), wouldn't the function return early
and completely bypass this newly added __need_bw_check(rq->donor) logic at
the bottom of the function?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725160513.57477-1-arighi@nvidia.com?part=2

  reply	other threads:[~2026-07-25 16:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25 16:04 [PATCHSET v9 sched_ext/for-7.3] sched: Make proxy execution compatible with sched_ext Andrea Righi
2026-07-25 16:04 ` [PATCH 01/14] sched/core: Avoid false migration warning for proxy donors Andrea Righi
2026-07-25 16:04 ` [PATCH 02/14] sched: Make NOHZ CFS bandwidth checks follow proxy donor Andrea Righi
2026-07-25 16:26   ` sashiko-bot [this message]
2026-07-25 16:04 ` [PATCH 03/14] sched: Add helper to block retained proxy donors Andrea Righi
2026-07-25 16:04 ` [PATCH 04/14] sched: Add prepare_switch() class callback Andrea Righi
2026-07-25 16:25   ` sashiko-bot
2026-07-25 16:04 ` [PATCH 05/14] sched: Add sched_ext hooks for proxy execution Andrea Righi
2026-07-25 16:14   ` sashiko-bot
2026-07-25 16:04 ` [PATCH 06/14] sched_ext: Block proxy donors across scheduler transitions Andrea Righi
2026-07-25 16:04 ` [PATCH 07/14] sched_ext: Fix ops.running/stopping() pairing for proxy-exec donors Andrea Righi
2026-07-25 16:04 ` [PATCH 08/14] sched_ext: Generalize the reject DSQ reenqueue path Andrea Righi
2026-07-25 16:04 ` [PATCH 09/14] sched_ext: Handle proxy-exec races in remote DSQ transfers Andrea Righi
2026-07-25 16:04 ` [PATCH 10/14] sched_ext: Split curr|donor references properly Andrea Righi
2026-07-25 16:04 ` [PATCH 11/14] sched_ext: Delegate proxy donor admission to BPF schedulers Andrea Righi
2026-07-25 16:04 ` [PATCH 12/14] sched_ext: Add selftest for blocked donor admission Andrea Righi
2026-07-25 16:04 ` [PATCH 13/14] sched_ext: scx_qmap: Add proxy execution support Andrea Righi
2026-07-25 16:04 ` [PATCH 14/14] sched: Allow enabling proxy exec with sched_ext Andrea Righi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260725162640.36CE51F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=arighi@nvidia.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sched-ext@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.