Re: [PATCH v4 sched_ext/for-7.3 34/40] sched_ext: Authorize remote-move inserts against the placing scheduler

Andrea Righi <[email protected]>
Newsgroups dev.linux.lists.sched-ext,org.kernel.vger.linux-kernel
Message-ID <ak__5beR_STJ5OsU@gpd4>
Hi Tejun,

On Wed, Jul 08, 2026 at 11:24:23AM -1000, Tejun Heo wrote:
> scx_local_or_reject_dsq() authorizes a local-DSQ insert against the caps
> of the scheduler doing the insert. On the consume/dispatch paths that is
> the scheduler running balance_one(), passed down through
> scx_consume_dispatch_q() and move_local_task_to_local_dsq(), so the check
> is correct.
> 
> The remote-move path loses it. move_remote_task_to_local_dsq()
> re-activates @p on the destination rq through enqueue_task_scx(), which
> reconstructs the scheduler from the task, i.e. @p's owner. When an
> ancestor places a descendant's task - e.g. draining a bypassed
> sub-scheduler - the owner is a sub-scheduler of the placer, so
> authorizing against the owner checks a narrower cap set and can
> spuriously reject a task the placer is entitled to run.
> 
> Carry the placing scheduler across the activate_task() boundary the same
> way enq_flags already are, via a per-rq field set only for the duration
> of the re-activation, and have scx_local_or_reject_dsq() authorize
> against it. The placer's caps are a superset of the owner's, so this
> admits what the placer may run and keeps rejecting what it may not.
> 
> Signed-off-by: Tejun Heo <[email protected]>
> ---
>  kernel/sched/ext/ext.c | 27 +++++++++++++++------------
>  kernel/sched/ext/sub.c |  9 +++++++--
>  kernel/sched/sched.h   |  3 ++-
>  3 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index e15297c29617..896f8f9eea6f 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -2256,7 +2256,8 @@ static void move_local_task_to_local_dsq(struct scx_sched *sch,
>   *
>   * Move @p which is currently on @src_rq to @dst_rq's local DSQ.
>   */
> -static void move_remote_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
> +static void move_remote_task_to_local_dsq(struct scx_sched *sch,
> +					  struct task_struct *p, u64 enq_flags,
>  					  struct rq *src_rq, struct rq *dst_rq)

nit: since the function gained the new @sch parameter, maybe add
"@sch: scx_sched being operated on" to the kernel-doc block.

Thanks,
-Andrea

>  {
>  	lockdep_assert_rq_held(src_rq);
> @@ -2273,15 +2274,19 @@ static void move_remote_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
>  	raw_spin_rq_lock(dst_rq);
>  
>  	/*
> -	 * We want to pass scx-specific enq_flags but activate_task() will
> -	 * truncate the upper 32 bit. As we own @rq, we can pass them through
> -	 * @rq->scx.remote_activate_enq_flags instead.
> +	 * activate_task() below truncates enq_flags to 32 bits and re-derives
> +	 * @p's owner, dropping our scx flags and the placing @sch. We own @rq,
> +	 * so stash both across the call. The enqueue reads them back, keeping
> +	 * the scx flags and checking caps against the placer, not the owner.
>  	 */
>  	WARN_ON_ONCE(!cpumask_test_cpu(cpu_of(dst_rq), p->cpus_ptr));
> -	WARN_ON_ONCE(dst_rq->scx.remote_activate_enq_flags);
> +	WARN_ON_ONCE(dst_rq->scx.remote_activate_enq_flags ||
> +		     dst_rq->scx.remote_activate_sch);
>  	dst_rq->scx.remote_activate_enq_flags = enq_flags;
> +	dst_rq->scx.remote_activate_sch = sch;
>  	activate_task(dst_rq, p, 0);
>  	dst_rq->scx.remote_activate_enq_flags = 0;
> +	dst_rq->scx.remote_activate_sch = NULL;
>  }
>  
>  /*
> @@ -2410,14 +2415,14 @@ static bool unlink_dsq_and_lock_src_rq(struct task_struct *p,
>  		!WARN_ON_ONCE(src_rq != task_rq(p));
>  }
>  
> -static bool consume_remote_task(struct rq *this_rq,
> +static bool consume_remote_task(struct scx_sched *sch, 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)) {
> -		move_remote_task_to_local_dsq(p, enq_flags, src_rq, this_rq);
> +		move_remote_task_to_local_dsq(sch, p, enq_flags, src_rq, this_rq);
>  		return true;
>  	} else {
>  		raw_spin_rq_unlock(src_rq);
> @@ -2479,8 +2484,7 @@ static struct rq *move_task_between_dsqs(struct scx_sched *sch,
>  			raw_spin_unlock(&src_dsq->lock);
>  		} else {
>  			raw_spin_unlock(&src_dsq->lock);
> -			move_remote_task_to_local_dsq(p, enq_flags,
> -						      src_rq, dst_rq);
> +			move_remote_task_to_local_dsq(sch, p, enq_flags, src_rq, dst_rq);
>  		}
>  	} else {
>  		/*
> @@ -2533,7 +2537,7 @@ bool scx_consume_dispatch_q(struct scx_sched *sch, struct rq *rq,
>  		}
>  
>  		if (task_can_run_on_remote_rq(sch, p, rq, false)) {
> -			if (likely(consume_remote_task(rq, p, enq_flags, dsq, task_rq)))
> +			if (likely(consume_remote_task(sch, rq, p, enq_flags, dsq, task_rq)))
>  				return true;
>  			goto retry;
>  		}
> @@ -2627,8 +2631,7 @@ static void dispatch_to_local_dsq(struct scx_sched *sch, struct rq *rq,
>  			scx_dispatch_enqueue(sch, src_rq, find_global_dsq(sch, task_cpu(p)),
>  					     p, enq_flags | SCX_ENQ_GDSQ_FALLBACK);
>  		} else {
> -			move_remote_task_to_local_dsq(p, enq_flags,
> -						      src_rq, dst_rq);
> +			move_remote_task_to_local_dsq(sch, p, enq_flags, src_rq, dst_rq);
>  			/* task has been moved to dst_rq, which is now locked */
>  			locked_rq = dst_rq;
>  		}
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 9a1eac886928..f66e97f08ce6 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -237,12 +237,17 @@ struct scx_dispatch_q *scx_local_or_reject_dsq(struct scx_sched *sch, struct rq
>  					       struct task_struct *p, u64 *enq_flags)
>  {
>  	s32 cid = __scx_cpu_to_cid(cpu_of(rq));
> +	struct scx_sched *asch = rq->scx.remote_activate_sch ?: sch;
>  	u64 needed = scx_caps_for_enq(*enq_flags);
>  	u64 missing;
>  
> +	/*
> +	 * On a remote activation the scheduling sched (@asch) differs from
> +	 * @p's owner (@sch). Check caps against the scheduling sched.
> +	 */
>  	if (*enq_flags & SCX_ENQ_PREEMPT)
> -		needed |= scx_caps_for_preempt(sch, rq);
> -	missing = scx_missing_caps(sch, cpu_of(rq), needed);
> +		needed |= scx_caps_for_preempt(asch, rq);
> +	missing = scx_missing_caps(asch, cpu_of(rq), needed);
>  
>  	/* requirements met */
>  	if (likely(!missing))
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index c0a4699a6c0a..cdd21c814008 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -800,8 +800,9 @@ struct scx_rq {
>  	struct list_head	runnable_list;		/* runnable tasks on this rq */
>  	struct list_head	ddsp_deferred_locals;	/* deferred ddsps from enq */
>  	unsigned long		ops_qseq;
> -	/* see move_remote_task_to_local_dsq() */
> +	/* both stashed across the activate_task() in move_remote_task_to_local_dsq() */
>  	u64			remote_activate_enq_flags;
> +	struct scx_sched	*remote_activate_sch;
>  	u32			nr_running;
>  	u32			cpuperf_target;		/* [0, SCHED_CAPACITY_SCALE] */
>  	bool			in_select_cpu;
> -- 
> 2.54.0
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.