mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Daniel Bristot de Oliveira <bristot@kernel.org>
To: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>,
	linux-kernel@vger.kernel.org,
	Luca Abeni <luca.abeni@santannapisa.it>,
	Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>,
	Thomas Gleixner <tglx@linutronix.de>,
	Joel Fernandes <joel@joelfernandes.org>,
	Vineeth Pillai <vineeth@bitbyteword.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	bristot@kernel.org, Phil Auld <pauld@redhat.com>,
	Suleiman Souhlal <suleiman@google.com>,
	Youssef Esmat <youssefesmat@google.com>
Subject: [PATCH V7 8/9] sched/core: Fix picking of tasks for core scheduling with DL server
Date: Mon, 27 May 2024 14:06:54 +0200	[thread overview]
Message-ID: <b10489ab1f03d23e08e6097acea47442e7d6466f.1716811044.git.bristot@kernel.org> (raw)
In-Reply-To: <cover.1716811043.git.bristot@kernel.org>

From: "Joel Fernandes (Google)" <joel@joelfernandes.org>

* Use simple CFS pick_task for DL pick_task

  DL server's pick_task calls CFS's pick_next_task_fair(), this is wrong
  because core scheduling's pick_task only calls CFS's pick_task() for
  evaluation / checking of the CFS task (comparing across CPUs), not for
  actually affirmatively picking the next task. This causes RB tree
  corruption issues in CFS that were found by syzbot.

* Make pick_task_fair clear DL server

  A DL task pick might set ->dl_server, but it is possible the task will
  never run (say the other HT has a stop task). If the CFS task is picked
  in the future directly (say without DL server), ->dl_server will be
  set. So clear it in pick_task_fair().

This fixes the KASAN issue reported by syzbot in set_next_entity().

(DL refactoring suggestions by Vineeth Pillai).

Reviewed-by: Vineeth Pillai <vineeth@bitbyteword.org>
Reported-by: Suleiman Souhlal <suleiman@google.com>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 include/linux/sched.h   |  3 ++-
 kernel/sched/deadline.c | 27 ++++++++++++++++++++++-----
 kernel/sched/fair.c     | 23 +++++++++++++++++++++--
 kernel/sched/sched.h    |  3 ++-
 4 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 611771fec4df..eb8f8b7929c8 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -684,7 +684,8 @@ struct sched_dl_entity {
 	 */
 	struct rq			*rq;
 	dl_server_has_tasks_f		server_has_tasks;
-	dl_server_pick_f		server_pick;
+	dl_server_pick_f		server_pick_next;
+	dl_server_pick_f		server_pick_task;
 
 #ifdef CONFIG_RT_MUTEXES
 	/*
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index f8afe0a69c1e..0dbb42cf7fe6 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1664,11 +1664,13 @@ void dl_server_stop(struct sched_dl_entity *dl_se)
 
 void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 		    dl_server_has_tasks_f has_tasks,
-		    dl_server_pick_f pick)
+		    dl_server_pick_f pick_next,
+		    dl_server_pick_f pick_task)
 {
 	dl_se->rq = rq;
 	dl_se->server_has_tasks = has_tasks;
-	dl_se->server_pick = pick;
+	dl_se->server_pick_next = pick_next;
+	dl_se->server_pick_task = pick_task;
 }
 
 void __dl_server_attach_root(struct sched_dl_entity *dl_se, struct rq *rq)
@@ -2394,7 +2396,12 @@ static struct sched_dl_entity *pick_next_dl_entity(struct dl_rq *dl_rq)
 	return __node_2_dle(left);
 }
 
-static struct task_struct *pick_task_dl(struct rq *rq)
+/*
+ * __pick_next_task_dl - Helper to pick the next -deadline task to run.
+ * @rq: The runqueue to pick the next task from.
+ * @peek: If true, just peek at the next task. Only relevant for dlserver.
+ */
+static struct task_struct *__pick_next_task_dl(struct rq *rq, bool peek)
 {
 	struct sched_dl_entity *dl_se;
 	struct dl_rq *dl_rq = &rq->dl;
@@ -2408,7 +2415,10 @@ static struct task_struct *pick_task_dl(struct rq *rq)
 	WARN_ON_ONCE(!dl_se);
 
 	if (dl_server(dl_se)) {
-		p = dl_se->server_pick(dl_se);
+		if (IS_ENABLED(CONFIG_SMP) && peek)
+			p = dl_se->server_pick_task(dl_se);
+		else
+			p = dl_se->server_pick_next(dl_se);
 		if (!p) {
 			WARN_ON_ONCE(1);
 			dl_se->dl_yielded = 1;
@@ -2423,11 +2433,18 @@ static struct task_struct *pick_task_dl(struct rq *rq)
 	return p;
 }
 
+#ifdef CONFIG_SMP
+static struct task_struct *pick_task_dl(struct rq *rq)
+{
+	return __pick_next_task_dl(rq, true);
+}
+#endif
+
 static struct task_struct *pick_next_task_dl(struct rq *rq)
 {
 	struct task_struct *p;
 
-	p = pick_task_dl(rq);
+	p = __pick_next_task_dl(rq, false);
 	if (!p)
 		return p;
 
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 20e8b02c5cb3..14ec002bb4f9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8484,6 +8484,14 @@ static struct task_struct *pick_task_fair(struct rq *rq)
 		cfs_rq = group_cfs_rq(se);
 	} while (cfs_rq);
 
+	/*
+	 * This can be called from directly from CFS's ->pick_task() or indirectly
+	 * from DL's ->pick_task when fair server is enabled. In the indirect case,
+	 * DL will set ->dl_server just after this function is called, so its Ok to
+	 * clear. In the direct case, we are picking directly so we must clear it.
+	 */
+	task_of(se)->dl_server = NULL;
+
 	return task_of(se);
 }
 #endif
@@ -8643,7 +8651,16 @@ static bool fair_server_has_tasks(struct sched_dl_entity *dl_se)
 	return !!dl_se->rq->cfs.nr_running;
 }
 
-static struct task_struct *fair_server_pick(struct sched_dl_entity *dl_se)
+static struct task_struct *fair_server_pick_task(struct sched_dl_entity *dl_se)
+{
+#ifdef CONFIG_SMP
+	return pick_task_fair(dl_se->rq);
+#else
+	return NULL;
+#endif
+}
+
+static struct task_struct *fair_server_pick_next(struct sched_dl_entity *dl_se)
 {
 	return pick_next_task_fair(dl_se->rq, NULL, NULL);
 }
@@ -8654,7 +8671,9 @@ void fair_server_init(struct rq *rq)
 
 	init_dl_entity(dl_se);
 
-	dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick);
+	dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick_next,
+		       fair_server_pick_task);
+
 }
 
 /*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ed7be7b085af..3b8684b5ec8e 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -354,7 +354,8 @@ extern void dl_server_start(struct sched_dl_entity *dl_se);
 extern void dl_server_stop(struct sched_dl_entity *dl_se);
 extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 		    dl_server_has_tasks_f has_tasks,
-		    dl_server_pick_f pick);
+		    dl_server_pick_f pick_next,
+		    dl_server_pick_f pick_task);
 
 extern void dl_server_update_idle_time(struct rq *rq,
 		    struct task_struct *p);
-- 
2.45.1


  parent reply	other threads:[~2024-05-27 12:07 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-27 12:06 [PATCH V7 0/9] SCHED_DEADLINE server infrastructure Daniel Bristot de Oliveira
2024-05-27 12:06 ` [PATCH V7 1/9] sched/deadline: Comment sched_dl_entity::dl_server variable Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Daniel Bristot de Oliveira
2024-05-27 12:06 ` [PATCH V7 2/9] sched/core: Add clearing of ->dl_server in put_prev_task_balance() Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Joel Fernandes (Google)
2024-05-27 12:06 ` [PATCH V7 3/9] sched/core: Clear prev->dl_server in CFS pick fast path Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Youssef Esmat
2024-05-27 12:06 ` [PATCH V7 4/9] sched/fair: Add trivial fair server Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-05-27 12:06 ` [PATCH V7 5/9] sched/deadline: Deferrable dl server Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Daniel Bristot de Oliveira
2024-05-27 12:06 ` [PATCH V7 6/9] sched/fair: Fair server interface Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Daniel Bristot de Oliveira
2024-05-27 12:06 ` [PATCH V7 7/9] sched/core: Fix priority checking for DL server picks Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Joel Fernandes (Google)
2024-05-27 12:06 ` Daniel Bristot de Oliveira [this message]
2024-07-29 10:34   ` [tip: sched/core] sched/core: Fix picking of tasks for core scheduling with DL server tip-bot2 for Joel Fernandes (Google)
2024-05-27 12:06 ` [PATCH V7 9/9] sched/rt: Remove default bandwidth control Daniel Bristot de Oliveira
2024-07-29 10:34   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-11-27 10:55   ` [PATCH V7 9/9] " Michal Koutný
2024-11-27 15:35     ` Juri Lelli
2024-11-29 10:02       ` Michal Koutný
2024-11-29 14:44         ` Juri Lelli
2024-11-29 20:21           ` Michal Koutný
2024-12-02  9:39             ` Juri Lelli
2024-06-21 13:37 ` [PATCH V7 0/9] SCHED_DEADLINE server infrastructure Juri Lelli
2024-06-21 13:43   ` Daniel Bristot de Oliveira
2024-06-21 13:50     ` Juri Lelli
2024-06-21 14:41 ` Vineeth Remanan Pillai
2024-06-21 14:59   ` Daniel Bristot de Oliveira
2024-06-21 15:09     ` Vineeth Remanan Pillai
2024-06-21 15:16       ` Daniel Bristot de Oliveira
2024-07-29 10:32   ` Peter Zijlstra
2024-07-29 20:42     ` Vineeth Remanan Pillai

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=b10489ab1f03d23e08e6097acea47442e7d6466f.1716811044.git.bristot@kernel.org \
    --to=bristot@kernel.org \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joel@joelfernandes.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luca.abeni@santannapisa.it \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=skhan@linuxfoundation.org \
    --cc=suleiman@google.com \
    --cc=tglx@linutronix.de \
    --cc=tommaso.cucinotta@santannapisa.it \
    --cc=vincent.guittot@linaro.org \
    --cc=vineeth@bitbyteword.org \
    --cc=vschneid@redhat.com \
    --cc=youssefesmat@google.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

all inboxes | Powered by JetHome®