From dc30437ce7f80bbe936c879b5f7e742a909759e8 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Tue, 14 Feb 2023 20:29:40 -0800 Subject: [PATCH] [kernel] Remove page_table's cache counter The `s_cache_count` counter had the potential to get out of sync with the cache itself. Since we only call `fill_table_page_cache()` when the cache is empty, the counter was not useful. After chasing the bug for hours to figure out how they were getting out of sync, I just ripped it out. --- src/kernel/page_table.cpp | 35 ++++++++++++++--------------------- src/kernel/page_table.h | 1 - 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/kernel/page_table.cpp b/src/kernel/page_table.cpp index abd788a..ce4d822 100644 --- a/src/kernel/page_table.cpp +++ b/src/kernel/page_table.cpp @@ -11,7 +11,6 @@ using mem::linear_offset; using level = page_table::level; free_page_header * page_table::s_page_cache = nullptr; -size_t page_table::s_cache_count = 0; util::spinlock page_table::s_lock; constexpr size_t page_table::entry_sizes[4]; @@ -182,14 +181,13 @@ page_table::get_table_page() { util::scoped_lock lock(s_lock); - if (!s_cache_count) + if (!s_page_cache) fill_table_page_cache(); kassert(s_page_cache, "Somehow the page cache pointer is null"); page = s_page_cache; s_page_cache = s_page_cache->next; - --s_cache_count; } memset(page, 0, frame_size); @@ -204,34 +202,29 @@ page_table::free_table_page(page_table *pt) reinterpret_cast(pt); page->next = s_page_cache; s_page_cache = page; - ++s_cache_count; } void page_table::fill_table_page_cache() { - constexpr size_t min_pages = 32; - + static constexpr size_t request_pages = 32; frame_allocator &fa = frame_allocator::get(); - while (s_cache_count < min_pages) { - uintptr_t phys = 0; - size_t n = fa.allocate(min_pages - s_cache_count, &phys); - kassert(phys, "Got physical page 0 as a page table"); - free_page_header *start = - mem::to_virtual(phys); + uintptr_t phys = 0; + size_t n = fa.allocate(request_pages, &phys); - for (int i = 0; i < n - 1; ++i) - util::offset_pointer(start, i * frame_size) - ->next = util::offset_pointer(start, (i+1) * frame_size); + free_page_header *start = + mem::to_virtual(phys); - free_page_header *end = - util::offset_pointer(start, (n-1) * frame_size); + for (int i = 0; i < n - 1; ++i) + util::offset_pointer(start, i * frame_size) + ->next = util::offset_pointer(start, (i+1) * frame_size); - end->next = s_page_cache; - s_page_cache = start; - s_cache_count += n; - } + free_page_header *end = + util::offset_pointer(start, (n-1) * frame_size); + + end->next = s_page_cache; + s_page_cache = start; } void diff --git a/src/kernel/page_table.h b/src/kernel/page_table.h index c1d63e4..145e488 100644 --- a/src/kernel/page_table.h +++ b/src/kernel/page_table.h @@ -141,7 +141,6 @@ struct page_table static void fill_table_page_cache(); static free_page_header *s_page_cache; ///< Cache of free pages to use for tables - static size_t s_cache_count; ///< Number of pages in s_page_cache static util::spinlock s_lock; ///< Lock for shared page cache /// Get an entry in the page table as a page_table pointer