mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Kuba Piecuch <jpiecuch@google.com>
Cc: Tejun Heo <tj@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
	David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] sched/core: Preserve WF_RQ_SELECTED across queued wakeups
Date: Fri, 23 Oct 2026 22:12:46 +0200	[thread overview]
Message-ID: <atu_vjtcwe1BjCaf@gpd4> (raw)
In-Reply-To: <20261023144811.2230393-2-jpiecuch@google.com>

Hi Kuba,

On Fri, Oct 23, 2026 at 02:48:04PM +0000, Kuba Piecuch wrote:
> ENQUEUE_RQ_SELECTED tells a sched_class that ->select_task_rq() was
> called for a task wakeup. sched_ext exposes this flag to BPF schedulers
> as SCX_ENQ_CPU_SELECTED. However, the flag is lost for queued wakeups,
> which a BPF scheduler can opt into with SCX_OPS_ALLOW_QUEUED_WAKEUP.
> This also results in undercounting SCX_EV_SELECT_CPU_FALLBACK events.
> 
> Preserve WF_RQ_SELECTED for queued wakeups in the same way WF_MIGRATED
> is. This way, ttwu_activate() receives WF_RQ_SELECTED for queued wakeups
> and passes ENQUEUE_RQ_SELECTED to activate_task().
> 
> Fixes: 3539c6411a7c ("sched_ext: Implement SCX_OPS_ALLOW_QUEUED_WAKEUP")
> Signed-off-by: Kuba Piecuch <jpiecuch@google.com>

Thanks for catching this, it makes sense to me.

In the description we could clarify that we're fixing the queued wakeups
from try_to_wake_up() after ->select_task_rq() has been called.

Maybe something along these lines:
---
When a BPF scheduler sets SCX_OPS_ALLOW_QUEUED_WAKEUP, a wakeup can be
delivered through the ttwu wakelist in two ways:

 1) from the p->on_cpu path, before ->select_task_rq() is called. In this
    case WF_RQ_SELECTED is not set and SCX_ENQ_CPU_SELECTED is correctly
    clear;

 2) from ttwu_queue(), after ->select_task_rq() has been called. In this
    case WF_RQ_SELECTED is set in wake_flags, but sched_ttwu_pending()
    only carries WF_MIGRATED across the wakelist, so the flag is dropped
    and ops.enqueue() is wrongly told that ops.select_cpu() was skipped.
    This also results in undercounting SCX_EV_SELECT_CPU_FALLBACK events.

Fix case 2 by preserving WF_RQ_SELECTED across the wakelist in the same
way WF_MIGRATED is, so that ttwu_do_activate() passes
ENQUEUE_RQ_SELECTED to activate_task(). Case 1 is unaffected.
---

Also nit: s/ttwu_activate()/ttwu_do_activate()/.

With that:

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> ---
>  include/linux/sched.h |  6 ++++--
>  kernel/sched/core.c   | 10 +++++++++-
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 12a7e094a3e6..9dabffea1c54 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1006,7 +1006,7 @@ struct task_struct {
>  	/* Unserialized, strictly 'current' */
>  
>  	/*
> -	 * This field must not be in the scheduler word above due to wakelist
> +	 * These fields must not be in the scheduler word above due to wakelist
>  	 * queueing no longer being serialized by p->on_cpu. However:
>  	 *
>  	 * p->XXX = X;			ttwu()
> @@ -1016,9 +1016,11 @@ struct task_struct {
>  	 *     p->on_rq = 0;			p->sched_remote_wakeup = Y;
>  	 *
>  	 * guarantees all stores of 'current' are visible before
> -	 * ->sched_remote_wakeup gets used, so it can be in this word.
> +	 * ->sched_remote_wakeup and ->sched_rq_selected get used, so they can
> +	 * be in this word.
>  	 */
>  	unsigned			sched_remote_wakeup:1;
> +	unsigned			sched_rq_selected:1;
>  #ifdef CONFIG_RT_MUTEXES
>  	unsigned			sched_rt_mutex:1;
>  #endif
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 0ab15f03cb94..d8495735166d 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3975,13 +3975,20 @@ void sched_ttwu_pending(void *arg)
>  	update_rq_clock(rq);
>  
>  	llist_for_each_entry_safe(p, t, llist, wake_entry.llist) {
> +		int wake_flags = 0;
> +
>  		if (WARN_ON_ONCE(p->on_cpu))
>  			smp_cond_load_acquire(&p->on_cpu, !VAL);
>  
>  		if (WARN_ON_ONCE(task_cpu(p) != cpu_of(rq)))
>  			set_task_cpu(p, cpu_of(rq));
>  
> -		ttwu_do_activate(rq, p, p->sched_remote_wakeup ? WF_MIGRATED : 0, &rf);
> +		if (p->sched_remote_wakeup)
> +			wake_flags |= WF_MIGRATED;
> +		if (p->sched_rq_selected)
> +			wake_flags |= WF_RQ_SELECTED;
> +
> +		ttwu_do_activate(rq, p, wake_flags, &rf);
>  	}
>  
>  	/*
> @@ -4025,6 +4032,7 @@ static void __ttwu_queue_wakelist(struct task_struct *p, int cpu, int wake_flags
>  	struct rq *rq = cpu_rq(cpu);
>  
>  	p->sched_remote_wakeup = !!(wake_flags & WF_MIGRATED);
> +	p->sched_rq_selected = !!(wake_flags & WF_RQ_SELECTED);
>  
>  	WRITE_ONCE(rq->ttwu_pending, 1);
>  #ifdef CONFIG_SMP
> -- 
> 2.56.0.257.gf7497c7d27-goog
> 

  parent reply	other threads:[~2026-10-23 20:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23 14:48 [PATCH 0/2] sched: Fix SCX_ENQ_CPU_SELECTED for " Kuba Piecuch
2026-10-23 14:48 ` [PATCH 1/2] sched/core: Preserve WF_RQ_SELECTED across " Kuba Piecuch
2026-10-23 19:35   ` Tejun Heo
2026-10-23 20:12   ` Andrea Righi [this message]
2026-10-23 14:48 ` [PATCH 2/2] selftests/sched_ext: Test SCX_ENQ_CPU_SELECTED with " Kuba Piecuch

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=atu_vjtcwe1BjCaf@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=jpiecuch@google.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

all inboxes | Powered by JetHome®