[PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path

[PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path

[Andrea Righi]

consume_remote_task() drops this_rq before unlinking a remote task from
its DSQ and locking src_rq. When called from ops.dispatch(),
scx_locked_rq() keeps pointing to this_rq across the lock dance. The
subsequent switch_rq_lock(src_rq, this_rq) cannot update the tracking
because its guard does not match, leaving the tracking stale while
src_rq is held.

Keep this_rq locked until the task has been unlinked and the DSQ lock
released, then use switch_rq_lock() to switch directly to src_rq. Use
the same helper to restore this_rq after losing the dequeue race. The
successful path already switches back through
move_remote_task_to_local_dsq(), so scx_locked_rq() now follows the
actually held rq throughout the consume path.

Suggested-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/all/455e701bca66bdecde530d225f4dba0a@kernel.org
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
Based on sched_ext/for-7.3 (daf8e166ba59d) with sched_ext/for-7.2-fixes
(cfe950d79f524) assumed merged.

 kernel/sched/ext/ext.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 2ad768a7689f1..51c24df7be5d2 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -2203,13 +2203,14 @@ static bool task_can_run_on_remote_rq(struct scx_sched *sch,
 }
 
 /**
- * unlink_dsq_and_lock_src_rq() - Unlink task from its DSQ and lock its task_rq
+ * unlink_dsq_and_switch_rq_lock() - Unlink task and switch to its rq lock
  * @p: target task
  * @dsq: locked DSQ @p is currently on
+ * @locked_rq: currently locked rq
  * @src_rq: rq @p is currently on, stable with @dsq locked
  *
- * Called with @dsq locked but no rq's locked. We want to move @p to a different
- * DSQ, including any local DSQ, but are not locking @src_rq. Locking @src_rq is
+ * Called with @dsq and @locked_rq locked. We want to move @p to a different DSQ,
+ * including any local DSQ, but are not locking @src_rq. Locking @src_rq is
  * required when transferring into a local DSQ. Even when transferring into a
  * non-local DSQ, it's better to use the same mechanism to protect against
  * dequeues and maintain the invariant that @p->scx.dsq can only change while
@@ -2231,20 +2232,22 @@ static bool task_can_run_on_remote_rq(struct scx_sched *sch,
  * On return, @dsq is unlocked and @src_rq is locked. Returns %true if @p is
  * still valid. %false if lost to dequeue.
  */
-static bool unlink_dsq_and_lock_src_rq(struct task_struct *p,
-				       struct scx_dispatch_q *dsq,
-				       struct rq *src_rq)
+static bool unlink_dsq_and_switch_rq_lock(struct task_struct *p,
+					  struct scx_dispatch_q *dsq,
+					  struct rq *locked_rq,
+					  struct rq *src_rq)
 {
 	s32 cpu = raw_smp_processor_id();
 
 	lockdep_assert_held(&dsq->lock);
+	lockdep_assert_rq_held(locked_rq);
 
 	WARN_ON_ONCE(p->scx.holding_cpu >= 0);
 	task_unlink_from_dsq(p, dsq);
 	p->scx.holding_cpu = cpu;
 
 	raw_spin_unlock(&dsq->lock);
-	raw_spin_rq_lock(src_rq);
+	switch_rq_lock(locked_rq, src_rq);
 
 	/* task_rq couldn't have changed if we're still the holding cpu */
 	return likely(p->scx.holding_cpu == cpu) &&
@@ -2255,14 +2258,11 @@ static bool consume_remote_task(struct rq *this_rq,
 				struct task_struct *p, u64 enq_flags,
 				struct scx_dispatch_q *dsq, struct rq *src_rq)
 {
-	raw_spin_rq_unlock(this_rq);
-
-	if (unlink_dsq_and_lock_src_rq(p, dsq, src_rq)) {
+	if (unlink_dsq_and_switch_rq_lock(p, dsq, this_rq, src_rq)) {
 		move_remote_task_to_local_dsq(p, enq_flags, src_rq, this_rq);
 		return true;
 	} else {
-		raw_spin_rq_unlock(src_rq);
-		raw_spin_rq_lock(this_rq);
+		switch_rq_lock(src_rq, this_rq);
 		return false;
 	}
 }
@@ -2435,7 +2435,7 @@ static void dispatch_to_local_dsq(struct scx_sched *sch, struct rq *rq,
 	 * As DISPATCHING guarantees that @p is wholly ours, we can pretend that
 	 * we're moving from a DSQ and use the same mechanism - mark the task
 	 * under transfer with holding_cpu, release DISPATCHING and then follow
-	 * the same protocol. See unlink_dsq_and_lock_src_rq().
+	 * the same protocol. See unlink_dsq_and_switch_rq_lock().
 	 */
 	p->scx.holding_cpu = raw_smp_processor_id();
 
-- 
2.55.0

Re: [PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `scx_locked_rq_state` tracking will be left permanently stale on the successful task migration path, directly contradicting the commit message's claim.
- [Low] The patch introduces calls to an undefined function or macro `switch_rq_lock()`, causing a compilation error.
--

--- Patch [1]: [PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path ---
commit 0f65257c6efec221d0687e563233c82e3c63e9c8
Author: Andrea Righi <arighi@nvidia.com>

sched_ext: Keep rq tracking accurate in the consume path

This commit attempts to fix a stale BPF runqueue lock tracking issue in
consume_remote_task() by introducing a new switch_rq_lock() helper. It
aims to ensure that scx_locked_rq() correctly follows the lock transitions
between this_rq and src_rq during remote task consumption.

Link: https://lore.kernel.org/all/455e701bca66bdecde530d225f4dba0a@kernel.org

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 691d53fe0f648..9644c37c87c58 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -2386,20 +2387,22 @@ static bool task_can_run_on_remote_rq(struct scx_sched *sch,
>   * On return, @dsq is unlocked and @src_rq is locked. Returns %true if @p is
>   * still valid. %false if lost to dequeue.
>   */
> -static bool unlink_dsq_and_lock_src_rq(struct task_struct *p,
> -				       struct scx_dispatch_q *dsq,
> -				       struct rq *src_rq)
> +static bool unlink_dsq_and_switch_rq_lock(struct task_struct *p,
> +					  struct scx_dispatch_q *dsq,
> +					  struct rq *locked_rq,
> +					  struct rq *src_rq)
>  {
>  	s32 cpu = raw_smp_processor_id();
>  
>  	lockdep_assert_held(&dsq->lock);
> +	lockdep_assert_rq_held(locked_rq);
>  
>  	WARN_ON_ONCE(p->scx.holding_cpu >= 0);
>  	task_unlink_from_dsq(p, dsq);
>  	p->scx.holding_cpu = cpu;
>  
>  	raw_spin_unlock(&dsq->lock);
> -	raw_spin_rq_lock(src_rq);
> +	switch_rq_lock(locked_rq, src_rq);

[Severity: Low]
Are we missing the definition of switch_rq_lock() in this patch?

Compiling this throws an implicit declaration error since switch_rq_lock()
doesn't seem to exist.

[ ... ]

> @@ -2410,14 +2413,11 @@ static bool consume_remote_task(struct rq *this_rq,
>  				struct task_struct *p, u64 enq_flags,
>  				struct scx_dispatch_q *dsq, struct rq *src_rq)
>  {
> -	raw_spin_rq_unlock(this_rq);
> -
> -	if (unlink_dsq_and_lock_src_rq(p, dsq, src_rq)) {
> +	if (unlink_dsq_and_switch_rq_lock(p, dsq, this_rq, src_rq)) {
>  		move_remote_task_to_local_dsq(p, enq_flags, src_rq, this_rq);

[Severity: High]
Does this leave the lock tracking state permanently stale on the success path?

In move_remote_task_to_local_dsq(), the physical lock is swapped from src_rq
to dst_rq (which is this_rq) using raw spinlocks:

kernel/sched/ext/ext.c:move_remote_task_to_local_dsq()
    ...
    raw_spin_rq_unlock(src_rq);
    raw_spin_rq_lock(dst_rq);
    ...

If unlink_dsq_and_switch_rq_lock() updates the tracking state to src_rq,
but move_remote_task_to_local_dsq() doesn't update it back to this_rq, won't
scx_locked_rq() falsely claim src_rq is still held when returning to the BPF
program?

>  		return true;
>  	} else {
> -		raw_spin_rq_unlock(src_rq);
> -		raw_spin_rq_lock(this_rq);
> +		switch_rq_lock(src_rq, this_rq);
>  		return false;
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709051708.306636-1-arighi@nvidia.com?part=1

Re: [PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path

[Andrea Righi]

On Thu, Jul 09, 2026 at 05:33:48AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The `scx_locked_rq_state` tracking will be left permanently stale on the successful task migration path, directly contradicting the commit message's claim.
> - [Low] The patch introduces calls to an undefined function or macro `switch_rq_lock()`, causing a compilation error.

Both false positives. See below.

> --
> 
> --- Patch [1]: [PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path ---
> commit 0f65257c6efec221d0687e563233c82e3c63e9c8
> Author: Andrea Righi <arighi@nvidia.com>
> 
> sched_ext: Keep rq tracking accurate in the consume path
> 
> This commit attempts to fix a stale BPF runqueue lock tracking issue in
> consume_remote_task() by introducing a new switch_rq_lock() helper. It
> aims to ensure that scx_locked_rq() correctly follows the lock transitions
> between this_rq and src_rq during remote task consumption.
> 
> Link: https://lore.kernel.org/all/455e701bca66bdecde530d225f4dba0a@kernel.org
> 
> > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > index 691d53fe0f648..9644c37c87c58 100644
> > --- a/kernel/sched/ext/ext.c
> > +++ b/kernel/sched/ext/ext.c
> 
> [ ... ]
> 
> > @@ -2386,20 +2387,22 @@ static bool task_can_run_on_remote_rq(struct scx_sched *sch,
> >   * On return, @dsq is unlocked and @src_rq is locked. Returns %true if @p is
> >   * still valid. %false if lost to dequeue.
> >   */
> > -static bool unlink_dsq_and_lock_src_rq(struct task_struct *p,
> > -				       struct scx_dispatch_q *dsq,
> > -				       struct rq *src_rq)
> > +static bool unlink_dsq_and_switch_rq_lock(struct task_struct *p,
> > +					  struct scx_dispatch_q *dsq,
> > +					  struct rq *locked_rq,
> > +					  struct rq *src_rq)
> >  {
> >  	s32 cpu = raw_smp_processor_id();
> >  
> >  	lockdep_assert_held(&dsq->lock);
> > +	lockdep_assert_rq_held(locked_rq);
> >  
> >  	WARN_ON_ONCE(p->scx.holding_cpu >= 0);
> >  	task_unlink_from_dsq(p, dsq);
> >  	p->scx.holding_cpu = cpu;
> >  
> >  	raw_spin_unlock(&dsq->lock);
> > -	raw_spin_rq_lock(src_rq);
> > +	switch_rq_lock(locked_rq, src_rq);
> 
> [Severity: Low]
> Are we missing the definition of switch_rq_lock() in this patch?
> 
> Compiling this throws an implicit declaration error since switch_rq_lock()
> doesn't seem to exist.

This was triggered because the current sched_ext/for-7.3 is missing the
for-7.2-fixes merge.

As mentioned in the patch notes: "Based on sched_ext/for-7.3 (daf8e166ba59d)
with sched_ext/for-7.2-fixes (cfe950d79f524) assumed merged".

> 
> [ ... ]
> 
> > @@ -2410,14 +2413,11 @@ static bool consume_remote_task(struct rq *this_rq,
> >  				struct task_struct *p, u64 enq_flags,
> >  				struct scx_dispatch_q *dsq, struct rq *src_rq)
> >  {
> > -	raw_spin_rq_unlock(this_rq);
> > -
> > -	if (unlink_dsq_and_lock_src_rq(p, dsq, src_rq)) {
> > +	if (unlink_dsq_and_switch_rq_lock(p, dsq, this_rq, src_rq)) {
> >  		move_remote_task_to_local_dsq(p, enq_flags, src_rq, this_rq);
> 
> [Severity: High]
> Does this leave the lock tracking state permanently stale on the success path?
> 
> In move_remote_task_to_local_dsq(), the physical lock is swapped from src_rq
> to dst_rq (which is this_rq) using raw spinlocks:
> 
> kernel/sched/ext/ext.c:move_remote_task_to_local_dsq()
>     ...
>     raw_spin_rq_unlock(src_rq);
>     raw_spin_rq_lock(dst_rq);
>     ...
> 
> If unlink_dsq_and_switch_rq_lock() updates the tracking state to src_rq,
> but move_remote_task_to_local_dsq() doesn't update it back to this_rq, won't
> scx_locked_rq() falsely claim src_rq is still held when returning to the BPF
> program?

False positive due to the reason above.

Thanks,
-Andrea

> 
> >  		return true;
> >  	} else {
> > -		raw_spin_rq_unlock(src_rq);
> > -		raw_spin_rq_lock(this_rq);
> > +		switch_rq_lock(src_rq, this_rq);
> >  		return false;
> >  	}
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709051708.306636-1-arighi@nvidia.com?part=1

Re: [PATCH sched_ext/for-7.3] sched_ext: Keep rq tracking accurate in the consume path

[Tejun Heo]

Hello,

Applied to sched_ext/for-7.3.

Thanks.

--
tejun