The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: John Stultz <jstultz@google.com>
Cc: Tejun Heo <tj@kernel.org>, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	Christian Loehle <christian.loehle@arm.com>,
	David Dai <david.dai@linux.dev>, Koba Ko <kobak@nvidia.com>,
	Aiqun Yu <aiqun.yu@oss.qualcomm.com>,
	Shuah Khan <shuah@kernel.org>,
	sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 08/11] sched_ext: Delegate proxy donor admission to BPF schedulers
Date: Sat, 18 Jul 2026 16:23:21 +0200	[thread overview]
Message-ID: <aluMWf5H936NXA0N@gpd4> (raw)
In-Reply-To: <CANDhNCp01g3PN4=hZgCBLUn79_H++y8hyAr4RZP+C3MYckon7g@mail.gmail.com>

Hi John,

On Fri, Jul 17, 2026 at 11:50:20PM -0700, John Stultz wrote:
> On Fri, Jul 17, 2026 at 11:16 PM John Stultz <jstultz@google.com> wrote:
> >
> > On Thu, Jul 16, 2026 at 6:23 AM Andrea Righi <arighi@nvidia.com> wrote:
> > > @@ -3098,18 +3164,16 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
> > >                 set_task_runnable(rq, p);
> > >
> > >                 /*
> > > -                * Mutex-blocked donors stay queued on the runqueue under proxy
> > > -                * execution, but the donor never runs as itself, proxy-exec
> > > -                * walks the blocked_on chain on the next __schedule() and runs
> > > -                * the lock owner in its place.
> > > +                * Mutex-blocked donors only stay queued when their BPF scheduler
> > > +                * enables %SCX_OPS_ENQ_BLOCKED. The rq lock has remained held since
> > > +                * scx_allow_proxy_exec(), so @p's scheduler association cannot have
> > > +                * changed and @sch must be non-NULL with the flag set.
> > >                  *
> > > -                * Put the donor on the local DSQ directly so pick_next_task()
> > > -                * can still see it. find_proxy_task() will either run the chain
> > > -                * owner or deactivate the donor so the wakeup path can return it
> > > -                * and let BPF make a new dispatch decision once it is unblocked.
> > > +                * Delegate admission to the BPF scheduler.
> > >                  */
> > >                 if (p->is_blocked) {
> > > -                       scx_dispatch_enqueue(sch, rq, &rq->scx.local_dsq, p, 0);
> > > +                       WARN_ON_ONCE(!(sch->ops.flags & SCX_OPS_ENQ_BLOCKED));
> > > +                       scx_do_enqueue_task(rq, p, 0, -1);
> > >                         goto switch_class;
> > >                 }
> > >
> >
> > So this isn't a blocker for your patches, but just as a heads up: when
> > applying my sleeping owner handling changes (even just patches 4-7
> > from my v30 submission[1]), I managed to trip the above WARN_ON, when
> > running the test-ww_mutex driver under the scx_pair (usually right as
> > scx_pair loads).
> >
> > [ 5818.970324] WARNING: kernel/sched/ext/ext.c:3175 at
> > put_prev_task_scx+0x527/0x550, CPU#54: kworker/u261:11/30527
> > [ 5818.974927] CPU: 54 UID: 0 PID: 30527 Comm: kworker/u261:11
> > Tainted: G        W           7.1.0-13303-gb5ecf3c9f881 #116
> > PREEMPT(full)
> > [ 5818.979484] Tainted: [W]=WARN
> > [ 5818.980654] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> > BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> > [ 5818.984225] Workqueue: test-ww_mutex test_cycle_work
> > [ 5818.986199] Sched_ext: pair (enabled+all), task: runnable_at=-369ms
> > [ 5818.986202] RIP: 0010:put_prev_task_scx+0x527/0x550
> > [ 5818.990473] Code: 48 2b 05 34 f3 21 03 75 3c 48 83 c4 30 48 89 de
> > 48 89 ef b9 ff ff ff ff 5b 31 d2 5d 41 5c 41 5d 41 5e 41 5f e9 fa f0
> > ff ff 90 <0f> 0b 90 e9 b8 fe ff ff 90 0f 0b 90 e9 35 fe ff ff b8 02 00
> > 00 00
> > [ 5818.997428] RSP: 0018:ffffc900088dfb28 EFLAGS: 00010046
> > [ 5818.999426] RAX: 0000000000000036 RBX: ffff888101258000 RCX: ffff888101d64990
> > [ 5819.002113] RDX: ffff8881b9bae638 RSI: ffff888101d64990 RDI: ffff888101258390
> > [ 5819.004787] RBP: ffff8881b9badb00 R08: ffff8881b9badb00 R09: 0000000000000000
> > [ 5819.007967] R10: 0000000000000036 R11: ffff888120a9b030 R12: ffff888100c30000
> > [ 5819.010697] R13: ffff88810d163800 R14: ffff888101258000 R15: ffff888100c30000
> > [ 5819.013378] FS:  0000000000000000(0000) GS:ffff8882355c0000(0000)
> > knlGS:0000000000000000
> > [ 5819.016354] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 5819.018524] CR2: 00007ffe7b985348 CR3: 0000000123ec6004 CR4: 0000000000370ef0
> > [ 5819.021163] Call Trace:
> > [ 5819.022168]  <TASK>
> > [ 5819.023010]  __schedule+0x15c4/0x2460
> > [ 5819.024504]  ? find_held_lock+0x2b/0x80
> > [ 5819.025981]  ? lock_release+0x191/0x310
> > [ 5819.027503]  schedule+0x3d/0x130
> > [ 5819.028772]  schedule_preempt_disabled+0x18/0x30
> > [ 5819.030539]  __ww_mutex_lock.constprop.0+0xaea/0x19e0
> > [ 5819.032492]  ? schedule_timeout+0xca/0x130
> > [ 5819.034095]  ? test_cycle_work+0x15d/0x340
> > [ 5819.035686]  ? ww_mutex_lock+0x3c/0xb0
> > [ 5819.037094]  ww_mutex_lock+0x3c/0xb0
> > [ 5819.038494]  test_cycle_work+0x15d/0x340
> > [ 5819.039949]  process_one_work+0x20c/0x5e0
> > [ 5819.041631]  ? lock_is_held_type+0xcd/0x130
> > [ 5819.043277]  worker_thread+0x1a4/0x360
> > [ 5819.044717]  ? __pfx_worker_thread+0x10/0x10
> > [ 5819.046392]  kthread+0x103/0x130
> > [ 5819.047650]  ? __pfx_kthread+0x10/0x10
> > [ 5819.049074]  ret_from_fork+0x27c/0x340
> > [ 5819.050525]  ? __pfx_kthread+0x10/0x10
> > [ 5819.051990]  ret_from_fork_asm+0x1a/0x30
> > [ 5819.053488]  </TASK>
> > [ 5819.054371] irq event stamp: 38
> > [ 5819.055583] hardirqs last  enabled at (37): [<ffffffff826a7100>]
> > _raw_spin_unlock_irqrestore+0x50/0x60
> > [ 5819.059054] hardirqs last disabled at (38): [<ffffffff82699205>]
> > __schedule+0xb95/0x2460
> > [ 5819.061998] softirqs last  enabled at (26): [<ffffffff813244d0>]
> > __irq_exit_rcu+0xe0/0x150
> > [ 5819.065015] softirqs last disabled at (21): [<ffffffff813244d0>]
> > __irq_exit_rcu+0xe0/0x150
> > [ 5819.068013] ---[ end trace 0000000000000000 ]---
> >
> > It seems to be coming from the proxy_resched_idle() call in the "if
> > (!READ_ONCE(owner->on_rq) || owner->se.sched_delayed) {" case in
> > find_proxy_task(), prior to proxy_enqueue_on_owner() calling
> > block_task().
> >
> > I'll have to dig a bit more next week on this, as I'm not yet seeing
> > whats going wrong here.
> 
> Ok, my tired theory is something like:
> 
> 1) You have a task A that that is_blocked waiting on a sleeping owner
> B. It gets enqueued onto that owner and waits.
> 
> 2) We start scx_pair, and scx_prepare_task_sched_change() calls
> sched_proxy_block_task(), which bails because !task_on_rq_queued()
> 
> 3) B wakes up, and that causes us to activate_blocked_waiters(), which
> re-adds A (with is_blocked still set) to the rq
>    - this is problematic because now we have is_blocked tasks on the
> sched_ext DSQ where its not allowed.
> 
> 4) A is selected as a donor, and maybe B is sleeping again, so we call
> put_prev_set_next() and hti the warning because we see A is is_blocked
> when the sched_ext scheduler doesn't support it.
> 
> I'll work to prove this out a bit further next week.  I suspect we'll
> need something somewhere between activate_blocked_waiters() ->
> scx_do_enqueue_task() to skip enquing of is_blocked tasks when
> SCX_OPS_ENQ_BLOCKED isn't set.

Your theory looks correct to me. I was also able to reproduce this. I think the
problem is that scx_prepare_task_sched_change() calls sched_proxy_block_task(),
but the latter has nothing to do when the donor is already off the runqueue
behind a sleeping owner.

When the owner subsequently wakes, activate_blocked_waiters() unconditionally
reactivates the donor while is_blocked is still set. Since this activation
carries ENQUEUE_WAKEUP, sched_ext treats it as a normal wakeup rather than a
blocked-donor admission. The donor can therefore enter an scx scheduler that
doesn't set SCX_OPS_ENQ_BLOCKED (scx_pair in this case), eventually triggering
the warning in put_prev_task_scx(): unexpected proxy donor without
SCX_OPS_ENQ_BLOCKED set.

I think we can fix this by checking scx_allow_proxy_exec() in
do_activate_blocked_waiter() before reactivating the donor. If the scx scheduler
doesn't support proxy donors (SCX_OPS_ENQ_BLOCKED not set), the task should
remain blocked and will be activated normally by the mutex wakeup.

While looking at this, I noticed another issue in the same path:
activate_blocked_waiters() currently passes ENQUEUE_WAKEUP because the generic
scheduling classes need wakeup-style enqueue accounting. However, this is not a
real mutex wakeup, the donor remains blocked and is only being made runnable so
that it can donate its scheduling context.

As a result, even a scheduler that sets SCX_OPS_ENQ_BLOCKED currently receives
this sleeping-owner activation as SCX_ENQ_WAKEUP rather than SCX_ENQ_BLOCKED.
That's because the existing sched_ext test:

  p->is_blocked && !(enq_flags & SCX_ENQ_WAKEUP)

doesn't distinguish a genuine mutex wakeup from this proxy activation.

Maybe we can address this by adding an internal ENQUEUE_PROXY flag to the
sleeping-owner activation. The generic classes will continue to see
ENQUEUE_WAKEUP, preserving their accounting behavior, while sched_ext will use
ENQUEUE_PROXY to expose the event to BPF as SCX_ENQ_BLOCKED without
SCX_ENQ_WAKEUP. And a genuine mutex wakeup will continue to be reported as
SCX_ENQ_WAKEUP. What do you think?

Thanks,
-Andrea

  reply	other threads:[~2026-07-18 14:23 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 13:20 [PATCHSET v7 sched_ext/for-7.3] sched: Make proxy execution compatible with sched_ext Andrea Righi
2026-07-16 13:20 ` [PATCH 01/11] sched: Make NOHZ CFS bandwidth checks follow proxy donor Andrea Righi
2026-07-18  3:11   ` John Stultz
2026-07-16 13:20 ` [PATCH 02/11] sched: Add helper to block retained proxy donors Andrea Righi
2026-07-16 13:20 ` [PATCH 03/11] sched_ext: Block proxy donors across scheduler transitions Andrea Righi
2026-07-18  3:16   ` John Stultz
2026-07-16 13:20 ` [PATCH 04/11] sched_ext: Fix ops.running/stopping() pairing for proxy-exec donors Andrea Righi
2026-07-16 13:20 ` [PATCH 05/11] sched_ext: Fix TOCTOU race in consume_remote_task() Andrea Righi
2026-07-16 21:29   ` Tejun Heo
2026-07-16 21:38     ` Tejun Heo
2026-07-17  6:35       ` Andrea Righi
2026-07-16 13:20 ` [PATCH 06/11] sched_ext: Split curr|donor references properly Andrea Righi
2026-07-16 13:20 ` [PATCH 07/11] sched_ext: Handle blocked donor migration with proxy execution Andrea Righi
2026-07-16 13:20 ` [PATCH 08/11] sched_ext: Delegate proxy donor admission to BPF schedulers Andrea Righi
2026-07-18  6:16   ` John Stultz
2026-07-18  6:50     ` John Stultz
2026-07-18 14:23       ` Andrea Righi [this message]
2026-07-16 13:20 ` [PATCH 09/11] sched_ext: Add selftest for blocked donor admission Andrea Righi
2026-07-16 13:20 ` [PATCH 10/11] sched_ext: scx_qmap: Add proxy execution support Andrea Righi
2026-07-18  2:28   ` John Stultz
2026-07-18  5:47     ` Andrea Righi
2026-07-18  6:04       ` John Stultz
2026-07-18  8:24         ` Andrea Righi
2026-07-16 13:20 ` [PATCH 11/11] sched: Allow enabling proxy exec with sched_ext Andrea Righi
  -- strict thread matches above, loose matches on Subject: below --
2026-07-15 20:54 [PATCHSET v6 sched_ext/for-7.3] sched: Make proxy execution compatible " Andrea Righi
2026-07-15 20:54 ` [PATCH 08/11] sched_ext: Delegate proxy donor admission to BPF schedulers 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=aluMWf5H936NXA0N@gpd4 \
    --to=arighi@nvidia.com \
    --cc=aiqun.yu@oss.qualcomm.com \
    --cc=bsegall@google.com \
    --cc=changwoo@igalia.com \
    --cc=christian.loehle@arm.com \
    --cc=david.dai@linux.dev \
    --cc=dietmar.eggemann@arm.com \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kobak@nvidia.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=shuah@kernel.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=void@manifault.com \
    --cc=vschneid@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox