From 65741c8b56b2fc74e8e1cd419b92f799e75f556d Mon Sep 17 00:00:00 2001 From: Pawel Dziepak Date: Fri, 22 Nov 2013 03:00:08 +0100 Subject: [PATCH] scheduler: Improve locking --- src/system/kernel/scheduler/low_latency.cpp | 52 ++++++-- src/system/kernel/scheduler/power_saving.cpp | 90 ++++++++----- src/system/kernel/scheduler/scheduler.cpp | 122 ++++-------------- .../kernel/scheduler/scheduler_common.h | 15 ++- 4 files changed, 130 insertions(+), 149 deletions(-) diff --git a/src/system/kernel/scheduler/low_latency.cpp b/src/system/kernel/scheduler/low_latency.cpp index fa534ad76f..9c264998ad 100644 --- a/src/system/kernel/scheduler/low_latency.cpp +++ b/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) { - // wake new package - PackageEntry* package = gIdlePackageList->Last(); - entry = package->fIdleCores.Last(); - } else if (gPackageUsageHeap->PeekMaximum() != NULL) { + SpinLocker locker(gIdlePackageLock); + // wake new package + PackageEntry* package = gIdlePackageList->Last(); + if (package == 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()]; diff --git a/src/system/kernel/scheduler/power_saving.cpp b/src/system/kernel/scheduler/power_saving.cpp index ac848e9ab0..d054f56575 100644 --- a/src/system/kernel/scheduler/power_saving.cpp +++ b/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,22 +102,22 @@ 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) { - // 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 - entry = gCoreLoadHeap->PeekMinimum(); - if (entry == NULL) - entry = gCoreHighLoadHeap->PeekMinimum(); + ReadSpinLocker coreLocker(gCoreHeapsLock); + if (gCoreLoadHeap->PeekMinimum() != NULL) { + // run immediately on already woken core + entry = gCoreLoadHeap->PeekMinimum(); + } else { + coreLocker.Unlock(); + + entry = choose_idle_core(); + + coreLocker.Lock(); + if (entry == NULL) + entry = gCoreLoadHeap->PeekMinimum(); + if (entry == NULL) + entry = gCoreHighLoadHeap->PeekMinimum(); + } } ASSERT(entry != NULL); @@ -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 (other == NULL) - sSmallTaskCore = -1; - else if (coreEntry->fLoad - schedulerThreadData->load < kHighLoad) + 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) diff --git a/src/system/kernel/scheduler/scheduler.cpp b/src/system/kernel/scheduler/scheduler.cpp index e64330dda5..ecd8f7d0c6 100644 --- a/src/system/kernel/scheduler/scheduler.cpp +++ b/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::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) { - // 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) { + if (packageEntry->fIdleCoreCount == packageEntry->fCoreCount) { // package goes idle - gPackageUsageHeap->ModifyKey(packageEntry, 0); - ASSERT(gPackageUsageHeap->PeekMinimum() == packageEntry); - gPackageUsageHeap->RemoveMinimum(); - + SpinLocker _(gIdlePackageLock); 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) { - SpinLocker coreLocker(gCoreHeapsLock); + if (thread->state == B_THREAD_RUNNING) 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)); - } + 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 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 packageEntriesDeleter(gPackageEntries); - gPackageUsageHeap = new(std::nothrow) PackageHeap(packageCount); - if (gPackageUsageHeap == NULL) - return B_NO_MEMORY; - ObjectDeleter 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; } diff --git a/src/system/kernel/scheduler/scheduler_common.h b/src/system/kernel/scheduler/scheduler_common.h index ea3aa422ad..7340e93320 100644 --- a/src/system/kernel/scheduler/scheduler_common.h +++ b/src/system/kernel/scheduler/scheduler_common.h @@ -79,7 +79,8 @@ struct CoreEntry : public MinMaxHeapLinkImpl, int32 fCoreID; - spinlock fLock; + spinlock fCPULock; + spinlock fQueueLock; bigtime_t fStartedBottom; bigtime_t fReachedBottom; @@ -95,9 +96,9 @@ typedef MinMaxHeap 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, - DoublyLinkedListLinkImpl { +struct PackageEntry : public DoublyLinkedListLinkImpl { PackageEntry(); int32 fPackageID; + spinlock fCoreLock; + DoublyLinkedList fIdleCores; int32 fIdleCoreCount; int32 fCoreCount; } CACHE_LINE_ALIGN; -typedef MinMaxHeap PackageHeap; typedef DoublyLinkedList 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