LadybirdBrowser/ladybird
1
1
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2024-12-30 22:54:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

Kernel: Fix SMP deadlock in MM::allocate_contiguous_physical_pages

Browse Source 
This deadlock was introduced with the creation of this API. The lock
order is such that we always need to take the page directory lock
before we ever take the MM lock.

This function violated that, as both Region creation and region
destruction require the pd and mm locks, but with the mm lock
already acquired we deadlocked with SMP mode enabled while other
threads were allocating regions.

With this change SMP boots to the desktop successfully for me,
(and then subsequently has other issues). :^)
This commit is contained in:
Brian Gianforcaro 2022-08-08 21:50:14 -07:00 committed by Andreas Kling
parent d11ce1d808
commit 2d06f6399f
Notes: sideshowbarker 2024-07-17 08:21:15 +09:00 
Author: https://github.com/bgianfo
Commit: https://github.com/SerenityOS/serenity/commit/2d06f6399f
Pull-request: https://github.com/SerenityOS/serenity/pull/14812
1 changed files with 7 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Kernel/Memory/MemoryManager.cpp
View File

@ -949,7 +949,7 @@ ErrorOr<NonnullRefPtr<PhysicalPage>> MemoryManager::allocate_physical_page(Shoul
ErrorOr<NonnullRefPtrVector<PhysicalPage>> MemoryManager::allocate_contiguous_physical_pages(size_t size)
{
VERIFY(!(size % PAGE_SIZE));
SpinlockLocker lock(s_mm_lock);
SpinlockLocker mm_lock(s_mm_lock);
size_t page_count = ceil_div(size, static_cast<size_t>(PAGE_SIZE));
// We need to make sure we don't touch pages that we have committed to
@ -959,6 +959,12 @@ ErrorOr<NonnullRefPtrVector<PhysicalPage>> MemoryManager::allocate_contiguous_ph
for (auto& physical_region : m_physical_regions) {
auto physical_pages = physical_region.take_contiguous_free_pages(page_count);
if (!physical_pages.is_empty()) {
// Note: The locking semantics are a bit awkward here, the region destructor can
// deadlock due to invalid lock ordering if we still hold the s_mm_lock when the
// region is constructed or destructed. Since we are now done enumerating anyway,
// it's safe to just unlock here and have MM.allocate_kernel_region do the right thing.
mm_lock.unlock();
{
auto cleanup_region = TRY(MM.allocate_kernel_region(physical_pages[0].paddr(), PAGE_SIZE * page_count, "MemoryManager Allocation Sanitization"sv, Region::Access::Read | Region::Access::Write));
memset(cleanup_region->vaddr().as_ptr(), 0, PAGE_SIZE * page_count);