In order to distinguish between a regular vs a core pick_task()
invocation, add a boolean argument.

Notably SCX seems to need this, since its core pick

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/sched.h    |    2 +-
 kernel/sched/core.c      |    8 ++++----
 kernel/sched/deadline.c  |    9 ++-------
 kernel/sched/fair.c      |    9 +++++----
 kernel/sched/idle.c      |    2 +-
 kernel/sched/rt.c        |    2 +-
 kernel/sched/sched.h     |    4 ++--
 kernel/sched/stop_task.c |    2 +-
 8 files changed, 17 insertions(+), 21 deletions(-)

--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -601,7 +601,7 @@ struct sched_rt_entity {
 } __randomize_layout;
 
 typedef bool (*dl_server_has_tasks_f)(struct sched_dl_entity *);
-typedef struct task_struct *(*dl_server_pick_f)(struct sched_dl_entity *);
+typedef struct task_struct *(*dl_server_pick_f)(struct sched_dl_entity *, bool);
 
 struct sched_dl_entity {
 	struct rb_node			rb_node;
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5885,7 +5885,7 @@ __pick_next_task(struct rq *rq, struct t
 
 		/* Assume the next prioritized class is idle_sched_class */
 		if (!p) {
-			p = pick_task_idle(rq);
+			p = pick_task_idle(rq, false);
 			put_prev_set_next_task(rq, prev, p);
 		}
 
@@ -5901,7 +5901,7 @@ __pick_next_task(struct rq *rq, struct t
 			if (p)
 				return p;
 		} else {
-			p = class->pick_task(rq);
+			p = class->pick_task(rq, false);
 			if (p) {
 				put_prev_set_next_task(rq, prev, p);
 				return p;
@@ -5939,7 +5939,7 @@ static inline struct task_struct *pick_t
 	rq->dl_server = NULL;
 
 	for_each_class(class) {
-		p = class->pick_task(rq);
+		p = class->pick_task(rq, true);
 		if (p)
 			return p;
 	}
@@ -6092,7 +6092,7 @@ pick_next_task(struct rq *rq, struct tas
 			if (cookie)
 				p = sched_core_find(rq_i, cookie);
 			if (!p)
-				p = idle_sched_class.pick_task(rq_i);
+				p = idle_sched_class.pick_task(rq_i, true);
 		}
 
 		rq_i->core_pick = p;
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2409,7 +2409,7 @@ static struct sched_dl_entity *pick_next
  * __pick_next_task_dl - Helper to pick the next -deadline task to run.
  * @rq: The runqueue to pick the next task from.
  */
-static struct task_struct *__pick_task_dl(struct rq *rq)
+static struct task_struct *pick_task_dl(struct rq *rq, bool core)
 {
 	struct sched_dl_entity *dl_se;
 	struct dl_rq *dl_rq = &rq->dl;
@@ -2423,7 +2423,7 @@ static struct task_struct *__pick_task_d
 	WARN_ON_ONCE(!dl_se);
 
 	if (dl_server(dl_se)) {
-		p = dl_se->server_pick_task(dl_se);
+		p = dl_se->server_pick_task(dl_se, core);
 		if (!p) {
 			dl_se->dl_yielded = 1;
 			update_curr_dl_se(rq, dl_se, 0);
@@ -2437,11 +2437,6 @@ static struct task_struct *__pick_task_d
 	return p;
 }
 
-static struct task_struct *pick_task_dl(struct rq *rq)
-{
-	return __pick_task_dl(rq);
-}
-
 static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_struct *next)
 {
 	struct sched_dl_entity *dl_se = &p->dl;
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8723,7 +8723,7 @@ static void check_preempt_wakeup_fair(st
 	resched_curr(rq);
 }
 
-static struct task_struct *pick_task_fair(struct rq *rq)
+static struct task_struct *pick_task_fair(struct rq *rq, bool core)
 {
 	struct sched_entity *se;
 	struct cfs_rq *cfs_rq;
@@ -8761,7 +8761,7 @@ pick_next_task_fair(struct rq *rq, struc
 	int new_tasks;
 
 again:
-	p = pick_task_fair(rq);
+	p = pick_task_fair(rq, false);
 	if (!p)
 		goto idle;
 	se = &p->se;
@@ -8850,9 +8850,10 @@ static bool fair_server_has_tasks(struct
 	return !!dl_se->rq->cfs.nr_running;
 }
 
-static struct task_struct *fair_server_pick_task(struct sched_dl_entity *dl_se)
+static struct task_struct *
+fair_server_pick_task(struct sched_dl_entity *dl_se, bool core)
 {
-	return pick_task_fair(dl_se->rq);
+	return pick_task_fair(dl_se->rq, core);
 }
 
 void fair_server_init(struct rq *rq)
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -462,7 +462,7 @@ static void set_next_task_idle(struct rq
 	next->se.exec_start = rq_clock_task(rq);
 }
 
-struct task_struct *pick_task_idle(struct rq *rq)
+struct task_struct *pick_task_idle(struct rq *rq, bool core)
 {
 	return rq->idle;
 }
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1736,7 +1736,7 @@ static struct task_struct *_pick_next_ta
 	return rt_task_of(rt_se);
 }
 
-static struct task_struct *pick_task_rt(struct rq *rq)
+static struct task_struct *pick_task_rt(struct rq *rq, bool core)
 {
 	struct task_struct *p;
 
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2302,7 +2302,7 @@ struct sched_class {
 
 	void (*wakeup_preempt)(struct rq *rq, struct task_struct *p, int flags);
 
-	struct task_struct *(*pick_task)(struct rq *rq);
+	struct task_struct *(*pick_task)(struct rq *rq, bool core);
 	/*
 	 * Optional! When implemented pick_next_task() should be equivalent to:
 	 *
@@ -2451,7 +2451,7 @@ static inline bool sched_fair_runnable(s
 }
 
 extern struct task_struct *pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf);
-extern struct task_struct *pick_task_idle(struct rq *rq);
+extern struct task_struct *pick_task_idle(struct rq *rq, bool core);
 
 #define SCA_CHECK		0x01
 #define SCA_MIGRATE_DISABLE	0x02
--- a/kernel/sched/stop_task.c
+++ b/kernel/sched/stop_task.c
@@ -33,7 +33,7 @@ static void set_next_task_stop(struct rq
 	stop->se.exec_start = rq_clock_task(rq);
 }
 
-static struct task_struct *pick_task_stop(struct rq *rq)
+static struct task_struct *pick_task_stop(struct rq *rq, bool core)
 {
 	if (!sched_stop_runnable(rq))
 		return NULL;

Hello, Peter.

On Wed, Aug 14, 2024 at 12:25:57AM +0200, Peter Zijlstra wrote:
> In order to distinguish between a regular vs a core pick_task()
> invocation, add a boolean argument.
> 
> Notably SCX seems to need this, since its core pick

Tried converting scx to use the new interface and it looks like SCX doesn't
need this either. With small behavior changes around ENQ_LAST and
stopping/running, pick_task() can behave the same for regular and core-sched
cases.

Everything else looks fine too. Once this patchset gets applied, I'll pull
and update right away.

Thanks.

-- 
tejun

On Wed, Aug 21, 2024 at 01:05:15PM -1000, Tejun Heo wrote:
> Hello, Peter.
> 
> On Wed, Aug 14, 2024 at 12:25:57AM +0200, Peter Zijlstra wrote:
> > In order to distinguish between a regular vs a core pick_task()
> > invocation, add a boolean argument.
> > 
> > Notably SCX seems to need this, since its core pick
> 
> Tried converting scx to use the new interface and it looks like SCX doesn't
> need this either. With small behavior changes around ENQ_LAST and
> stopping/running, pick_task() can behave the same for regular and core-sched
> cases.

OK, dropped the last one.

> Everything else looks fine too. Once this patchset gets applied, I'll pull
> and update right away.

And the rest should now be in tip/sched/core

Hi Peter,

On 14/08/24 00:25, Peter Zijlstra wrote:
> In order to distinguish between a regular vs a core pick_task()
> invocation, add a boolean argument.
> 
> Notably SCX seems to need this, since its core pick
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---

...

> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2409,7 +2409,7 @@ static struct sched_dl_entity *pick_next
>   * __pick_next_task_dl - Helper to pick the next -deadline task to run.

Super minor thing, but the above comment becomes stale after this and
previous changes.

>   * @rq: The runqueue to pick the next task from.
>   */
> -static struct task_struct *__pick_task_dl(struct rq *rq)
> +static struct task_struct *pick_task_dl(struct rq *rq, bool core)
>  {
>  	struct sched_dl_entity *dl_se;
>  	struct dl_rq *dl_rq = &rq->dl;
> @@ -2423,7 +2423,7 @@ static struct task_struct *__pick_task_d
>  	WARN_ON_ONCE(!dl_se);
>  
>  	if (dl_server(dl_se)) {
> -		p = dl_se->server_pick_task(dl_se);
> +		p = dl_se->server_pick_task(dl_se, core);
>  		if (!p) {
>  			dl_se->dl_yielded = 1;
>  			update_curr_dl_se(rq, dl_se, 0);
> @@ -2437,11 +2437,6 @@ static struct task_struct *__pick_task_d
>  	return p;
>  }
>  
> -static struct task_struct *pick_task_dl(struct rq *rq)
> -{
> -	return __pick_task_dl(rq);
> -}
> -

Best,
Juri

On Wed, Aug 14, 2024 at 12:25:57AM +0200, Peter Zijlstra wrote:
> In order to distinguish between a regular vs a core pick_task()
> invocation, add a boolean argument.
> 
> Notably SCX seems to need this, since its core pick

... and clearly I never finished that sentence. But mostly it has extra
bits on for sched_core that are not needed in the normal case, and since
we now use pick_task() per default, it needs to know.