All of lore.kernel.org
 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: 29+ 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 14:39   ` sashiko-bot
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 15:02   ` sashiko-bot
2026-07-16 13:20 ` [PATCH 07/11] sched_ext: Handle blocked donor migration with proxy execution Andrea Righi
2026-07-16 15:22   ` sashiko-bot
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-16 15:54   ` sashiko-bot
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 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.