All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	Qiurong Fang <fangqiurong@kylinos.cn>,
	sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH sched_ext/for-7.3-fixes] sched_ext: Wait for SCX_OPSS_DISPATCHING before reenqueueing a task
Date: Tue, 15 Sep 2026 22:54:32 +0200	[thread overview]
Message-ID: <aqmwiJ1StVe1EH4L@gpd4> (raw)
In-Reply-To: <a4304b3f0c89fba23f1e5e92997a8f56@kernel.org>

Hi Tejun,

On Tue, Sep 15, 2026 at 10:03:37AM -1000, Tejun Heo wrote:
> ebf1ccff79c4 ("sched_ext: Fix ops.dequeue() semantics") moved the final
> ops_state store in scx_dispatch_enqueue() after the DSQ unlock so that the
> custody update and ops.dequeue() precede it. A task can thus be found on a
> DSQ while still SCX_OPSS_DISPATCHING.
> 
> The dequeue and core-sched pick paths wait for the state to clear in
> ops_dequeue() but the reenqueue paths don't. A reenqueue in that window runs
> ops.enqueue() and sets SCX_OPSS_QUEUED before the dispatch has completed.
> The dispatcher's final store then overwrites it with SCX_OPSS_NONE and
> finish_dispatch() drops every later dispatch of the task.
> 
> Wait for SCX_OPSS_DISPATCHING to clear before dequeueing a task for
> reenqueue, the same way ops_dequeue() does.
> 
> Fixes: ebf1ccff79c4 ("sched_ext: Fix ops.dequeue() semantics")
> Cc: stable@vger.kernel.org # v7.1+
> Signed-off-by: Tejun Heo <tj@kernel.org>

Thanks for fixing this, it makes sense to me.

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

-Andrea

> ---
>  kernel/sched/ext/ext.c      |   13 +++++++++++++
>  kernel/sched/ext/internal.h |    1 +
>  kernel/sched/ext/sub.c      |    1 +
>  3 files changed, 15 insertions(+)
> 
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -4404,6 +4404,17 @@ static bool local_task_should_reenq(stru
>  	return *reenq_flags & SCX_REENQ_ANY;
>  }
>  
> +/*
> + * The dispatcher stores the final ops_state after dropping the DSQ lock, so @p
> + * can be found on a DSQ while still %SCX_OPSS_DISPATCHING. Reenqueueing @p
> + * before that store lands would have it clobber the new %SCX_OPSS_QUEUED.
> + */
> +void scx_reenq_wait_dispatching(struct task_struct *p)
> +{
> +	if (unlikely(atomic_long_read_acquire(&p->scx.ops_state) == SCX_OPSS_DISPATCHING))
> +		wait_ops_state(p, SCX_OPSS_DISPATCHING);
> +}
> +
>  static u32 reenq_local(struct scx_sched *sch, struct rq *rq, u64 reenq_flags)
>  {
>  	LIST_HEAD(tasks);
> @@ -4447,6 +4458,7 @@ static u32 reenq_local(struct scx_sched
>  		if (!local_task_should_reenq(rq, p, &reenq_flags, &reason))
>  			continue;
>  
> +		scx_reenq_wait_dispatching(p);
>  		scx_dispatch_dequeue(rq, p);
>  
>  		if (WARN_ON_ONCE(p->scx.flags & SCX_TASK_REENQ_REASON_MASK))
> @@ -4570,6 +4582,7 @@ static void reenq_user(struct rq *rq, st
>  		}
>  
>  		/* @p is on @dsq, its rq and @dsq are locked */
> +		scx_reenq_wait_dispatching(p);
>  		dispatch_dequeue_locked(p, dsq);
>  		raw_spin_unlock(&dsq->lock);
>  
> --- a/kernel/sched/ext/internal.h
> +++ b/kernel/sched/ext/internal.h
> @@ -2078,6 +2078,7 @@ void scx_kick_cpu(struct scx_sched *sch,
>  u64 __scx_bpf_now(struct rq *rq);
>  void schedule_dsq_reenq(struct scx_sched *sch, struct scx_dispatch_q *dsq,
>  			u64 reenq_flags, struct rq *locked_rq);
> +void scx_reenq_wait_dispatching(struct task_struct *p);
>  int __scx_init_task(struct scx_sched *sch, struct task_struct *p,
>  		    struct cgroup *cgrp, bool fork);
>  void scx_enable_task(struct scx_sched *sch, struct task_struct *p);
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -801,6 +801,7 @@ void scx_reenq_reject(struct rq *rq)
>  		if (WARN_ON_ONCE(p->migration_pending))
>  			continue;
>  
> +		scx_reenq_wait_dispatching(p);
>  		scx_dispatch_dequeue(rq, p);
>  
>  		if (WARN_ON_ONCE(p->scx.flags & SCX_TASK_REENQ_REASON_MASK))

  reply	other threads:[~2026-09-15 20:54 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15 20:03 [PATCH sched_ext/for-7.3-fixes] sched_ext: Wait for SCX_OPSS_DISPATCHING before reenqueueing a task Tejun Heo
2026-09-15 20:54 ` Andrea Righi [this message]
2026-09-16 20:08 ` 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=aqmwiJ1StVe1EH4L@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=emil@etsalapatis.com \
    --cc=fangqiurong@kylinos.cn \
    --cc=linux-kernel@vger.kernel.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 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.