Re: [PATCH 13/34] sched_ext: Refactor task init/exit helpers

[Andrea Righi]

Hi Tejun,

On Tue, Feb 24, 2026 at 07:00:48PM -1000, Tejun Heo wrote:
> - Add the @sch parameter to scx_init_task() and drop @tg as it can be
>   obtained from @p. Separate out __scx_init_task() which does everything
>   except for the task state transition.
> 
> - Add the @sch parameter to scx_enable_task(). Separate out
>   __scx_enable_task() which does everything except for the task state
>   transition.
> 
> - Add the @sch parameter to scx_disable_task().
> 
> - Rename scx_exit_task() to scx_disable_and_exit_task() and separate out
>   __scx_disable_and_exit_task() which does everything except for the task
>   state transition.
> 
> While some task state transitions are relocated, no meaningful behavior
> changes are expected.
> 
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> ---
>  kernel/sched/ext.c | 67 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 33e9129a8073..5d13b9b93249 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -3107,9 +3107,9 @@ static void scx_set_task_state(struct task_struct *p, enum scx_task_state state)
>  	p->scx.flags |= state << SCX_TASK_STATE_SHIFT;
>  }
>  
> -static int scx_init_task(struct task_struct *p, struct task_group *tg, bool fork)
> +static int __scx_init_task(struct scx_sched *sch, struct task_struct *p, bool fork)
>  {
> -	struct scx_sched *sch = scx_root;
> +	struct task_group *tg = task_group(p);

Without CONFIG_EXT_GROUP_SCHED we get this:

In file included from kernel/sched/build_policy.c:62:
kernel/sched/ext.c: In function ‘__scx_init_task’:
kernel/sched/ext.c:3228:28: error: unused variable ‘tg’ [-Werror=unused-variable]
 3228 |         struct task_group *tg = task_group(p);
      |                            ^~
cc1: all warnings being treated as errors

Maybe wrap tg inside an #ifdef CONFIG_EXT_GROUP_SCHED or we can do
SCX_INIT_TASK_ARGS_CGROUP(task_group(p)) directly.

Thanks,
-Andrea

>  	int ret;
>  
>  	p->scx.disallow = false;
> @@ -3128,8 +3128,6 @@ static int scx_init_task(struct task_struct *p, struct task_group *tg, bool fork
>  		}
>  	}
>  
> -	scx_set_task_state(p, SCX_TASK_INIT);
> -
>  	if (p->scx.disallow) {
>  		if (unlikely(scx_parent(sch))) {
>  			scx_error(sch, "non-root ops.init_task() set task->scx.disallow for %s[%d]",
> @@ -3159,13 +3157,27 @@ static int scx_init_task(struct task_struct *p, struct task_group *tg, bool fork
>  		}
>  	}
>  
> -	p->scx.flags |= SCX_TASK_RESET_RUNNABLE_AT;
>  	return 0;
>  }
>  
> -static void scx_enable_task(struct task_struct *p)
> +static int scx_init_task(struct scx_sched *sch, struct task_struct *p, bool fork)
> +{
> +	int ret;
> +
> +	ret = __scx_init_task(sch, p, fork);
> +	if (!ret) {
> +		/*
> +		 * While @p's rq is not locked. @p is not visible to the rest of
> +		 * SCX yet and it's safe to update the flags and state.
> +		 */
> +		p->scx.flags |= SCX_TASK_RESET_RUNNABLE_AT;
> +		scx_set_task_state(p, SCX_TASK_INIT);
> +	}
> +	return ret;
> +}
> +
> +static void __scx_enable_task(struct scx_sched *sch, struct task_struct *p)
>  {
> -	struct scx_sched *sch = scx_root;
>  	struct rq *rq = task_rq(p);
>  	u32 weight;
>  
> @@ -3191,16 +3203,20 @@ static void scx_enable_task(struct task_struct *p)
>  
>  	if (SCX_HAS_OP(sch, enable))
>  		SCX_CALL_OP_TASK(sch, SCX_KF_REST, enable, rq, p);
> -	scx_set_task_state(p, SCX_TASK_ENABLED);
>  
>  	if (SCX_HAS_OP(sch, set_weight))
>  		SCX_CALL_OP_TASK(sch, SCX_KF_REST, set_weight, rq,
>  				 p, p->scx.weight);
>  }
>  
> -static void scx_disable_task(struct task_struct *p)
> +static void scx_enable_task(struct scx_sched *sch, struct task_struct *p)
> +{
> +	__scx_enable_task(sch, p);
> +	scx_set_task_state(p, SCX_TASK_ENABLED);
> +}
> +
> +static void scx_disable_task(struct scx_sched *sch, struct task_struct *p)
>  {
> -	struct scx_sched *sch = scx_root;
>  	struct rq *rq = task_rq(p);
>  
>  	lockdep_assert_rq_held(rq);
> @@ -3218,9 +3234,9 @@ static void scx_disable_task(struct task_struct *p)
>  	WARN_ON_ONCE(p->scx.flags & SCX_TASK_IN_CUSTODY);
>  }
>  
> -static void scx_exit_task(struct task_struct *p)
> +static void __scx_disable_and_exit_task(struct scx_sched *sch,
> +					struct task_struct *p)
>  {
> -	struct scx_sched *sch = scx_task_sched(p);
>  	struct scx_exit_task_args args = {
>  		.cancelled = false,
>  	};
> @@ -3237,7 +3253,7 @@ static void scx_exit_task(struct task_struct *p)
>  	case SCX_TASK_READY:
>  		break;
>  	case SCX_TASK_ENABLED:
> -		scx_disable_task(p);
> +		scx_disable_task(sch, p);
>  		break;
>  	default:
>  		WARN_ON_ONCE(true);
> @@ -3247,6 +3263,13 @@ static void scx_exit_task(struct task_struct *p)
>  	if (SCX_HAS_OP(sch, exit_task))
>  		SCX_CALL_OP_TASK(sch, SCX_KF_REST, exit_task, task_rq(p),
>  				 p, &args);
> +}
> +
> +static void scx_disable_and_exit_task(struct scx_sched *sch,
> +				      struct task_struct *p)
> +{
> +	__scx_disable_and_exit_task(sch, p);
> +
>  	scx_set_task_sched(p, NULL);
>  	scx_set_task_state(p, SCX_TASK_NONE);
>  }
> @@ -3282,7 +3305,7 @@ int scx_fork(struct task_struct *p, struct kernel_clone_args *kargs)
>  	percpu_rwsem_assert_held(&scx_fork_rwsem);
>  
>  	if (scx_init_task_enabled) {
> -		ret = scx_init_task(p, task_group(p), true);
> +		ret = scx_init_task(scx_root, p, true);
>  		if (!ret)
>  			scx_set_task_sched(p, scx_root);
>  		return ret;
> @@ -3306,7 +3329,7 @@ void scx_post_fork(struct task_struct *p)
>  			struct rq *rq;
>  
>  			rq = task_rq_lock(p, &rf);
> -			scx_enable_task(p);
> +			scx_enable_task(scx_task_sched(p), p);
>  			task_rq_unlock(rq, p, &rf);
>  		}
>  	}
> @@ -3326,7 +3349,7 @@ void scx_cancel_fork(struct task_struct *p)
>  
>  		rq = task_rq_lock(p, &rf);
>  		WARN_ON_ONCE(scx_get_task_state(p) >= SCX_TASK_READY);
> -		scx_exit_task(p);
> +		scx_disable_and_exit_task(scx_task_sched(p), p);
>  		task_rq_unlock(rq, p, &rf);
>  	}
>  
> @@ -3385,7 +3408,7 @@ void sched_ext_dead(struct task_struct *p)
>  		struct rq *rq;
>  
>  		rq = task_rq_lock(p, &rf);
> -		scx_exit_task(p);
> +		scx_disable_and_exit_task(scx_task_sched(p), p);
>  		task_rq_unlock(rq, p, &rf);
>  	}
>  }
> @@ -3417,7 +3440,7 @@ static void switching_to_scx(struct rq *rq, struct task_struct *p)
>  	if (task_dead_and_done(p))
>  		return;
>  
> -	scx_enable_task(p);
> +	scx_enable_task(sch, p);
>  
>  	/*
>  	 * set_cpus_allowed_scx() is not called while @p is associated with a
> @@ -3433,7 +3456,7 @@ static void switched_from_scx(struct rq *rq, struct task_struct *p)
>  	if (task_dead_and_done(p))
>  		return;
>  
> -	scx_disable_task(p);
> +	scx_disable_task(scx_task_sched(p), p);
>  }
>  
>  static void wakeup_preempt_scx(struct rq *rq, struct task_struct *p, int wake_flags) {}
> @@ -4662,7 +4685,7 @@ static void scx_root_disable(struct scx_sched *sch)
>  
>  	/*
>  	 * Shut down cgroup support before tasks so that the cgroup attach path
> -	 * doesn't race against scx_exit_task().
> +	 * doesn't race against scx_disable_and_exit_task().
>  	 */
>  	scx_cgroup_lock();
>  	scx_cgroup_exit(sch);
> @@ -4691,7 +4714,7 @@ static void scx_root_disable(struct scx_sched *sch)
>  			p->sched_class = new_class;
>  		}
>  
> -		scx_exit_task(p);
> +		scx_disable_and_exit_task(scx_task_sched(p), p);
>  	}
>  	scx_task_iter_stop(&sti);
>  
> @@ -5567,7 +5590,7 @@ static s32 scx_root_enable(struct sched_ext_ops *ops, struct bpf_link *link)
>  
>  		scx_task_iter_unlock(&sti);
>  
> -		ret = scx_init_task(p, task_group(p), false);
> +		ret = scx_init_task(sch, p, false);
>  		if (ret) {
>  			put_task_struct(p);
>  			scx_task_iter_stop(&sti);
> -- 
> 2.53.0
>