From 72cdc62155be8af67ff9328bc4226a69bbbb659c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 5 Nov 2018 10:23:00 +0100 Subject: [PATCH] Replace zones with individually tracked physical pages. It's just a simple struct { ref_count, paddr }. This will allow me to implement lazy zeroing and COW pages. --- AK/RetainPtr.h | 3 + AK/Vector.h | 17 ++++- Kernel/MemoryManager.cpp | 149 ++++++++++++++++++-------------------- Kernel/MemoryManager.h | 64 +++++++++------- Kernel/ProcFileSystem.cpp | 12 ++- Kernel/Process.cpp | 23 ++---- Kernel/i386.cpp | 2 +- Kernel/sync.sh | 1 + Userland/.gitignore | 1 + Userland/ft2.cpp | 16 ++++ 10 files changed, 161 insertions(+), 127 deletions(-) create mode 100644 Userland/ft2.cpp diff --git a/AK/RetainPtr.h b/AK/RetainPtr.h index b2fc6c7d2a8..b5c7c370a28 100644 --- a/AK/RetainPtr.h +++ b/AK/RetainPtr.h @@ -32,6 +32,7 @@ public: RetainPtr(RetainPtr& other) : m_ptr(other.copyRef().leakRef()) { } RetainPtr(RetainPtr&& other) : m_ptr(other.leakRef()) { } template RetainPtr(RetainPtr&& other) : m_ptr(static_cast(other.leakRef())) { } + RetainPtr(const RetainPtr& other) : m_ptr(const_cast(other).copyRef().leakRef()) { } ~RetainPtr() { clear(); @@ -121,6 +122,8 @@ public: operator bool() { return !!m_ptr; } + bool is_null() const { return !m_ptr; } + private: T* m_ptr = nullptr; }; diff --git a/AK/Vector.h b/AK/Vector.h index 6b3f7f9070d..f57496533e0 100644 --- a/AK/Vector.h +++ b/AK/Vector.h @@ -75,6 +75,13 @@ public: other.m_impl = nullptr; } + Vector(const Vector& other) + { + ensureCapacity(other.size()); + for (size_t i = 0; i < other.size(); ++i) + unchecked_append(other[i]); + } + Vector& operator=(Vector&& other) { if (this != &other) { @@ -140,16 +147,22 @@ public: Vector tmp = move(other); ensureCapacity(size() + tmp.size()); for (auto&& v : tmp) { - uncheckedAppend(move(v)); + unchecked_append(move(v)); } } - void uncheckedAppend(T&& value) + void unchecked_append(T&& value) { new (m_impl->slot(m_impl->m_size)) T(move(value)); ++m_impl->m_size; } + void unchecked_append(const T& value) + { + new (m_impl->slot(m_impl->m_size)) T(value); + ++m_impl->m_size; + } + void append(T&& value) { ensureCapacity(size() + 1); diff --git a/Kernel/MemoryManager.cpp b/Kernel/MemoryManager.cpp index 8ab31a64e3d..c6fc69ab1d6 100644 --- a/Kernel/MemoryManager.cpp +++ b/Kernel/MemoryManager.cpp @@ -45,12 +45,12 @@ void MemoryManager::release_page_directory(PageDirectory& page_directory) dbgprintf("MM: release_page_directory for PD K%x\n", &page_directory); #endif for (size_t i = 0; i < 1024; ++i) { - auto page_table = page_directory.physical_addresses[i]; + auto& page_table = page_directory.physical_pages[i]; if (!page_table.is_null()) { #ifdef MM_DEBUG - dbgprintf("MM: deallocating process page table [%u] P%x @ %p\n", i, page_table.get(), &process.m_page_directory->physical_addresses[i]); + dbgprintf("MM: deallocating user page table P%x\n", page_table->paddr().get()); #endif - deallocate_page_table(page_table); + deallocate_page_table(page_directory, i); } } #ifdef SCRUB_DEALLOCATED_PAGE_TABLES @@ -76,9 +76,9 @@ void MemoryManager::initializePaging() // The bottom 4 MB are identity mapped & supervisor only. Every process shares these mappings. create_identity_mapping(LinearAddress(PAGE_SIZE), 4 * MB); - // The physical pages 4 MB through 8 MB are available for Zone allocation. + // The physical pages 4 MB through 8 MB are available for allocation. for (size_t i = (4 * MB) + PAGE_SIZE; i < (8 * MB); i += PAGE_SIZE) - m_freePages.append(PhysicalAddress(i)); + m_free_physical_pages.append(adopt(*new PhysicalPage(PhysicalAddress(i)))); asm volatile("movl %%eax, %%cr3"::"a"(m_kernel_page_directory)); asm volatile( @@ -88,20 +88,29 @@ void MemoryManager::initializePaging() ); } -PhysicalAddress MemoryManager::allocate_page_table() +RetainPtr MemoryManager::allocate_page_table(PageDirectory& page_directory, unsigned index) { - auto ppages = allocatePhysicalPages(1); - dword address = ppages[0].get(); + auto& page_directory_physical_ptr = page_directory.physical_pages[index]; + ASSERT(!page_directory_physical_ptr); + auto ppages = allocate_physical_pages(1); + ASSERT(ppages.size() == 1); + dword address = ppages[0]->paddr().get(); create_identity_mapping(LinearAddress(address), PAGE_SIZE); memset((void*)address, 0, PAGE_SIZE); - return PhysicalAddress(address); + page_directory.physical_pages[index] = move(ppages[0]); + return page_directory.physical_pages[index]; } -void MemoryManager::deallocate_page_table(PhysicalAddress paddr) +void MemoryManager::deallocate_page_table(PageDirectory& page_directory, unsigned index) { - ASSERT(!m_freePages.contains_slow(paddr)); - remove_identity_mapping(LinearAddress(paddr.get()), PAGE_SIZE); - m_freePages.append(paddr); + auto& physical_page = page_directory.physical_pages[index]; + ASSERT(physical_page); + ASSERT(!m_free_physical_pages.contains_slow(physical_page)); + for (size_t i = 0; i < MM.m_free_physical_pages.size(); ++i) { + ASSERT(MM.m_free_physical_pages[i].ptr() != physical_page.ptr()); + } + remove_identity_mapping(LinearAddress(physical_page->paddr().get()), PAGE_SIZE); + page_directory.physical_pages[index] = nullptr; } void MemoryManager::remove_identity_mapping(LinearAddress laddr, size_t size) @@ -143,20 +152,21 @@ auto MemoryManager::ensurePTE(PageDirectory* page_directory, LinearAddress laddr pde.setPresent(true); pde.setWritable(true); } else { - auto page_table = allocate_page_table(); + auto page_table = allocate_page_table(*page_directory, page_directory_index); #ifdef MM_DEBUG dbgprintf("MM: PD K%x (%s) allocated page table #%u (for L%x) at P%x\n", page_directory, page_directory == m_kernel_page_directory ? "Kernel" : "User", page_directory_index, laddr.get(), - page_table); + page_table->paddr().get()); #endif - page_directory->physical_addresses[page_directory_index] = page_table; - pde.setPageTableBase(page_table.get()); + + pde.setPageTableBase(page_table->paddr().get()); pde.setUserAllowed(true); pde.setPresent(true); pde.setWritable(true); + page_directory->physical_pages[page_directory_index] = move(page_table); } } return PageTableEntry(&pde.pageTableBase()[page_table_index]); @@ -209,61 +219,18 @@ PageFaultResponse MemoryManager::handlePageFault(const PageFault& fault) return PageFaultResponse::ShouldCrash; } -void MemoryManager::registerZone(Zone& zone) -{ - ASSERT_INTERRUPTS_DISABLED(); - m_zones.set(&zone); -#ifdef MM_DEBUG - for (size_t i = 0; i < zone.m_pages.size(); ++i) - dbgprintf("MM: allocated to zone: P%x\n", zone.m_pages[i].get()); -#endif -} - -void MemoryManager::unregisterZone(Zone& zone) -{ - ASSERT_INTERRUPTS_DISABLED(); -#ifdef MM_DEBUG - for (size_t i = 0; i < zone.m_pages.size(); ++i) - dbgprintf("MM: deallocated from zone: P%x\n", zone.m_pages[i].get()); -#endif - m_zones.remove(&zone); - m_freePages.append(move(zone.m_pages)); -} - -Zone::Zone(Vector&& pages) - : m_pages(move(pages)) -{ - MM.registerZone(*this); -} - -Zone::~Zone() -{ - MM.unregisterZone(*this); -} - -RetainPtr MemoryManager::createZone(size_t size) +Vector> MemoryManager::allocate_physical_pages(size_t count) { InterruptDisabler disabler; - auto pages = allocatePhysicalPages(ceilDiv(size, PAGE_SIZE)); - if (pages.isEmpty()) { - kprintf("MM: createZone: no physical pages for size %u\n", size); - return nullptr; - } - return adopt(*new Zone(move(pages))); -} - -Vector MemoryManager::allocatePhysicalPages(size_t count) -{ - InterruptDisabler disabler; - if (count > m_freePages.size()) + if (count > m_free_physical_pages.size()) return { }; - Vector pages; + Vector> pages; pages.ensureCapacity(count); for (size_t i = 0; i < count; ++i) { - pages.append(m_freePages.takeLast()); + pages.append(m_free_physical_pages.takeLast()); #ifdef MM_DEBUG - dbgprintf("MM: allocate_physical_pages vending P%x\n", pages.last()); + dbgprintf("MM: allocate_physical_pages vending P%x\n", pages.last()->paddr().get()); #endif } return pages; @@ -299,17 +266,22 @@ void MemoryManager::flushTLB(LinearAddress laddr) void MemoryManager::map_region_at_address(PageDirectory* page_directory, Region& region, LinearAddress laddr, bool user_allowed) { InterruptDisabler disabler; - auto& zone = *region.zone; - for (size_t i = 0; i < zone.m_pages.size(); ++i) { + for (size_t i = 0; i < region.physical_pages.size(); ++i) { auto page_laddr = laddr.offset(i * PAGE_SIZE); auto pte = ensurePTE(page_directory, page_laddr); - pte.setPhysicalPageBase(zone.m_pages[i].get()); - pte.setPresent(true); // FIXME: Maybe we could use the is_readable flag here? + auto& physical_page = region.physical_pages[i]; + if (physical_page) { + pte.setPhysicalPageBase(physical_page->paddr().get()); + pte.setPresent(true); // FIXME: Maybe we should use the is_readable flag here? + } else { + pte.setPhysicalPageBase(0); + pte.setPresent(false); + } pte.setWritable(region.is_writable); pte.setUserAllowed(user_allowed); flushTLB(page_laddr); #ifdef MM_DEBUG - dbgprintf("MM: >> map_region_at_address (PD=%x) L%x => P%x\n", page_directory, page_laddr, zone.m_pages[i].get()); + dbgprintf("MM: >> map_region_at_address (PD=%x) '%s' L%x => P%x (@%p)\n", page_directory, region.name.characters(), page_laddr, physical_page ? physical_page->paddr().get() : 0, physical_page.ptr()); #endif } } @@ -369,8 +341,7 @@ void MemoryManager::remove_kernel_alias_for_region(Region& region, byte* addr) bool MemoryManager::unmapRegion(Process& process, Region& region) { InterruptDisabler disabler; - auto& zone = *region.zone; - for (size_t i = 0; i < zone.m_pages.size(); ++i) { + for (size_t i = 0; i < region.physical_pages.size(); ++i) { auto laddr = region.linearAddress.offset(i * PAGE_SIZE); auto pte = ensurePTE(process.m_page_directory, laddr); pte.setPhysicalPageBase(0); @@ -379,7 +350,8 @@ bool MemoryManager::unmapRegion(Process& process, Region& region) pte.setUserAllowed(false); flushTLB(laddr); #ifdef MM_DEBUG - //dbgprintf("MM: >> Unmapped L%x => P%x <<\n", laddr, zone.m_pages[i].get()); + auto& physical_page = region.physical_pages[i]; + dbgprintf("MM: >> Unmapped L%x => P%x <<\n", laddr, physical_page ? physical_page->paddr().get() : 0); #endif } return true; @@ -429,12 +401,12 @@ RetainPtr Region::clone() KernelPagingScope pagingScope; if (is_readable && !is_writable) { - // Create a new region backed by the same zone. - return adopt(*new Region(linearAddress, size, zone.copyRef(), String(name), is_readable, is_writable)); + // Create a new region backed by the same physical pages. + return adopt(*new Region(linearAddress, size, physical_pages, String(name), is_readable, is_writable)); } // FIXME: Implement COW regions. - auto clone_zone = MM.createZone(zone->size()); - auto clone_region = adopt(*new Region(linearAddress, size, move(clone_zone), String(name), is_readable, is_writable)); + auto clone_physical_pages = MM.allocate_physical_pages(physical_pages.size()); + auto clone_region = adopt(*new Region(linearAddress, size, move(clone_physical_pages), String(name), is_readable, is_writable)); // FIXME: It would be cool to make the src_alias a read-only mapping. byte* src_alias = MM.create_kernel_alias_for_region(*this); @@ -447,3 +419,26 @@ RetainPtr Region::clone() return clone_region; } +Region::Region(LinearAddress a, size_t s, Vector> pp, String&& n, bool r, bool w) + : linearAddress(a) + , size(s) + , physical_pages(move(pp)) + , name(move(n)) + , is_readable(r) + , is_writable(w) +{ +} + +Region::~Region() +{ +} + +void PhysicalPage::return_to_freelist() +{ + InterruptDisabler disabler; + m_retain_count = 1; + MM.m_free_physical_pages.append(adopt(*this)); +#ifdef MM_DEBUG + dbgprintf("MM: P%x released to freelist\n", m_paddr.get()); +#endif +} diff --git a/Kernel/MemoryManager.h b/Kernel/MemoryManager.h index 0ebe056666c..39e295afa3b 100644 --- a/Kernel/MemoryManager.h +++ b/Kernel/MemoryManager.h @@ -17,34 +17,50 @@ enum class PageFaultResponse { Continue, }; -struct PageDirectory { - dword entries[1024]; - PhysicalAddress physical_addresses[1024]; -}; - -struct Zone : public Retainable { - friend ByteBuffer procfs$mm(); +class PhysicalPage { + friend class MemoryManager; public: - ~Zone(); - size_t size() const { return m_pages.size() * PAGE_SIZE; } + ~PhysicalPage() { } + PhysicalAddress paddr() const { return m_paddr; } - const Vector& pages() const { return m_pages; } + void retain() + { + ASSERT(m_retain_count); + ++m_retain_count; + } + + void release() + { + ASSERT(m_retain_count); + if (!--m_retain_count) + return_to_freelist(); + } private: - friend class MemoryManager; - explicit Zone(Vector&&); + PhysicalPage(PhysicalAddress paddr) + : m_paddr(paddr) + { + } - Vector m_pages; + void return_to_freelist(); + + unsigned m_retain_count { 1 }; + PhysicalAddress m_paddr; +}; + +struct PageDirectory { + dword entries[1024]; + RetainPtr physical_pages[1024]; }; struct Region : public Retainable { - Region(LinearAddress, size_t, RetainPtr&&, String&&, bool r, bool w); + Region(LinearAddress, size_t, Vector>, String&&, bool r, bool w); ~Region(); RetainPtr clone(); LinearAddress linearAddress; size_t size { 0 }; - RetainPtr zone; + Vector> physical_pages; String name; bool is_readable { true }; bool is_writable { true }; @@ -54,6 +70,7 @@ struct Region : public Retainable { class MemoryManager { AK_MAKE_ETERNAL + friend class PhysicalPage; friend ByteBuffer procfs$mm(); public: static MemoryManager& the() PURE; @@ -64,14 +81,9 @@ public: PageFaultResponse handlePageFault(const PageFault&); - RetainPtr createZone(size_t); - bool mapRegion(Process&, Region&); bool unmapRegion(Process&, Region&); - void registerZone(Zone&); - void unregisterZone(Zone&); - void populate_page_directory(PageDirectory&); void release_page_directory(PageDirectory&); @@ -84,6 +96,8 @@ public: bool validate_user_read(const Process&, LinearAddress) const; bool validate_user_write(const Process&, LinearAddress) const; + Vector> allocate_physical_pages(size_t count); + private: MemoryManager(); ~MemoryManager(); @@ -96,16 +110,14 @@ private: void flushEntireTLB(); void flushTLB(LinearAddress); - PhysicalAddress allocate_page_table(); - void deallocate_page_table(PhysicalAddress); + RetainPtr allocate_page_table(PageDirectory&, unsigned index); + void deallocate_page_table(PageDirectory&, unsigned index); void protectMap(LinearAddress, size_t length); void create_identity_mapping(LinearAddress, size_t length); void remove_identity_mapping(LinearAddress, size_t); - Vector allocatePhysicalPages(size_t count); - struct PageDirectoryEntry { explicit PageDirectoryEntry(dword* pde) : m_pde(pde) { } @@ -192,9 +204,7 @@ private: LinearAddress m_next_laddr; - HashTable m_zones; - - Vector m_freePages; + Vector> m_free_physical_pages; }; struct KernelPagingScope { diff --git a/Kernel/ProcFileSystem.cpp b/Kernel/ProcFileSystem.cpp index 8ad761fbeaf..d68813eb6f9 100644 --- a/Kernel/ProcFileSystem.cpp +++ b/Kernel/ProcFileSystem.cpp @@ -49,7 +49,7 @@ ByteBuffer procfs$pid_vm(Process& process) { ProcessInspectionScope scope(process); char* buffer; - auto stringImpl = StringImpl::createUninitialized(80 + process.regionCount() * 80, buffer); + auto stringImpl = StringImpl::createUninitialized(80 + process.regionCount() * 80 + 4096, buffer); memset(buffer, 0, stringImpl->length()); char* ptr = buffer; ptr += ksprintf(ptr, "BEGIN END SIZE NAME\n"); @@ -59,6 +59,10 @@ ByteBuffer procfs$pid_vm(Process& process) region->linearAddress.offset(region->size - 1).get(), region->size, region->name.characters()); + for (auto& physical_page : region->physical_pages) { + ptr += ksprintf(ptr, "P%x ", physical_page ? physical_page->paddr().get() : 0); + } + ptr += ksprintf(ptr, "\n"); } *ptr = '\0'; return ByteBuffer::copy((byte*)buffer, ptr - buffer); @@ -130,6 +134,8 @@ void ProcFileSystem::removeProcess(Process& process) ByteBuffer procfs$mm() { + // FIXME: Implement +#if 0 InterruptDisabler disabler; size_t zonePageCount = 0; for (auto* zone : MM.m_zones) @@ -143,9 +149,11 @@ ByteBuffer procfs$mm() ptr += ksprintf(ptr, "\n"); } ptr += ksprintf(ptr, "Zone count: %u\n", MM.m_zones.size()); - ptr += ksprintf(ptr, "Free physical pages: %u\n", MM.m_freePages.size()); + ptr += ksprintf(ptr, "Free physical pages: %u\n", MM.m_free_physical_pages.size()); buffer.trim(ptr - (char*)buffer.pointer()); return buffer; +#endif + return { }; } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index d3724b7c7e0..701065d7e77 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -17,6 +17,7 @@ //#define DEBUG_IO //#define TASK_DEBUG +//#define FORK_DEBUG //#define SCHEDULER_DEBUG // FIXME: Only do a single validation for accesses that don't span multiple pages. @@ -144,11 +145,11 @@ Region* Process::allocate_region(LinearAddress laddr, size_t size, String&& name laddr.mask(0xfffff000); - auto zone = MM.createZone(size); - ASSERT(zone); - - m_regions.append(adopt(*new Region(laddr, size, move(zone), move(name), is_readable, is_writable))); + unsigned page_count = ceilDiv(size, PAGE_SIZE); + auto physical_pages = MM.allocate_physical_pages(page_count); + ASSERT(physical_pages.size() == page_count); + m_regions.append(adopt(*new Region(laddr, size, move(physical_pages), move(name), is_readable, is_writable))); MM.mapRegion(*this, *m_regions.last()); return m_regions.last().ptr(); } @@ -1258,20 +1259,6 @@ Process* Process::kernelProcess() return s_kernelProcess; } -Region::Region(LinearAddress a, size_t s, RetainPtr&& z, String&& n, bool r, bool w) - : linearAddress(a) - , size(s) - , zone(move(z)) - , name(move(n)) - , is_readable(r) - , is_writable(w) -{ -} - -Region::~Region() -{ -} - bool Process::isValidAddressForKernel(LinearAddress laddr) const { // We check extra carefully here since the first 4MB of the address space is identity-mapped. diff --git a/Kernel/i386.cpp b/Kernel/i386.cpp index 22caeb0c431..fd1bb7564ea 100644 --- a/Kernel/i386.cpp +++ b/Kernel/i386.cpp @@ -318,7 +318,7 @@ void gdt_init() s_gdt_freelist = new Vector(); s_gdt_freelist->ensureCapacity(256); for (size_t i = s_gdtLength; i < 256; ++i) - s_gdt_freelist->uncheckedAppend(i * 8); + s_gdt_freelist->unchecked_append(i * 8); s_gdtLength = 256; s_gdtr.address = s_gdt; diff --git a/Kernel/sync.sh b/Kernel/sync.sh index 3248ac1d6ef..eed85a2a43f 100755 --- a/Kernel/sync.sh +++ b/Kernel/sync.sh @@ -19,6 +19,7 @@ cp ../Userland/uname mnt/bin/uname cp ../Userland/clear mnt/bin/clear cp ../Userland/tst mnt/bin/tst cp ../Userland/ft mnt/bin/ft +cp ../Userland/ft2 mnt/bin/ft2 cp ../Userland/mm mnt/bin/mm cp ../Userland/kill mnt/bin/kill cp ../Userland/tty mnt/bin/tty diff --git a/Userland/.gitignore b/Userland/.gitignore index d116c680f7f..51ce12a3ca2 100644 --- a/Userland/.gitignore +++ b/Userland/.gitignore @@ -17,3 +17,4 @@ mm kill tty ft +ft2 diff --git a/Userland/ft2.cpp b/Userland/ft2.cpp new file mode 100644 index 00000000000..3f30ee2c953 --- /dev/null +++ b/Userland/ft2.cpp @@ -0,0 +1,16 @@ +#include +#include + +int main(int argc, char** argv) +{ + printf("Testing fork()...\n"); + pid_t pid = fork(); + if (!pid) { + printf("child, pid=%d\n", getpid()); + for (;;); + } else { + printf("parent, child pid=%d\n", pid); + for (;;); + } + return 0; +}