From 8dc67def4b11c4f93af85b51ec1e9b2de6973f92 Mon Sep 17 00:00:00 2001
From: Augustin Cavalier <waddlesplash@gmail.com>
Date: Thu, 4 Aug 2022 17:08:38 -0400
Subject: [PATCH] kernel/scheduler: Always reschedule after enqueueing if the
 runqueue was empty.

Normally the heap's priority will suffice to check if we need to reschedule.
However, in cases where the CPU in question is currently in the middle of
rescheduling already, and it is about to change its priority to "idle",
we can race with it and not notice that we need to send it an ICI.
Previously this meant we would just lose some performance, but after
recent fixes to not reschedule only as necessary, this race led to hangs.

Now we report whether the runqueue we added the thread to was empty
in ThreadData::Enqueue(), and if it was, we then always trigger a
scheduler invocation on the target CPU.

In my testing, this fixes #17847; and at least in my unscientific
benchmarks, improves compile performance by as much as 10% (I saw
~55s -> ~50s in some tests.)
---
 src/system/kernel/scheduler/scheduler.cpp      | 16 +++++++++-------
 src/system/kernel/scheduler/scheduler_thread.h | 13 +++++++++----
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/system/kernel/scheduler/scheduler.cpp b/src/system/kernel/scheduler/scheduler.cpp
index 9ecb4e3e15..1913e57b35 100644
--- a/src/system/kernel/scheduler/scheduler.cpp
+++ b/src/system/kernel/scheduler/scheduler.cpp
@@ -108,19 +108,20 @@ enqueue(Thread* thread, bool newOne)
 		ASSERT(thread->previous_cpu != NULL);
 		ASSERT(threadData->Core() != NULL);
 		targetCPU = &gCPUEntries[thread->previous_cpu->cpu_num];
-	} else if (gSingleCore)
+	} else if (gSingleCore) {
 		targetCore = &gCoreEntries[0];
-	else if (threadData->Core() != NULL
+	} else if (threadData->Core() != NULL
 		&& (!newOne || !threadData->HasCacheExpired())) {
 		targetCore = threadData->Rebalance();
 	}
 
-	bool rescheduleNeeded = threadData->ChooseCoreAndCPU(targetCore, targetCPU);
+	const bool rescheduleNeeded = threadData->ChooseCoreAndCPU(targetCore, targetCPU);
 
 	TRACE("enqueueing thread %ld with priority %ld on CPU %ld (core %ld)\n",
 		thread->id, threadPriority, targetCPU->ID(), targetCore->ID());
 
-	threadData->Enqueue();
+	bool wasRunQueueEmpty = false;
+	threadData->Enqueue(wasRunQueueEmpty);
 
 	// notify listeners
 	NotifySchedulerListeners(&SchedulerListener::ThreadEnqueuedInRunQueue,
@@ -128,11 +129,12 @@ enqueue(Thread* thread, bool newOne)
 
 	int32 heapPriority = CPUPriorityHeap::GetKey(targetCPU);
 	if (threadPriority > heapPriority
-		|| (threadPriority == heapPriority && rescheduleNeeded)) {
+		|| (threadPriority == heapPriority && rescheduleNeeded)
+		|| wasRunQueueEmpty) {
 
-		if (targetCPU->ID() == smp_get_current_cpu())
+		if (targetCPU->ID() == smp_get_current_cpu()) {
 			gCPU[targetCPU->ID()].invoke_scheduler = true;
-		else {
+		} else {
 			smp_send_ici(targetCPU->ID(), SMP_MSG_RESCHEDULE, 0, 0, 0,
 				NULL, SMP_MSG_FLAG_ASYNC);
 		}
diff --git a/src/system/kernel/scheduler/scheduler_thread.h b/src/system/kernel/scheduler/scheduler_thread.h
index 73e581e282..e3504569c9 100644
--- a/src/system/kernel/scheduler/scheduler_thread.h
+++ b/src/system/kernel/scheduler/scheduler_thread.h
@@ -74,7 +74,7 @@ public:
 	inline	bigtime_t	WentSleepActive() const	{ return fWentSleepActive; }
 
 	inline	void		PutBack();
-	inline	void		Enqueue();
+	inline	void		Enqueue(bool& wasRunQueueEmpty);
 	inline	bool		Dequeue();
 
 	inline	void		UpdateActivity(bigtime_t active);
@@ -406,7 +406,7 @@ ThreadData::PutBack()
 
 
 inline void
-ThreadData::Enqueue()
+ThreadData::Enqueue(bool& wasRunQueueEmpty)
 {
 	SCHEDULER_ENTER_FUNCTION();
 
@@ -427,8 +427,7 @@ ThreadData::Enqueue()
 
 	fThread->state = B_THREAD_READY;
 
-	int32 priority = GetEffectivePriority();
-
+	const int32 priority = GetEffectivePriority();
 	if (fThread->pinned_to_cpu > 0) {
 		ASSERT(fThread->previous_cpu != NULL);
 		CPUEntry* cpu = CPUEntry::GetCPU(fThread->previous_cpu->cpu_num);
@@ -437,12 +436,18 @@ ThreadData::Enqueue()
 		ASSERT(!fEnqueued);
 		fEnqueued = true;
 
+		ThreadData* top = cpu->PeekThread();
+		wasRunQueueEmpty = (top == NULL || top->IsIdle());
+
 		cpu->PushBack(this, priority);
 	} else {
 		CoreRunQueueLocker _(fCore);
 		ASSERT(!fEnqueued);
 		fEnqueued = true;
 
+		ThreadData* top = fCore->PeekThread();
+		wasRunQueueEmpty = (top == NULL || top->IsIdle());
+
 		fCore->PushBack(this, priority);
 	}
 }