scheduler: Improve locking

src/system/kernel/scheduler/low_latency.cpp

@@ -36,20 +36,43 @@ has_cache_expired(Thread* thread)
 }
 
 
+static inline PackageEntry*
+get_most_idle_package(void)
+{
+	PackageEntry* current = &gPackageEntries[0];
+	for (int32 i = 1; i < gPackageCount; i++) {
+		if (gPackageEntries[i].fIdleCoreCount > current->fIdleCoreCount)
+			current = &gPackageEntries[i];
+	}
+
+	if (current->fIdleCoreCount == 0)
+		return NULL;
+
+	return current;
+}
+
+
 static int32
 choose_core(Thread* thread)
 {
-	CoreEntry* entry;
+	CoreEntry* entry = NULL;
 
-	if (gIdlePackageList->Last() != NULL) {
+	SpinLocker locker(gIdlePackageLock);
 	// wake new package
 	PackageEntry* package = gIdlePackageList->Last();
-		entry = package->fIdleCores.Last();
+	if (package == NULL) {
-	} else if (gPackageUsageHeap->PeekMaximum() != NULL) {
 		// wake new core
-		PackageEntry* package = gPackageUsageHeap->PeekMaximum();
+		package = get_most_idle_package();
+	}
+	locker.Unlock();
+
+	if (package != NULL) {
+		SpinLocker _(package->fCoreLock);
 		entry = package->fIdleCores.Last();
-	} else {
+	}
+
+	if (entry == NULL) {
+		ReadSpinLocker coreLocker(gCoreHeapsLock);
 		// no idle cores, use least occupied core
 		entry = gCoreLoadHeap->PeekMinimum();
 		if (entry == NULL)
@@ -77,7 +100,7 @@ should_rebalance(Thread* thread)
 	// If there is high load on this core but this thread does not contribute
 	// significantly consider giving it to someone less busy.
 	if (coreEntry->fLoad > kHighLoad) {
-		SpinLocker coreLocker(gCoreHeapsLock);
+		ReadSpinLocker coreLocker(gCoreHeapsLock);
 
 		CoreEntry* other = gCoreLoadHeap->PeekMinimum();
 		if (other != NULL && coreEntry->fLoad - other->fLoad >= kLoadDifference)
@@ -86,7 +109,7 @@ should_rebalance(Thread* thread)
 
 	// No cpu bound threads - the situation is quite good. Make sure it
 	// won't get much worse...
-	SpinLocker coreLocker(gCoreHeapsLock);
+	ReadSpinLocker coreLocker(gCoreHeapsLock);
 
 	CoreEntry* other = gCoreLoadHeap->PeekMinimum();
 	if (other == NULL)
@@ -121,14 +144,17 @@ rebalance_irqs(bool idle)
 	if (chosen == NULL || totalLoad < kLowLoad)
 		return;
 
-	SpinLocker coreLocker(gCoreHeapsLock);
+	ReadSpinLocker coreLocker(gCoreHeapsLock);
 	CoreEntry* other = gCoreLoadHeap->PeekMinimum();
 	if (other == NULL)
 		other = gCoreHighLoadHeap->PeekMinimum();
-
-	int32 newCPU = gCPUPriorityHeaps[other->fCoreID].PeekMinimum()->fCPUNumber;
 	coreLocker.Unlock();
 
+	SpinLocker cpuLocker(other->fCPULock);
+	int32 newCPU = gCPUPriorityHeaps[other->fCoreID].PeekMinimum()->fCPUNumber;
+	cpuLocker.Unlock();
+
+
 	ASSERT(other != NULL);
 
 	int32 thisCore = gCPUToCore[smp_get_current_cpu()];

src/system/kernel/scheduler/power_saving.cpp

@@ -41,6 +41,8 @@ switch_to_mode(void)
 static bool
 try_small_task_packing(Thread* thread)
 {
+	ReadSpinLocker locker(gCoreHeapsLock);
+
 	int32 core = sSmallTaskCore;
 	return (core == -1 && gCoreLoadHeap->PeekMaximum() != NULL)
 		|| (core != -1
@@ -52,7 +54,9 @@ try_small_task_packing(Thread* thread)
 static int32
 choose_small_task_core(void)
 {
+	ReadSpinLocker locker(gCoreHeapsLock);
 	CoreEntry* candidate = gCoreLoadHeap->PeekMaximum();
+	locker.Unlock();
 	if (candidate == NULL)
 		return sSmallTaskCore;
 
@@ -64,6 +68,32 @@ choose_small_task_core(void)
 }
 
 
+static CoreEntry*
+choose_idle_core(void)
+{
+	PackageEntry* current = NULL;
+	for (int32 i = 0; i < gPackageCount; i++) {
+		if (gPackageEntries[i].fIdleCoreCount != 0 && (current == NULL
+				|| gPackageEntries[i].fIdleCoreCount
+					< current->fIdleCoreCount)) {
+			current = &gPackageEntries[i];
+		}
+	}
+
+	if (current == NULL) {
+		SpinLocker _(gIdlePackageLock);
+		current = gIdlePackageList->Last();
+	}
+
+	if (current != NULL) {
+		SpinLocker _(current->fCoreLock);
+		return current->fIdleCores.Last();
+	}
+
+	return NULL;
+}
+
+
 static int32
 choose_core(Thread* thread)
 {
@@ -72,23 +102,23 @@ choose_core(Thread* thread)
 	if (try_small_task_packing(thread)) {
 		// try to pack all threads on one core
 		entry = &gCoreEntries[choose_small_task_core()];
-	} else if (gCoreLoadHeap->PeekMinimum() != NULL) {
+	} else {
+		ReadSpinLocker coreLocker(gCoreHeapsLock);
+		if (gCoreLoadHeap->PeekMinimum() != NULL) {
 			// run immediately on already woken core
 			entry = gCoreLoadHeap->PeekMinimum();
-	} else if (gPackageUsageHeap->PeekMinimum() != NULL) {
-		// wake new core
-		PackageEntry* package = gPackageUsageHeap->PeekMinimum();
-		entry = package->fIdleCores.Last();
-	} else if (gIdlePackageList->Last() != NULL) {
-		// wake new package
-		PackageEntry* package = gIdlePackageList->Last();
-		entry = package->fIdleCores.Last();
 		} else {
-		// no idle cores, use least occupied core
+			coreLocker.Unlock();
+
+			entry = choose_idle_core();
+
+			coreLocker.Lock();
+			if (entry == NULL)
 				entry = gCoreLoadHeap->PeekMinimum();
 			if (entry == NULL)
 				entry = gCoreHighLoadHeap->PeekMinimum();
 		}
+	}
 
 	ASSERT(entry != NULL);
 	return entry->fCoreID;
@@ -110,26 +140,26 @@ should_rebalance(Thread* thread)
 	CoreEntry* coreEntry = &gCoreEntries[core];
 
 	if (coreEntry->fLoad > kHighLoad) {
-		SpinLocker coreLocker(gCoreHeapsLock);
+		ReadSpinLocker coreLocker(gCoreHeapsLock);
 		if (sSmallTaskCore == core) {
-			CoreEntry* other = gCoreLoadHeap->PeekMaximum();
+			if (coreEntry->fLoad - schedulerThreadData->load < kHighLoad)
-				
-			if (other == NULL)
-				sSmallTaskCore = -1;
-			else if (coreEntry->fLoad - schedulerThreadData->load < kHighLoad)
 				return true;
-			else 
+
-				sSmallTaskCore = other->fCoreID;
+			choose_small_task_core();
 			return coreEntry->fLoad > kVeryHighLoad;
 		}
 
-		CoreEntry* other = gCoreHighLoadHeap->PeekMinimum();
+		CoreEntry* other = gCoreLoadHeap->PeekMaximum();
 		if (other == NULL)
-			other = gCoreHighLoadHeap->PeekMaximum();
+			other = gCoreHighLoadHeap->PeekMinimum();
+		ASSERT(other != NULL);
 		return coreEntry->fLoad - other->fLoad >= kLoadDifference / 2;
 	}
 
-	return choose_small_task_core() != core;
+	int32 smallTaskCore = choose_small_task_core();
+	if (smallTaskCore == -1)
+		return false;
+	return smallTaskCore != core;
 }
 
 
@@ -144,7 +174,7 @@ pack_irqs(void)
 		irq_assignment* irq = (irq_assignment*)list_get_first_item(&cpu->irqs);
 		locker.Unlock();
 
-		SpinLocker coreLocker(gCoreHeapsLock);
+		ReadSpinLocker coreLocker(gCoreHeapsLock);
 		int32 newCPU
 			= gCPUPriorityHeaps[sSmallTaskCore].PeekMinimum()->fCPUNumber;
 		coreLocker.Unlock();
@@ -185,12 +215,14 @@ rebalance_irqs(bool idle)
 	if (chosen == NULL || chosen->load < kLowLoad)
 		return;
 
-	SpinLocker coreLocker(gCoreHeapsLock);
+	ReadSpinLocker coreLocker(gCoreHeapsLock);
 	CoreEntry* other = gCoreLoadHeap->PeekMinimum();
+	coreLocker.Unlock();
 	if (other == NULL)
 		return;
+	SpinLocker cpuLocker(other->fCPULock);
 	int32 newCPU = gCPUPriorityHeaps[other->fCoreID].PeekMinimum()->fCPUNumber;
-	coreLocker.Unlock();
+	cpuLocker.Unlock();
 
 	int32 thisCore = gCPUToCore[smp_get_current_cpu()];
 	if (other->fCoreID == thisCore)

src/system/kernel/scheduler/scheduler.cpp

@@ -60,12 +60,12 @@ CPUHeap* gCPUPriorityHeaps;
 CoreEntry* gCoreEntries;
 CoreLoadHeap* gCoreLoadHeap;
 CoreLoadHeap* gCoreHighLoadHeap;
-spinlock gCoreHeapsLock = B_SPINLOCK_INITIALIZER;
+rw_spinlock gCoreHeapsLock = B_RW_SPINLOCK_INITIALIZER;
 
 PackageEntry* gPackageEntries;
-PackageHeap* gPackageUsageHeap;
 IdlePackageList* gIdlePackageList;
-spinlock gIdlePackageLock = B_SPINLOCK_INITIALIZER;
+spinlock gIdlePackageLock;
+int32 gPackageCount = B_SPINLOCK_INITIALIZER;
 
 ThreadRunQueue* gRunQueues;
 ThreadRunQueue* gPinnedRunQueues;
@@ -101,7 +101,6 @@ public:
 
 static CPUHeap* sDebugCPUHeap;
 static CoreLoadHeap* sDebugCoreHeap;
-static PackageHeap* sDebugPackageHeap;
 
 
 CPUEntry::CPUEntry()
@@ -120,7 +119,8 @@ CoreEntry::CoreEntry()
 	fActiveTime(0),
 	fLoad(0)
 {
-	B_INITIALIZE_SPINLOCK(&fLock);
+	B_INITIALIZE_SPINLOCK(&fCPULock);
+	B_INITIALIZE_SPINLOCK(&fQueueLock);
 }
 
 
@@ -129,6 +129,7 @@ PackageEntry::PackageEntry()
 	fIdleCoreCount(0),
 	fCoreCount(0)
 {
+	B_INITIALIZE_SPINLOCK(&fCoreLock);
 }
 
 
@@ -349,44 +350,6 @@ dump_idle_cores(int argc, char** argv)
 	} else
 		kprintf("No idle packages.\n");
 
-	kprintf("\nPackages with idle cores:\n");
-
-	PackageEntry* entry = gPackageUsageHeap->PeekMinimum();
-	if (entry == NULL)
-		kprintf("No packages.\n");
-	else
-		kprintf("package count cores\n");
-
-	while (entry != NULL) {
-		kprintf("%-7" B_PRId32 " %-5" B_PRId32 " ", entry->fPackageID,
-			entry->fIdleCoreCount);
-
-		DoublyLinkedList<CoreEntry>::ReverseIterator iterator
-			= entry->fIdleCores.GetReverseIterator();
-		if (iterator.HasNext()) {
-			while (iterator.HasNext()) {
-				CoreEntry* coreEntry = iterator.Next();
-				kprintf("%" B_PRId32 "%s", coreEntry->fCoreID,
-					iterator.HasNext() ? ", " : "");
-			}
-		} else
-			kprintf("-");
-		kprintf("\n");
-
-		gPackageUsageHeap->RemoveMinimum();
-		sDebugPackageHeap->Insert(entry, entry->fIdleCoreCount);
-
-		entry = gPackageUsageHeap->PeekMinimum();
-	}
-
-	entry = sDebugPackageHeap->PeekMinimum();
-	while (entry != NULL) {
-		int32 key = PackageHeap::GetKey(entry);
-		sDebugPackageHeap->RemoveMinimum();
-		gPackageUsageHeap->Insert(entry, key);
-		entry = sDebugPackageHeap->PeekMinimum();
-	}
-
 	return 0;
 }
 
@@ -437,7 +400,7 @@ update_load_heaps(int32 core)
 
 	CoreEntry* entry = &gCoreEntries[core];
 
-	SpinLocker coreLocker(gCoreHeapsLock);
+	WriteSpinLocker coreLocker(gCoreHeapsLock);
 
 	int32 cpuPerCore = smp_get_num_cpus() / gRunQueueCount;
 	int32 newKey = entry->fLoad / cpuPerCore;
@@ -512,6 +475,8 @@ update_cpu_priority(int32 cpu, int32 priority)
 {
 	int32 core = gCPUToCore[cpu];
 
+	SpinLocker coreLocker(gCoreEntries[core].fCPULock);
+
 	int32 corePriority = CPUHeap::GetKey(gCPUPriorityHeaps[core].PeekMaximum());
 
 	gCPUEntries[cpu].fPriority = priority;
@@ -529,7 +494,7 @@ update_cpu_priority(int32 cpu, int32 priority)
 	int32 package = gCPUToPackage[cpu];
 	PackageEntry* packageEntry = &gPackageEntries[package];
 	if (maxPriority == B_IDLE_PRIORITY) {
-		SpinLocker _(gIdlePackageLock);
+		SpinLocker _(packageEntry->fCoreLock);
 
 		// core goes idle
 		ASSERT(packageEntry->fIdleCoreCount >= 0);
@@ -538,27 +503,13 @@ update_cpu_priority(int32 cpu, int32 priority)
 		packageEntry->fIdleCoreCount++;
 		packageEntry->fIdleCores.Add(&gCoreEntries[core]);
 
-		if (packageEntry->fIdleCoreCount == 1) {
+		if (packageEntry->fIdleCoreCount == packageEntry->fCoreCount) {
-			// first core on that package to go idle
-
-			if (packageEntry->fCoreCount > 1)
-				gPackageUsageHeap->Insert(packageEntry, 1);
-			else
-				gIdlePackageList->Add(packageEntry);
-		} else if (packageEntry->fIdleCoreCount
-			== packageEntry->fCoreCount) {
 			// package goes idle
-			gPackageUsageHeap->ModifyKey(packageEntry, 0);
+			SpinLocker _(gIdlePackageLock);
-			ASSERT(gPackageUsageHeap->PeekMinimum() == packageEntry);
-			gPackageUsageHeap->RemoveMinimum();
-
 			gIdlePackageList->Add(packageEntry);
-		} else {
-			gPackageUsageHeap->ModifyKey(packageEntry,
-				packageEntry->fIdleCoreCount);
 		}
 	} else if (corePriority == B_IDLE_PRIORITY) {
-		SpinLocker _(gIdlePackageLock);
+		SpinLocker _(packageEntry->fCoreLock);
 
 		// core wakes up
 		ASSERT(packageEntry->fIdleCoreCount > 0);
@@ -569,20 +520,8 @@ update_cpu_priority(int32 cpu, int32 priority)
 
 		if (packageEntry->fIdleCoreCount + 1 == packageEntry->fCoreCount) {
 			// package wakes up
+			SpinLocker _(gIdlePackageLock);
 			gIdlePackageList->Remove(packageEntry);
-
-			if (packageEntry->fIdleCoreCount > 0) {
-				gPackageUsageHeap->Insert(packageEntry,
-					packageEntry->fIdleCoreCount);
-			}
-		} else if (packageEntry->fIdleCoreCount == 0) {
-			// no more idle cores in the package
-			gPackageUsageHeap->ModifyKey(packageEntry, 0);
-			ASSERT(gPackageUsageHeap->PeekMinimum() == packageEntry);
-			gPackageUsageHeap->RemoveMinimum();
-		} else {
-			gPackageUsageHeap->ModifyKey(packageEntry,
-				packageEntry->fIdleCoreCount);
 		}
 	}
 }
@@ -599,6 +538,7 @@ choose_core(Thread* thread)
 static inline int32
 choose_cpu(int32 core)
 {
+	SpinLocker cpuLocker(gCoreEntries[core].fCPULock);
 	CPUEntry* entry = gCPUPriorityHeaps[core].PeekMinimum();
 	ASSERT(entry != NULL);
 	return entry->fCPUNumber;
@@ -608,8 +548,6 @@ choose_cpu(int32 core)
 static bool
 choose_core_and_cpu(Thread* thread, int32& targetCore, int32& targetCPU)
 {
-	SpinLocker coreLocker(gCoreHeapsLock);
-
 	if (targetCore == -1 && targetCPU != -1)
 		targetCore = gCPUToCore[targetCPU];
 	else if (targetCore != -1 && targetCPU == -1)
@@ -779,7 +717,7 @@ enqueue(Thread* thread, bool newOne)
 	TRACE("enqueueing thread %ld with priority %ld on CPU %ld (core %ld)\n",
 		thread->id, threadPriority, targetCPU, targetCore);
 
-	SpinLocker runQueueLocker(gCoreEntries[targetCore].fLock);
+	SpinLocker runQueueLocker(gCoreEntries[targetCore].fQueueLock);
 	thread->scheduler_data->enqueued = true;
 	if (pinned)
 		gPinnedRunQueues[targetCPU].PushBack(thread, threadPriority);
@@ -832,7 +770,7 @@ put_back(Thread* thread)
 
 	int32 core = gCPUToCore[smp_get_current_cpu()];
 
-	SpinLocker runQueueLocker(gCoreEntries[core].fLock);
+	SpinLocker runQueueLocker(gCoreEntries[core].fQueueLock);
 	thread->scheduler_data->enqueued = true;
 	if (thread->pinned_to_cpu > 0) {
 		int32 pinnedCPU = thread->previous_cpu->cpu_num;
@@ -875,10 +813,8 @@ scheduler_set_thread_priority(Thread *thread, int32 priority)
 		cancel_penalty(thread);
 		thread->priority = priority;
 
-		if (thread->state == B_THREAD_RUNNING) {
+		if (thread->state == B_THREAD_RUNNING)
-			SpinLocker coreLocker(gCoreHeapsLock);
 			update_cpu_priority(thread->cpu->cpu_num, priority);
-		}
 		return oldPriority;
 	}
 
@@ -890,7 +826,7 @@ scheduler_set_thread_priority(Thread *thread, int32 priority)
 	int32 previougCore = thread->scheduler_data->previous_core;
 	ASSERT(previougCore >= 0);
 
-	SpinLocker runQueueLocker(gCoreEntries[previougCore].fLock);
+	SpinLocker runQueueLocker(gCoreEntries[previougCore].fQueueLock);
 
 	// the thread might have been already dequeued and is about to start
 	// running once we release its scheduler_lock, in such case we can not
@@ -1022,7 +958,7 @@ choose_next_thread(int32 thisCPU, Thread* oldThread, bool putAtBack)
 {
 	int32 thisCore = gCPUToCore[thisCPU];
 
-	SpinLocker runQueueLocker(gCoreEntries[thisCore].fLock);
+	SpinLocker runQueueLocker(gCoreEntries[thisCore].fQueueLock);
 
 	Thread* sharedThread = gRunQueues[thisCore].PeekMaximum();
 	Thread* pinnedThread = gPinnedRunQueues[thisCPU].PeekMaximum();
@@ -1234,10 +1170,7 @@ _scheduler_reschedule(void)
 		oldThread, nextThread);
 
 	// update CPU heap
-	{
-		SpinLocker coreLocker(gCoreHeapsLock);
 	update_cpu_priority(thisCPU, get_effective_priority(nextThread));
-	}
 
 	nextThread->state = B_THREAD_RUNNING;
 	nextThread->next_state = B_THREAD_READY;
@@ -1440,13 +1373,7 @@ create_debug_heaps()
 	sDebugCoreHeap = new(std::nothrow) CoreLoadHeap(smp_get_num_cpus());
 	if (sDebugCoreHeap == NULL)
 		return B_NO_MEMORY;
-	ObjectDeleter<CoreLoadHeap> coreDeleter(sDebugCoreHeap);
 
-	sDebugPackageHeap = new(std::nothrow) PackageHeap(smp_get_num_cpus());
-	if (sDebugPackageHeap == NULL)
-		return B_NO_MEMORY;
-
-	coreDeleter.Detach();
 	cpuDeleter.Detach();
 	return B_OK;
 }
@@ -1463,6 +1390,7 @@ _scheduler_init()
 		return result;
 	gRunQueueCount = coreCount;
 	gSingleCore = coreCount == 1;
+	gPackageCount = packageCount;
 
 	// create package heap and idle package stack
 	gPackageEntries = new(std::nothrow) PackageEntry[packageCount];
@@ -1470,11 +1398,6 @@ _scheduler_init()
 		return B_NO_MEMORY;
 	ArrayDeleter<PackageEntry> packageEntriesDeleter(gPackageEntries);
 
-	gPackageUsageHeap = new(std::nothrow) PackageHeap(packageCount);
-	if (gPackageUsageHeap == NULL)
-		return B_NO_MEMORY;
-	ObjectDeleter<PackageHeap> packageHeapDeleter(gPackageUsageHeap);
-
 	gIdlePackageList = new(std::nothrow) IdlePackageList;
 	if (gIdlePackageList == NULL)
 		return B_NO_MEMORY;
@@ -1589,7 +1512,6 @@ _scheduler_init()
 	coreEntriesDeleter.Detach();
 	cpuEntriesDeleter.Detach();
 	packageEntriesDeleter.Detach();
-	packageHeapDeleter.Detach();
 	packageListDeleter.Detach();
 	return B_OK;
 }

src/system/kernel/scheduler/scheduler_common.h

@@ -79,7 +79,8 @@ struct CoreEntry : public MinMaxHeapLinkImpl<CoreEntry, int32>,
 
 	int32		fCoreID;
 
-	spinlock	fLock;
+	spinlock	fCPULock;
+	spinlock	fQueueLock;
 
 	bigtime_t	fStartedBottom;
 	bigtime_t	fReachedBottom;
@@ -95,9 +96,9 @@ typedef MinMaxHeap<CoreEntry, int32> CoreLoadHeap;
 extern CoreEntry* gCoreEntries;
 extern CoreLoadHeap* gCoreLoadHeap;
 extern CoreLoadHeap* gCoreHighLoadHeap;
-extern spinlock gCoreHeapsLock;
+extern rw_spinlock gCoreHeapsLock;
 
-// sPackageUsageHeap is used to decide which core should be woken up from the
+// gPackageEntries are used to decide which core should be woken up from the
 // idle state. When aiming for performance we should use as many packages as
 // possible with as little cores active in each package as possible (so that the
 // package can enter any boost mode if it has one and the active core have more
@@ -106,24 +107,24 @@ extern spinlock gCoreHeapsLock;
 // packages can go to the deep state of sleep). The heap stores only packages
 // with at least one core active and one core idle. The packages with all cores
 // idle are stored in sPackageIdleList (in LIFO manner).
-struct PackageEntry : public MinMaxHeapLinkImpl<PackageEntry, int32>,
+struct PackageEntry : public DoublyLinkedListLinkImpl<PackageEntry> {
-	DoublyLinkedListLinkImpl<PackageEntry> {
 								PackageEntry();
 
 	int32						fPackageID;
 
+	spinlock					fCoreLock;
+
 	DoublyLinkedList<CoreEntry>	fIdleCores;
 	int32						fIdleCoreCount;
 
 	int32						fCoreCount;
 } CACHE_LINE_ALIGN;
-typedef MinMaxHeap<PackageEntry, int32> PackageHeap;
 typedef DoublyLinkedList<PackageEntry> IdlePackageList;
 
 extern PackageEntry* gPackageEntries;
-extern PackageHeap* gPackageUsageHeap;
 extern IdlePackageList* gIdlePackageList;
 extern spinlock gIdlePackageLock;
+extern int32 gPackageCount;
 
 // The run queues. Holds the threads ready to run ordered by priority.
 // One queue per schedulable target per core. Additionally, each