Received-SPF: pass (google.com: domain of linux-kernel+bounces-179819-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 15 May 2024 13:48:28 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Luis Machado <luis.machado@arm.com>
Cc: mingo@redhat.com, juri.lelli@redhat.com, vincent.guittot@linaro.org,
	dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
	mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com,
	linux-kernel@vger.kernel.org, kprateek.nayak@amd.com,
	wuyun.abel@bytedance.com, tglx@linutronix.de, efault@gmx.de,
	nd <nd@arm.com>, John Stultz <jstultz@google.com>,
	Hongyan.Xia2@arm.com
Subject: Re: [RFC][PATCH 08/10] sched/fair: Implement delayed dequeue
Message-ID: <20240515114828.GO12673@noisy.programming.kicks-ass.net>
References: <20240405110010.631664251@infradead.org>
 <3888d7c8-660e-479c-8c10-8295204e5f36@arm.com>
 <1461277e-af68-41e7-947c-9178b55810b1@arm.com>
 <20240425104220.GE21980@noisy.programming.kicks-ass.net>
 <20240425114949.GH12673@noisy.programming.kicks-ass.net>
 <20240426093241.GI12673@noisy.programming.kicks-ass.net>
 <c6152855-ef92-4c24-a3f5-64d4256b6789@arm.com>
 <2fba04b0-e55e-41f4-8b7a-723734fe1ad2@arm.com>
 <219b8b49-3767-4010-aa68-9e1cf66c2ccb@arm.com>
 <20240515093649.GF40213@noisy.programming.kicks-ass.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240515093649.GF40213@noisy.programming.kicks-ass.net>

On Wed, May 15, 2024 at 11:36:49AM +0200, Peter Zijlstra wrote:
> On Fri, May 10, 2024 at 03:49:46PM +0100, Luis Machado wrote:
> > Just a quick update on this. While investigating this behavior, I
> > spotted very high loadavg values on an idle system. For instance:
> > 
> > load average: 4733.84, 4721.24, 4680.33
> > 
> > I wonder if someone else also spotted this.
> 
> Hadn't spotted it, but now that you mention it, I can definitely see it.
> 
> Let me go prod with something sharp. Thanks!

What's the point of making notes if you then don't read them... *sigh*.

Does this help?

---
 kernel/sched/core.c  | 23 ++++++++++++-----------
 kernel/sched/fair.c  |  4 ++--
 kernel/sched/sched.h |  8 ++++++++
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5ffd7e047393..43f061bcfe54 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2154,14 +2154,18 @@ void activate_task(struct rq *rq, struct task_struct *p, int flags)
 
 void deactivate_task(struct rq *rq, struct task_struct *p, int flags)
 {
-	bool sleep = flags & DEQUEUE_SLEEP;
+	SCHED_WARN_ON(flags & DEQUEUE_SLEEP);
 
-	if (dequeue_task(rq, p, flags)) {
-		WRITE_ONCE(p->on_rq, sleep ? 0 : TASK_ON_RQ_MIGRATING);
-		ASSERT_EXCLUSIVE_WRITER(p->on_rq);
-	} else {
-		SCHED_WARN_ON(!sleep); /* only sleep can fail */
-	}
+	dequeue_task(rq, p, flags);
+
+	WRITE_ONCE(p->on_rq, TASK_ON_RQ_MIGRATING);
+	ASSERT_EXCLUSIVE_WRITER(p->on_rq);
+}
+
+static void block_task(struct rq *rq, struct task_struct *p, int flags)
+{
+	if (dequeue_task(rq, p, DEQUEUE_SLEEP | flags))
+		__block_task(rq, p);
 }
 
 static inline int __normal_prio(int policy, int rt_prio, int nice)
@@ -6693,9 +6697,6 @@ static void __sched notrace __schedule(unsigned int sched_mode)
 				!(prev_state & TASK_NOLOAD) &&
 				!(prev_state & TASK_FROZEN);
 
-			if (prev->sched_contributes_to_load)
-				rq->nr_uninterruptible++;
-
 			/*
 			 * __schedule()			ttwu()
 			 *   prev_state = prev->state;    if (p->on_rq && ...)
@@ -6707,7 +6708,7 @@ static void __sched notrace __schedule(unsigned int sched_mode)
 			 *
 			 * After this, schedule() must not care about p->state any more.
 			 */
-			deactivate_task(rq, prev, DEQUEUE_SLEEP | DEQUEUE_NOCLOCK);
+			block_task(rq, prev, DEQUEUE_SLEEP | DEQUEUE_NOCLOCK);
 
 			if (prev->in_iowait) {
 				atomic_inc(&rq->nr_iowait);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 536eabcb1a71..596a5fabe490 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7032,8 +7032,8 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
 			util_est_update(&rq->cfs, p, task_sleep);
 			hrtick_update(rq);
 
-			/* Fix-up what deactivate_task() skipped. */
-			WRITE_ONCE(p->on_rq, 0);
+			/* Fix-up what block_task() skipped. */
+			__block_task(rq, p);
 		}
 	}
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 12841d8195c5..48e5f49d9bc2 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2560,6 +2560,14 @@ static inline void sub_nr_running(struct rq *rq, unsigned count)
 	sched_update_tick_dependency(rq);
 }
 
+static inline void __block_task(struct rq *rq, struct task_struct *p)
+{
+	WRITE_ONCE(p->on_rq, 0);
+	ASSERT_EXCLUSIVE_WRITER(p->on_rq);
+	if (p->sched_contributes_to_load)
+		rq->nr_uninterruptible++;
+}
+
 extern void activate_task(struct rq *rq, struct task_struct *p, int flags);
 extern void deactivate_task(struct rq *rq, struct task_struct *p, int flags);