public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, sched-ext@lists.linux.dev,
	void@manifault.com, changwoo@igalia.com, emil@etsalapatis.com,
	hannes@cmpxchg.org, mkoutny@suse.com, cgroups@vger.kernel.org
Subject: Re: [PATCH 13/34] sched_ext: Refactor task init/exit helpers
Date: Fri, 27 Feb 2026 07:55:49 +0100	[thread overview]
Message-ID: <aaE_9YPu3T-pMXfO@gpd4> (raw)
In-Reply-To: <20260225050109.1070059-14-tj@kernel.org>

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@kernel.org>
> ---
>  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
> 

  reply	other threads:[~2026-02-27  6:56 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25  5:00 [PATCHSET v2 sched_ext/for-7.1] sched_ext: Implement cgroup sub-scheduler support Tejun Heo
2026-02-25  5:00 ` [PATCH 01/34] sched_ext: Implement cgroup subtree iteration for scx_task_iter Tejun Heo
2026-02-25  5:00 ` [PATCH 02/34] sched_ext: Add @kargs to scx_fork() Tejun Heo
2026-02-25  5:00 ` [PATCH 03/34] sched/core: Swap the order between sched_post_fork() and cgroup_post_fork() Tejun Heo
2026-02-25  5:00 ` [PATCH 04/34] cgroup: Expose some cgroup helpers Tejun Heo
2026-02-25  5:00 ` [PATCH 05/34] sched_ext: Update p->scx.disallow warning in scx_init_task() Tejun Heo
2026-02-25  5:00 ` [PATCH 06/34] sched_ext: Reorganize enable/disable path for multi-scheduler support Tejun Heo
2026-02-25  5:00 ` [PATCH 07/34] sched_ext: Introduce cgroup sub-sched support Tejun Heo
2026-02-26 15:37   ` Andrea Righi
2026-02-27 20:14     ` Tejun Heo
2026-02-26 15:52   ` Andrea Righi
2026-02-27 19:51     ` Tejun Heo
2026-02-27 20:04       ` Tejun Heo
2026-02-25  5:00 ` [PATCH 08/34] sched_ext: Introduce scx_task_sched[_rcu]() Tejun Heo
2026-02-25  5:00 ` [PATCH 09/34] sched_ext: Introduce scx_prog_sched() Tejun Heo
2026-02-25  5:00 ` [PATCH 10/34] sched_ext: Enforce scheduling authority in dispatch and select_cpu operations Tejun Heo
2026-02-25  5:00 ` [PATCH 11/34] sched_ext: Enforce scheduler ownership when updating slice and dsq_vtime Tejun Heo
2026-02-26 15:13   ` Andrea Righi
2026-02-27 22:25     ` Tejun Heo
2026-02-27 23:50       ` Andrea Righi
2026-02-25  5:00 ` [PATCH 12/34] sched_ext: scx_dsq_move() should validate the task belongs to the right scheduler Tejun Heo
2026-02-25  5:00 ` [PATCH 13/34] sched_ext: Refactor task init/exit helpers Tejun Heo
2026-02-27  6:55   ` Andrea Righi [this message]
2026-02-27 19:50     ` Tejun Heo
2026-02-25  5:00 ` [PATCH 14/34] sched_ext: Make scx_prio_less() handle multiple schedulers Tejun Heo
2026-02-25  5:00 ` [PATCH 15/34] sched_ext: Move default slice to per-scheduler field Tejun Heo
2026-02-25  5:00 ` [PATCH 16/34] sched_ext: Move aborting flag " Tejun Heo
2026-02-25  5:00 ` [PATCH 17/34] sched_ext: Move bypass_dsq into scx_sched_pcpu Tejun Heo
2026-02-25  5:00 ` [PATCH 18/34] sched_ext: Move bypass state into scx_sched Tejun Heo
2026-02-25  5:00 ` [PATCH 19/34] sched_ext: Prepare bypass mode for hierarchical operation Tejun Heo
2026-02-25  5:00 ` [PATCH 20/34] sched_ext: Factor out scx_dispatch_sched() Tejun Heo
2026-02-25  5:00 ` [PATCH 21/34] sched_ext: When calling ops.dispatch() @prev must be on the same scx_sched Tejun Heo
2026-02-25  5:00 ` [PATCH 22/34] sched_ext: Separate bypass dispatch enabling from bypass depth tracking Tejun Heo
2026-02-25  5:00 ` [PATCH 23/34] sched_ext: Implement hierarchical bypass mode Tejun Heo
2026-02-25  5:00 ` [PATCH 24/34] sched_ext: Dispatch from all scx_sched instances Tejun Heo
2026-02-25  5:01 ` [PATCH 25/34] sched_ext: Move scx_dsp_ctx and scx_dsp_max_batch into scx_sched Tejun Heo
2026-02-25  5:01 ` [PATCH 26/34] sched_ext: Make watchdog sub-sched aware Tejun Heo
2026-02-25  5:01 ` [PATCH 27/34] sched_ext: Convert scx_dump_state() spinlock to raw spinlock Tejun Heo
2026-02-25  5:01 ` [PATCH 28/34] sched_ext: Support dumping multiple schedulers and add scheduler identification Tejun Heo
2026-02-25  5:01 ` [PATCH 29/34] sched_ext: Implement cgroup sub-sched enabling and disabling Tejun Heo
2026-02-25  5:01 ` [PATCH 30/34] sched_ext: Add scx_sched back pointer to scx_sched_pcpu Tejun Heo
2026-02-25  5:01 ` [PATCH 31/34] sched_ext: Make scx_bpf_reenqueue_local() sub-sched aware Tejun Heo
2026-02-25  5:01 ` [PATCH 32/34] sched_ext: Factor out scx_link_sched() and scx_unlink_sched() Tejun Heo
2026-02-25  5:01 ` [PATCH 33/34] sched_ext: Add rhashtable lookup for sub-schedulers Tejun Heo
2026-02-25  5:01 ` [PATCH 34/34] sched_ext: Add basic building blocks for nested sub-scheduler dispatching Tejun Heo
2026-02-25  5:14 ` [PATCHSET v2 sched_ext/for-7.1] sched_ext: Implement cgroup sub-scheduler support Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2026-03-04 22:00 [PATCHSET v3 " Tejun Heo
2026-03-04 22:00 ` [PATCH 13/34] sched_ext: Refactor task init/exit helpers Tejun Heo
2026-02-25  5:01 [PATCHSET v2 sched_ext/for-7.1] sched_ext: Implement cgroup sub-scheduler support Tejun Heo
2026-02-25  5:01 ` [PATCH 13/34] sched_ext: Refactor task init/exit helpers Tejun Heo
2026-01-21 23:11 [PATCHSET v1 sched_ext/for-6.20] sched_ext: Implement cgroup sub-scheduler support Tejun Heo
2026-01-21 23:11 ` [PATCH 13/34] sched_ext: Refactor task init/exit helpers 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=aaE_9YPu3T-pMXfO@gpd4 \
    --to=arighi@nvidia.com \
    --cc=cgroups@vger.kernel.org \
    --cc=changwoo@igalia.com \
    --cc=emil@etsalapatis.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkoutny@suse.com \
    --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