All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: sashiko-reviews@lists.linux.dev
Cc: sched-ext@lists.linux.dev
Subject: Re: [PATCH 14/18] sched_ext: Split curr|donor references properly
Date: Tue, 8 Sep 2026 12:15:14 +0200	[thread overview]
Message-ID: <ap_gMp2soO8xZLxa@gpd4> (raw)
In-Reply-To: <20260831174907.D6F001F000E9@smtp.kernel.org>

On Mon, Aug 31, 2026 at 05:49:07PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The physical proxy execution owner is spuriously exposed to the BPF scheduler and loses its personal slice during SAVE/RESTORE cycles.
> --
> 
> commit 9c0210f5a5bf78476570e1133b4136f7fc67852c
> Author: Andrea Righi <arighi@nvidia.com>
> 
> sched_ext: Split curr|donor references properly
> 
> This commit separates the physical execution context (rq->curr) from the
> scheduling context (rq->donor) to support proxy execution in sched_ext.
> It updates various scheduler paths to use the donor where appropriate.
> 
> > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > index b2e6be4a887de..f23e9f1ee2f3c 100644
> > --- a/kernel/sched/ext/ext.c
> > +++ b/kernel/sched/ext/ext.c
> [ ... ]
> > @@ -2170,13 +2177,14 @@ static void enqueue_task_scx(struct rq *rq, struct task_struct *p, int core_enq_
> >  		rq->scx.flags |= SCX_RQ_IN_WAKEUP;
> >  
> >  	/*
> > -	 * Restoring a running task will be immediately followed by
> > -	 * set_next_task_scx() which expects the task to not be on the BPF
> > +	 * Restoring the current scheduling context will be immediately followed
> > +	 * by set_next_task_scx() which expects the task to not be on the BPF
> >  	 * scheduler as tasks can only start running through local DSQs. Force
> >  	 * direct-dispatch into the local DSQ by setting the sticky_cpu. Mark
> >  	 * IGNORE_CAPS to force entry into the local DSQ.
> >  	 */
> > -	if (unlikely(enq_flags & ENQUEUE_RESTORE) && task_current(rq, p)) {
> > +	if (unlikely(enq_flags & ENQUEUE_RESTORE) &&
> > +	    task_current_donor(rq, p)) {
> 
> [Severity: High]
> Does this check fail to recognize the physical execution context during
> ENQUEUE_RESTORE?
> 
> During proxy execution, the currently executing task (rq->curr) and the donor
> (rq->donor) are different. If a property change (such as sched_setscheduler or
> setpriority) is requested on the physically executing task while it is actively
> running, it will undergo a DEQUEUE_SAVE / ENQUEUE_RESTORE cycle.
> 
> Because task_current_donor(rq, p) evaluates to false for the physically
> executing task, wouldn't this bypass the SCX_ENQ_IGNORE_CAPS fast-path and
> erroneously expose the actively executing task to ops.enqueue(), which violates
> the constraint that physical execution contexts are not exposed to the BPF
> scheduler?
> 
> [ ... ]
> > @@ -2342,7 +2350,7 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int core_deq_
> >  	p->scx.flags &= ~SCX_TASK_REENQ_REASON_MASK;
> >  
> >  	/* see scx_task_slice_ended() for the save/restore exception */
> > -	if (!((deq_flags & DEQUEUE_SAVE) && task_current(rq, p)))
> > +	if (!((deq_flags & DEQUEUE_SAVE) && task_current_donor(rq, p)))
> >  		scx_task_slice_ended(rq, p);
> 
> [Severity: High]
> Similarly, does this fail to recognize the physical execution context during
> DEQUEUE_SAVE?
> 
> If the physically executing task undergoes a DEQUEUE_SAVE cycle,
> task_current_donor(rq, p) evaluates to false. Could this spuriously trigger
> scx_task_slice_ended(), incorrectly zeroing out its personal slice while it is
> running on behalf of the donor?

I think these are all false positives, the use of task_current_donor() here is
intentional. SAVE/RESTORE is preserving the current scheduling context, rather
than the physical execution context.

-Andrea

  reply	other threads:[~2026-09-08 10:15 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 13:42 [PATCHSET v13 sched_ext/for-7.4] sched: Make proxy execution compatible with sched_ext Andrea Righi
2026-08-31 13:42 ` [PATCH 01/18] sched/core: Drop mutex locks before proxy rescheduling Andrea Righi
2026-08-31 13:42 ` [PATCH 02/18] sched/core: Dequeue waking proxy donors before reset Andrea Righi
2026-09-01  5:24   ` K Prateek Nayak
2026-09-08  9:28     ` Andrea Righi
2026-08-31 13:42 ` [PATCH 03/18] sched: Make NOHZ CFS bandwidth checks follow proxy donor Andrea Righi
2026-09-10  9:54   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 04/18] sched/core: Avoid false migration warning for proxy donors Andrea Righi
2026-09-10 10:06   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 05/18] sched: Pass next class to sched_change_begin() Andrea Righi
2026-09-10 10:12   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 06/18] sched: Add helper to block retained proxy donors Andrea Righi
2026-08-31 13:42 ` [PATCH 07/18] sched: Add sched_ext hooks for proxy execution Andrea Righi
2026-09-10 10:38   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 08/18] sched: Introduce WF_ON_RQ wake flag Andrea Righi
2026-09-10 10:45   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 09/18] sched_ext: Block proxy donors across scheduler transitions Andrea Righi
2026-09-10 10:53   ` Peter Zijlstra
2026-09-10 11:41     ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 10/18] sched_ext: Fix ops.running/stopping() pairing for proxy-exec donors Andrea Righi
2026-08-31 13:42 ` [PATCH 11/18] sched_ext: Move reject DSQ draining into core Andrea Righi
2026-08-31 13:42 ` [PATCH 12/18] sched_ext: Generalize the reject DSQ reenqueue path Andrea Righi
2026-09-03 22:39   ` Tejun Heo
2026-09-08  9:34     ` Andrea Righi
2026-08-31 13:42 ` [PATCH 13/18] sched_ext: Handle proxy-exec races in remote DSQ transfers Andrea Righi
2026-08-31 13:42 ` [PATCH 14/18] sched_ext: Split curr|donor references properly Andrea Righi
2026-08-31 17:49   ` sashiko-bot
2026-09-08 10:15     ` Andrea Righi [this message]
2026-09-10 11:47   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 15/18] sched_ext: Delegate proxy donor admission to BPF schedulers Andrea Righi
2026-08-31 18:08   ` sashiko-bot
2026-09-08 10:08     ` Andrea Righi
2026-09-10 13:39   ` Peter Zijlstra
2026-09-10 13:41   ` Peter Zijlstra
2026-08-31 13:42 ` [PATCH 16/18] sched_ext: Add selftest for blocked donor admission Andrea Righi
2026-08-31 13:42 ` [PATCH 17/18] sched_ext: scx_qmap: Add proxy execution support Andrea Righi
2026-08-31 18:33   ` sashiko-bot
2026-09-01  7:52   ` Richard Cheng
2026-09-08  9:42     ` Andrea Righi
2026-08-31 13:42 ` [PATCH 18/18] sched: Allow enabling proxy exec with sched_ext Andrea Righi
2026-09-03 22:51 ` [PATCHSET v13 sched_ext/for-7.4] sched: Make proxy execution compatible " Tejun Heo
2026-09-08  8:02   ` Peter Zijlstra

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=ap_gMp2soO8xZLxa@gpd4 \
    --to=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.