From: Kuba Piecuch <jpiecuch@google.com>
To: Tejun Heo <tj@kernel.org>, Kuba Piecuch <jpiecuch@google.com>
Cc: Andrea Righi <arighi@nvidia.com>,
Changwoo Min <changwoo@igalia.com>,
David Vernet <void@manifault.com>,
<linux-kernel@vger.kernel.org>, <sched-ext@lists.linux.dev>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH sched_ext/for-7.1-fixes] sched_ext: Call wakeup_preempt() in local_dsq_post_enq()
Date: Mon, 27 Apr 2026 14:14:33 +0000 [thread overview]
Message-ID: <DI3ZVX73P4N5.WP4H0TMOBIHF@google.com> (raw)
In-Reply-To: <aeulrBs6ZAopfcHG@slm.duckdns.org>
Hi Tejun,
On Fri Apr 24, 2026 at 5:17 PM UTC, Tejun Heo wrote:
> Hello, Kuba.
>
> On Fri, Apr 24, 2026 at 09:22:44AM +0000, Kuba Piecuch wrote:
>> @@ -1408,11 +1407,19 @@ static void local_dsq_post_enq(struct scx_sched *sch, struct scx_dispatch_q *dsq
>> if ((enq_flags & SCX_ENQ_PREEMPT) && p != rq->curr &&
>> rq->curr->sched_class == &ext_sched_class) {
>> rq->curr->scx.slice = 0;
>> - preempt = true;
>> + resched_curr(rq);
>> }
>>
>> - if (preempt || sched_class_above(&ext_sched_class, rq->curr->sched_class))
>> - resched_curr(rq);
>
> Hmm... I don't quite understand this part of the change. sched_class_above()
> got separated out into its own case but why is it dropping resched_curr() on
> SCX_ENQ_PREEMPT?
In the SCX_ENQ_PREEMPT case we call resched_curr() where we previously set
preempt = true.
In the sched_class_above() case, wakeup_preempt() will call resched_curr()
for us:
void wakeup_preempt(struct rq *rq, struct task_struct *p, int flags)
{
[...]
if (p->sched_class == rq->next_class) {
rq->next_class->wakeup_preempt(rq, p, flags);
} else if (sched_class_above(p->sched_class, rq->next_class)) {
rq->next_class->wakeup_preempt(rq, p, flags);
=====> resched_curr(rq); <=====
rq->next_class = p->sched_class;
}
[...]
}
>
>> + /*
>> + * If @rq->next_class is currently idle, we need to bump it
>> + * to &ext_sched_class using wakeup_preempt(). Otherwise, if we drop
>> + * the rq lock later in the pick and an RT task wakes up on @rq,
>> + * wakeup_preempt_idle() will be called during RT task wakeup and
>> + * SCX won't have an opportunity to re-enqueue IMMED tasks from @rq's
>> + * local DSQ.
>
> As this was really subtle, I think it warrants documenting all cases here.
Yeah, I was trying to keep it concise. How about something like this:
/*
* Note that @rq's lock may be dropped between this enqueue and @p
* actually getting on CPU. This gives higher-class tasks (e.g. RT)
* an opportunity to wake up on @rq and prevent @p from running.
* Here are some concrete examples:
*
* Example 1:
*
* We dispatch two tasks from a single ops.dispatch():
* - First, a local task to this CPU's local DSQ;
* - Second, a local/remote task to a remote CPU's local DSQ.
* We must drop the local rq lock in order to finish the second
* dispatch. In that time, an RT task can wake up on the local rq.
*
* Example 2:
*
* We dispatch a local/remote task to a remote CPU's local DSQ.
* We must drop the remote rq lock before the dispatched task can run,
* which gives an RT task an opportunity to wake up on the remote rq.
*
* Both examples work the same if we replace dispatching with moving
* the tasks from a user-created DSQ.
*
* We must detect these wakeups so that we can re-enqueue IMMED tasks
* from @rq's local DSQ. scx_wakeup_preempt() serves exactly this
* purpose, but for it to be invoked, we must ensure that we bump
* @rq->next_class to &ext_sched_class if it's currently idle.
*
* wakeup_preempt() does the bumping, and since we only invoke it if
* @rq->next_class is below &ext_sched_class, it will also
* resched_curr(rq).
*/
Thanks,
Kuba
next prev parent reply other threads:[~2026-04-27 14:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-24 9:22 [PATCH sched_ext/for-7.1-fixes] sched_ext: Call wakeup_preempt() in local_dsq_post_enq() Kuba Piecuch
2026-04-24 17:17 ` Tejun Heo
2026-04-27 14:14 ` Kuba Piecuch [this message]
2026-04-27 17:01 ` Tejun Heo
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=DI3ZVX73P4N5.WP4H0TMOBIHF@google.com \
--to=jpiecuch@google.com \
--cc=arighi@nvidia.com \
--cc=changwoo@igalia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=sched-ext@lists.linux.dev \
--cc=tj@kernel.org \
--cc=void@manifault.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