[PATCHSET sched_ext/for-6.11] sched_ext: Clean up kernel/sched/ext.h

[PATCHSET sched_ext/for-6.11] sched_ext: Clean up kernel/sched/ext.h

[Tejun Heo]

Hello,

While sched_ext was out of tree, kernel/sched/ext.h contained declarations
and definitions which aren't ideal but are helpful for forward porting. This
patchset cleans them up.

- for_balance_class_range() is removed and instead open coded in
  put_prev_task_balance().

- Some declarations and definitions in kernel/sched/ext.h are moved to
  kernel/sched/sched.h.

This patchset contains the following three patches:

  0001-sched_ext-Minor-cleanups-in-kernel-sched-ext.h.patch
  0002-sched-sched_ext-Open-code-for_balance_class_range.patch
  0003-sched-sched_ext-Move-some-declarations-from-kernel-s.patch

and is also available in the following git branch:

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext.git scx-header-cleanups

diffstat follows. Thanks.

 kernel/sched/core.c  |   14 +++++++++++++-
 kernel/sched/ext.c   |    2 +-
 kernel/sched/ext.h   |   54 ++----------------------------------------------------
 kernel/sched/sched.h |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 75 insertions(+), 60 deletions(-)

--
tejun

[PATCH 1/3] sched_ext: Minor cleanups in kernel/sched/ext.h

[Tejun Heo]

- scx_ops_cpu_preempt is only used in kernel/sched/ext.c and doesn't need to
  be global. Make it static.

- Relocate task_on_scx() so that the inline functions are located together.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>
---
 kernel/sched/ext.c |  2 +-
 kernel/sched/ext.h | 12 +++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 28f7a4266fde..d4c645f9e031 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -771,7 +771,7 @@ static bool scx_warned_zero_slice;
 
 static DEFINE_STATIC_KEY_FALSE(scx_ops_enq_last);
 static DEFINE_STATIC_KEY_FALSE(scx_ops_enq_exiting);
-DEFINE_STATIC_KEY_FALSE(scx_ops_cpu_preempt);
+static DEFINE_STATIC_KEY_FALSE(scx_ops_cpu_preempt);
 static DEFINE_STATIC_KEY_FALSE(scx_builtin_idle_enabled);
 
 struct static_key_false scx_has_op[SCX_OPI_END] =
diff --git a/kernel/sched/ext.h b/kernel/sched/ext.h
index 0a7b9a34b18f..229007693504 100644
--- a/kernel/sched/ext.h
+++ b/kernel/sched/ext.h
@@ -26,13 +26,6 @@ DECLARE_STATIC_KEY_FALSE(__scx_switched_all);
 #define scx_enabled()		static_branch_unlikely(&__scx_ops_enabled)
 #define scx_switched_all()	static_branch_unlikely(&__scx_switched_all)
 
-DECLARE_STATIC_KEY_FALSE(scx_ops_cpu_preempt);
-
-static inline bool task_on_scx(const struct task_struct *p)
-{
-	return scx_enabled() && p->sched_class == &ext_sched_class;
-}
-
 void scx_tick(struct rq *rq);
 void init_scx_entity(struct sched_ext_entity *scx);
 void scx_pre_fork(struct task_struct *p);
@@ -54,6 +47,11 @@ static inline u32 scx_cpuperf_target(s32 cpu)
 		return 0;
 }
 
+static inline bool task_on_scx(const struct task_struct *p)
+{
+	return scx_enabled() && p->sched_class == &ext_sched_class;
+}
+
 static inline const struct sched_class *next_active_class(const struct sched_class *class)
 {
 	class++;
-- 
2.45.2

Re: [PATCH 1/3] sched_ext: Minor cleanups in kernel/sched/ext.h

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On Sat, Jun 22, 2024 at 03:50:20PM -1000, Tejun Heo wrote:
> - scx_ops_cpu_preempt is only used in kernel/sched/ext.c and doesn't need to
>   be global. Make it static.
> 
> - Relocate task_on_scx() so that the inline functions are located together.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: David Vernet <void@manifault.com>

Acked-by: David Vernet <void@manifault.com>

> ---
>  kernel/sched/ext.c |  2 +-
>  kernel/sched/ext.h | 12 +++++-------
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 28f7a4266fde..d4c645f9e031 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -771,7 +771,7 @@ static bool scx_warned_zero_slice;
>  
>  static DEFINE_STATIC_KEY_FALSE(scx_ops_enq_last);
>  static DEFINE_STATIC_KEY_FALSE(scx_ops_enq_exiting);
> -DEFINE_STATIC_KEY_FALSE(scx_ops_cpu_preempt);
> +static DEFINE_STATIC_KEY_FALSE(scx_ops_cpu_preempt);
>  static DEFINE_STATIC_KEY_FALSE(scx_builtin_idle_enabled);
>  
>  struct static_key_false scx_has_op[SCX_OPI_END] =
> diff --git a/kernel/sched/ext.h b/kernel/sched/ext.h
> index 0a7b9a34b18f..229007693504 100644
> --- a/kernel/sched/ext.h
> +++ b/kernel/sched/ext.h
> @@ -26,13 +26,6 @@ DECLARE_STATIC_KEY_FALSE(__scx_switched_all);
>  #define scx_enabled()		static_branch_unlikely(&__scx_ops_enabled)
>  #define scx_switched_all()	static_branch_unlikely(&__scx_switched_all)
>  
> -DECLARE_STATIC_KEY_FALSE(scx_ops_cpu_preempt);
> -
> -static inline bool task_on_scx(const struct task_struct *p)
> -{
> -	return scx_enabled() && p->sched_class == &ext_sched_class;
> -}
> -
>  void scx_tick(struct rq *rq);
>  void init_scx_entity(struct sched_ext_entity *scx);
>  void scx_pre_fork(struct task_struct *p);
> @@ -54,6 +47,11 @@ static inline u32 scx_cpuperf_target(s32 cpu)
>  		return 0;
>  }
>  
> +static inline bool task_on_scx(const struct task_struct *p)
> +{
> +	return scx_enabled() && p->sched_class == &ext_sched_class;
> +}
> +
>  static inline const struct sched_class *next_active_class(const struct sched_class *class)
>  {
>  	class++;
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/3] sched, sched_ext: Open code for_balance_class_range()

[Tejun Heo]

For flexibility, sched_ext allows the BPF scheduler to select the CPU to
execute a task on at dispatch time so that e.g. a queue can be shared across
multiple CPUs. To enable this, the dispatch path is executed from balance()
so that a dispatched task can be hot-migrated to its target CPU. This means
that sched_ext needs its balance() method invoked before every
pick_next_task() even when the CPU is waking up from SCHED_IDLE.

for_balance_class_range() defined in kernel/sched/ext.h implements this
selective iteration promotion. However, the indirection obfuscates more than
helps. Open code the iteration promotion in put_prev_task_balance() and
remove for_balance_class_range().

No functional changes intended.

Signed-off-by: Tejun Heo <tj@kernel.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Vernet <void@manifault.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/sched/core.c | 14 +++++++++++++-
 kernel/sched/ext.h  |  9 ---------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1092955a7d6e..827e0dc78ea0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5834,7 +5834,19 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
 				  struct rq_flags *rf)
 {
 #ifdef CONFIG_SMP
+	const struct sched_class *start_class = prev->sched_class;
 	const struct sched_class *class;
+
+#ifdef CONFIG_SCHED_CLASS_EXT
+	/*
+	 * SCX requires a balance() call before every pick_next_task() including
+	 * when waking up from SCHED_IDLE. If @start_class is below SCX, start
+	 * from SCX instead.
+	 */
+	if (sched_class_above(&ext_sched_class, start_class))
+		start_class = &ext_sched_class;
+#endif
+
 	/*
 	 * We must do the balancing pass before put_prev_task(), such
 	 * that when we release the rq->lock the task is in the same
@@ -5843,7 +5855,7 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
 	 * We can terminate the balance pass as soon as we know there is
 	 * a runnable task of @class priority or higher.
 	 */
-	for_balance_class_range(class, prev->sched_class, &idle_sched_class) {
+	for_active_class_range(class, start_class, &idle_sched_class) {
 		if (class->balance(rq, prev, rf))
 			break;
 	}
diff --git a/kernel/sched/ext.h b/kernel/sched/ext.h
index 229007693504..1d7837bdfaba 100644
--- a/kernel/sched/ext.h
+++ b/kernel/sched/ext.h
@@ -68,14 +68,6 @@ static inline const struct sched_class *next_active_class(const struct sched_cla
 #define for_each_active_class(class)						\
 	for_active_class_range(class, __sched_class_highest, __sched_class_lowest)
 
-/*
- * SCX requires a balance() call before every pick_next_task() call including
- * when waking up from idle.
- */
-#define for_balance_class_range(class, prev_class, end_class)			\
-	for_active_class_range(class, (prev_class) > &ext_sched_class ?		\
-			       &ext_sched_class : (prev_class), (end_class))
-
 #ifdef CONFIG_SCHED_CORE
 bool scx_prio_less(const struct task_struct *a, const struct task_struct *b,
 		   bool in_fi);
@@ -100,7 +92,6 @@ static inline bool task_on_scx(const struct task_struct *p) { return false; }
 static inline void init_sched_ext_class(void) {}
 
 #define for_each_active_class		for_each_class
-#define for_balance_class_range		for_class_range
 
 #endif	/* CONFIG_SCHED_CLASS_EXT */
 
-- 
2.45.2

Re: [PATCH 2/3] sched, sched_ext: Open code for_balance_class_range()

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 3838 bytes --]

On Sat, Jun 22, 2024 at 03:50:21PM -1000, Tejun Heo wrote:
> For flexibility, sched_ext allows the BPF scheduler to select the CPU to
> execute a task on at dispatch time so that e.g. a queue can be shared across
> multiple CPUs. To enable this, the dispatch path is executed from balance()
> so that a dispatched task can be hot-migrated to its target CPU. This means
> that sched_ext needs its balance() method invoked before every
> pick_next_task() even when the CPU is waking up from SCHED_IDLE.
> 
> for_balance_class_range() defined in kernel/sched/ext.h implements this
> selective iteration promotion. However, the indirection obfuscates more than
> helps. Open code the iteration promotion in put_prev_task_balance() and
> remove for_balance_class_range().
> 
> No functional changes intended.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: David Vernet <void@manifault.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>

Acked-by: David Vernet <void@manifault.com>

> ---
>  kernel/sched/core.c | 14 +++++++++++++-
>  kernel/sched/ext.h  |  9 ---------
>  2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1092955a7d6e..827e0dc78ea0 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5834,7 +5834,19 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
>  				  struct rq_flags *rf)
>  {
>  #ifdef CONFIG_SMP
> +	const struct sched_class *start_class = prev->sched_class;
>  	const struct sched_class *class;
> +
> +#ifdef CONFIG_SCHED_CLASS_EXT
> +	/*
> +	 * SCX requires a balance() call before every pick_next_task() including
> +	 * when waking up from SCHED_IDLE. If @start_class is below SCX, start
> +	 * from SCX instead.
> +	 */
> +	if (sched_class_above(&ext_sched_class, start_class))
> +		start_class = &ext_sched_class;
> +#endif
> +
>  	/*
>  	 * We must do the balancing pass before put_prev_task(), such
>  	 * that when we release the rq->lock the task is in the same
> @@ -5843,7 +5855,7 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
>  	 * We can terminate the balance pass as soon as we know there is
>  	 * a runnable task of @class priority or higher.
>  	 */
> -	for_balance_class_range(class, prev->sched_class, &idle_sched_class) {
> +	for_active_class_range(class, start_class, &idle_sched_class) {
>  		if (class->balance(rq, prev, rf))
>  			break;
>  	}
> diff --git a/kernel/sched/ext.h b/kernel/sched/ext.h
> index 229007693504..1d7837bdfaba 100644
> --- a/kernel/sched/ext.h
> +++ b/kernel/sched/ext.h
> @@ -68,14 +68,6 @@ static inline const struct sched_class *next_active_class(const struct sched_cla
>  #define for_each_active_class(class)						\
>  	for_active_class_range(class, __sched_class_highest, __sched_class_lowest)
>  
> -/*
> - * SCX requires a balance() call before every pick_next_task() call including
> - * when waking up from idle.
> - */
> -#define for_balance_class_range(class, prev_class, end_class)			\
> -	for_active_class_range(class, (prev_class) > &ext_sched_class ?		\
> -			       &ext_sched_class : (prev_class), (end_class))
> -
>  #ifdef CONFIG_SCHED_CORE
>  bool scx_prio_less(const struct task_struct *a, const struct task_struct *b,
>  		   bool in_fi);
> @@ -100,7 +92,6 @@ static inline bool task_on_scx(const struct task_struct *p) { return false; }
>  static inline void init_sched_ext_class(void) {}
>  
>  #define for_each_active_class		for_each_class
> -#define for_balance_class_range		for_class_range
>  
>  #endif	/* CONFIG_SCHED_CLASS_EXT */
>  
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 3/3] sched, sched_ext: Move some declarations from kernel/sched/ext.h to sched.h

[Tejun Heo]

While sched_ext was out of tree, everything sched_ext specific which can be
put in kernel/sched/ext.h was put there to ease forward porting. However,
kernel/sched/sched.h is the better location for some of them. Relocate.

- struct sched_enq_and_set_ctx, sched_deq_and_put_task() and
  sched_enq_and_set_task().

- scx_enabled() and scx_switched_all().

- for_active_class_range() and for_each_active_class(). sched_class
  declarations are moved above the class iterators for this.

No functional changes intended.

Signed-off-by: Tejun Heo <tj@kernel.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Vernet <void@manifault.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/sched/ext.h   | 39 --------------------------
 kernel/sched/sched.h | 65 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/kernel/sched/ext.h b/kernel/sched/ext.h
index 1d7837bdfaba..32d3a51f591a 100644
--- a/kernel/sched/ext.h
+++ b/kernel/sched/ext.h
@@ -8,24 +8,6 @@
  */
 #ifdef CONFIG_SCHED_CLASS_EXT
 
-struct sched_enq_and_set_ctx {
-	struct task_struct	*p;
-	int			queue_flags;
-	bool			queued;
-	bool			running;
-};
-
-void sched_deq_and_put_task(struct task_struct *p, int queue_flags,
-			    struct sched_enq_and_set_ctx *ctx);
-void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx);
-
-extern const struct sched_class ext_sched_class;
-
-DECLARE_STATIC_KEY_FALSE(__scx_ops_enabled);
-DECLARE_STATIC_KEY_FALSE(__scx_switched_all);
-#define scx_enabled()		static_branch_unlikely(&__scx_ops_enabled)
-#define scx_switched_all()	static_branch_unlikely(&__scx_switched_all)
-
 void scx_tick(struct rq *rq);
 void init_scx_entity(struct sched_ext_entity *scx);
 void scx_pre_fork(struct task_struct *p);
@@ -52,22 +34,6 @@ static inline bool task_on_scx(const struct task_struct *p)
 	return scx_enabled() && p->sched_class == &ext_sched_class;
 }
 
-static inline const struct sched_class *next_active_class(const struct sched_class *class)
-{
-	class++;
-	if (scx_switched_all() && class == &fair_sched_class)
-		class++;
-	if (!scx_enabled() && class == &ext_sched_class)
-		class++;
-	return class;
-}
-
-#define for_active_class_range(class, _from, _to)				\
-	for (class = (_from); class != (_to); class = next_active_class(class))
-
-#define for_each_active_class(class)						\
-	for_active_class_range(class, __sched_class_highest, __sched_class_lowest)
-
 #ifdef CONFIG_SCHED_CORE
 bool scx_prio_less(const struct task_struct *a, const struct task_struct *b,
 		   bool in_fi);
@@ -75,9 +41,6 @@ bool scx_prio_less(const struct task_struct *a, const struct task_struct *b,
 
 #else	/* CONFIG_SCHED_CLASS_EXT */
 
-#define scx_enabled()		false
-#define scx_switched_all()	false
-
 static inline void scx_tick(struct rq *rq) {}
 static inline void scx_pre_fork(struct task_struct *p) {}
 static inline int scx_fork(struct task_struct *p) { return 0; }
@@ -91,8 +54,6 @@ static inline int scx_check_setscheduler(struct task_struct *p, int policy) { re
 static inline bool task_on_scx(const struct task_struct *p) { return false; }
 static inline void init_sched_ext_class(void) {}
 
-#define for_each_active_class		for_each_class
-
 #endif	/* CONFIG_SCHED_CLASS_EXT */
 
 #if defined(CONFIG_SCHED_CLASS_EXT) && defined(CONFIG_SMP)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 963a2fa180ad..2a433f760813 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2438,19 +2438,54 @@ const struct sched_class name##_sched_class \
 extern struct sched_class __sched_class_highest[];
 extern struct sched_class __sched_class_lowest[];
 
+extern const struct sched_class stop_sched_class;
+extern const struct sched_class dl_sched_class;
+extern const struct sched_class rt_sched_class;
+extern const struct sched_class fair_sched_class;
+extern const struct sched_class idle_sched_class;
+
+#ifdef CONFIG_SCHED_CLASS_EXT
+extern const struct sched_class ext_sched_class;
+
+DECLARE_STATIC_KEY_FALSE(__scx_ops_enabled);	/* SCX BPF scheduler loaded */
+DECLARE_STATIC_KEY_FALSE(__scx_switched_all);	/* all fair class tasks on SCX */
+
+#define scx_enabled()		static_branch_unlikely(&__scx_ops_enabled)
+#define scx_switched_all()	static_branch_unlikely(&__scx_switched_all)
+#else /* !CONFIG_SCHED_CLASS_EXT */
+#define scx_enabled()		false
+#define scx_switched_all()	false
+#endif /* !CONFIG_SCHED_CLASS_EXT */
+
+/*
+ * Iterate only active classes. SCX can take over all fair tasks or be
+ * completely disabled. If the former, skip fair. If the latter, skip SCX.
+ */
+static inline const struct sched_class *next_active_class(const struct sched_class *class)
+{
+	class++;
+#ifdef CONFIG_SCHED_CLASS_EXT
+	if (scx_switched_all() && class == &fair_sched_class)
+		class++;
+	if (!scx_enabled() && class == &ext_sched_class)
+		class++;
+#endif
+	return class;
+}
+
 #define for_class_range(class, _from, _to) \
 	for (class = (_from); class < (_to); class++)
 
 #define for_each_class(class) \
 	for_class_range(class, __sched_class_highest, __sched_class_lowest)
 
-#define sched_class_above(_a, _b)	((_a) < (_b))
+#define for_active_class_range(class, _from, _to)				\
+	for (class = (_from); class != (_to); class = next_active_class(class))
 
-extern const struct sched_class stop_sched_class;
-extern const struct sched_class dl_sched_class;
-extern const struct sched_class rt_sched_class;
-extern const struct sched_class fair_sched_class;
-extern const struct sched_class idle_sched_class;
+#define for_each_active_class(class)						\
+	for_active_class_range(class, __sched_class_highest, __sched_class_lowest)
+
+#define sched_class_above(_a, _b)	((_a) < (_b))
 
 static inline bool sched_stop_runnable(struct rq *rq)
 {
@@ -3698,6 +3733,24 @@ static inline void balance_callbacks(struct rq *rq, struct balance_callback *hea
 
 #endif
 
+#ifdef CONFIG_SCHED_CLASS_EXT
+/*
+ * Used by SCX in the enable/disable paths to move tasks between sched_classes
+ * and establish invariants.
+ */
+struct sched_enq_and_set_ctx {
+	struct task_struct	*p;
+	int			queue_flags;
+	bool			queued;
+	bool			running;
+};
+
+void sched_deq_and_put_task(struct task_struct *p, int queue_flags,
+			    struct sched_enq_and_set_ctx *ctx);
+void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx);
+
+#endif /* CONFIG_SCHED_CLASS_EXT */
+
 #include "ext.h"
 
 #endif /* _KERNEL_SCHED_SCHED_H */
-- 
2.45.2

Re: [PATCH 3/3] sched, sched_ext: Move some declarations from kernel/sched/ext.h to sched.h

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

On Sat, Jun 22, 2024 at 03:50:22PM -1000, Tejun Heo wrote:

Hello Tejun,

> While sched_ext was out of tree, everything sched_ext specific which can be
> put in kernel/sched/ext.h was put there to ease forward porting. However,
> kernel/sched/sched.h is the better location for some of them. Relocate.
> 
> - struct sched_enq_and_set_ctx, sched_deq_and_put_task() and
>   sched_enq_and_set_task().
> 
> - scx_enabled() and scx_switched_all().
> 
> - for_active_class_range() and for_each_active_class(). sched_class
>   declarations are moved above the class iterators for this.
> 
> No functional changes intended.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: David Vernet <void@manifault.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---

[...]

Bringing everything above this into sched.h seems reasonable. See below for
some thoughts about sched_deq_and_put_task() and sched_enq_and_set_task().

>  
>  static inline bool sched_stop_runnable(struct rq *rq)
>  {
> @@ -3698,6 +3733,24 @@ static inline void balance_callbacks(struct rq *rq, struct balance_callback *hea
>  
>  #endif
>  
> +#ifdef CONFIG_SCHED_CLASS_EXT
> +/*
> + * Used by SCX in the enable/disable paths to move tasks between sched_classes
> + * and establish invariants.
> + */
> +struct sched_enq_and_set_ctx {
> +	struct task_struct	*p;
> +	int			queue_flags;
> +	bool			queued;
> +	bool			running;
> +};
> +
> +void sched_deq_and_put_task(struct task_struct *p, int queue_flags,
> +			    struct sched_enq_and_set_ctx *ctx);
> +void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx);

I'm not opposed to bringing these into sched.h, but they're implementing a
pattern that's used almost verbatim in a few other places in core.c, so it
would be nice if we could figure out some way to just have every caller use the
same API for doing the whole dequeue/save -> update state -> re-enqueue/restore
dance. We had SCHED_CHANGE_BLOCK [0] way back in v2, but IIRC we dropped it
because upstream was moving towards a more generic implementation. It looks
like that hasn't materialized yet, so maybe we should resurrect that?

[0]: https://lore.kernel.org/all/20230128001639.3510083-3-tj@kernel.org/

Anyways, no objection from me in moving this into sched.h (as long as Peter,
Thomas and Ingo are OK with it). But it does feel kind of unfortunate to stick
it there in its current form given that it's only used by ext.c, despite being
95% of the way there for quite a few potential callsites in core.c.

Thanks,
David

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCHSET sched_ext/for-6.11] sched_ext: Clean up kernel/sched/ext.h

[Tejun Heo]

On Sat, Jun 22, 2024 at 03:50:19PM -1000, Tejun Heo wrote:
> Hello,
> 
> While sched_ext was out of tree, kernel/sched/ext.h contained declarations
> and definitions which aren't ideal but are helpful for forward porting. This
> patchset cleans them up.
> 
> - for_balance_class_range() is removed and instead open coded in
>   put_prev_task_balance().
> 
> - Some declarations and definitions in kernel/sched/ext.h are moved to
>   kernel/sched/sched.h.
> 
> This patchset contains the following three patches:
> 
>   0001-sched_ext-Minor-cleanups-in-kernel-sched-ext.h.patch
>   0002-sched-sched_ext-Open-code-for_balance_class_range.patch
>   0003-sched-sched_ext-Move-some-declarations-from-kernel-s.patch

Applied to sched_ext/for-6.11.

Thanks.

-- 
tejun