From 379bcd26e406ae520ed31168ba6634436fa11570 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Tue, 13 Jul 2021 19:52:42 +0200
Subject: [PATCH] Kernel: Avoid O(n) loop over zones when allocating from
 PhysicalRegion

We now keep all the PhysicalZones on one of two intrusive lists within
the PhysicalRegion.

The "usable" list contains all zones that can be allocated from,
and the "full" list contains all zones with no free pages.
---
 Kernel/VM/PhysicalRegion.cpp | 34 ++++++++++++++++++++++++++--------
 Kernel/VM/PhysicalRegion.h   |  6 ++++--
 Kernel/VM/PhysicalZone.h     |  8 ++++++++
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/Kernel/VM/PhysicalRegion.cpp b/Kernel/VM/PhysicalRegion.cpp
index c301b18f2a..61c6301ae5 100644
--- a/Kernel/VM/PhysicalRegion.cpp
+++ b/Kernel/VM/PhysicalRegion.cpp
@@ -47,6 +47,7 @@ void PhysicalRegion::initialize_zones()
             m_zones.append(make<PhysicalZone>(base_address, zone_size));
             dmesgln(" * Zone {:016x}-{:016x} ({} bytes)", base_address.get(), base_address.get() + zone_size * PAGE_SIZE - 1, zone_size * PAGE_SIZE);
             base_address = base_address.offset(zone_size * PAGE_SIZE);
+            m_usable_zones.append(m_zones.last());
             remaining_pages -= zone_size;
         }
     };
@@ -75,9 +76,13 @@ NonnullRefPtrVector<PhysicalPage> PhysicalRegion::take_contiguous_free_pages(siz
     Optional<PhysicalAddress> page_base;
     for (auto& zone : m_zones) {
         page_base = zone.allocate_block(order);
-
-        if (page_base.has_value())
+        if (page_base.has_value()) {
+            if (zone.is_empty()) {
+                // We've exhausted this zone, move it to the full zones list.
+                m_full_zones.append(zone);
+            }
             break;
+        }
     }
 
     if (!page_base.has_value())
@@ -93,21 +98,34 @@ NonnullRefPtrVector<PhysicalPage> PhysicalRegion::take_contiguous_free_pages(siz
 
 RefPtr<PhysicalPage> PhysicalRegion::take_free_page()
 {
-    for (auto& zone : m_zones) {
-        auto page = zone.allocate_block(0);
-        if (page.has_value())
-            return PhysicalPage::create(page.value());
+    if (m_usable_zones.is_empty()) {
+        dbgln("PhysicalRegion::take_free_page: No free physical pages");
+        return nullptr;
     }
 
-    dbgln("PhysicalRegion::take_free_page: No free physical pages");
-    return nullptr;
+    auto& zone = *m_usable_zones.first();
+    auto page = zone.allocate_block(0);
+    VERIFY(page.has_value());
+
+    if (zone.is_empty()) {
+        // We've exhausted this zone, move it to the full zones list.
+        m_full_zones.append(zone);
+    }
+
+    return PhysicalPage::create(page.value());
 }
 
 void PhysicalRegion::return_page(PhysicalAddress paddr)
 {
+    // FIXME: Find a way to avoid looping over the zones here.
+    //        (Do some math on the address to find the right zone index.)
+    //        The main thing that gets in the way of this is non-uniform zone sizes.
+    //        Perhaps it would be better if all zones had the same size.
     for (auto& zone : m_zones) {
         if (zone.contains(paddr)) {
             zone.deallocate_block(paddr, 0);
+            if (m_full_zones.contains(zone))
+                m_usable_zones.append(zone);
             return;
         }
     }
diff --git a/Kernel/VM/PhysicalRegion.h b/Kernel/VM/PhysicalRegion.h
index 4288c2bc4a..b0f8450b88 100644
--- a/Kernel/VM/PhysicalRegion.h
+++ b/Kernel/VM/PhysicalRegion.h
@@ -11,11 +11,10 @@
 #include <AK/Optional.h>
 #include <AK/OwnPtr.h>
 #include <Kernel/VM/PhysicalPage.h>
+#include <Kernel/VM/PhysicalZone.h>
 
 namespace Kernel {
 
-class PhysicalZone;
-
 class PhysicalRegion {
     AK_MAKE_ETERNAL
 public:
@@ -44,6 +43,9 @@ private:
 
     NonnullOwnPtrVector<PhysicalZone> m_zones;
 
+    PhysicalZone::List m_usable_zones;
+    PhysicalZone::List m_full_zones;
+
     PhysicalAddress m_lower;
     PhysicalAddress m_upper;
     unsigned m_pages { 0 };
diff --git a/Kernel/VM/PhysicalZone.h b/Kernel/VM/PhysicalZone.h
index f2a7d1d583..1eb8b74944 100644
--- a/Kernel/VM/PhysicalZone.h
+++ b/Kernel/VM/PhysicalZone.h
@@ -8,6 +8,7 @@
 
 #include <AK/Bitmap.h>
 #include <AK/Forward.h>
+#include <AK/IntrusiveList.h>
 #include <AK/Types.h>
 #include <Kernel/Forward.h>
 
@@ -32,6 +33,8 @@ public:
     void dump() const;
     size_t available() const { return m_page_count - (m_used_chunks / 2); }
 
+    bool is_empty() const { return !available(); }
+
     PhysicalAddress base() const { return m_base_address; }
     bool contains(PhysicalAddress paddr) const
     {
@@ -82,6 +85,11 @@ private:
     PhysicalAddress m_base_address { 0 };
     size_t m_page_count { 0 };
     size_t m_used_chunks { 0 };
+
+    IntrusiveListNode<PhysicalZone> m_list_node;
+
+public:
+    using List = IntrusiveList<PhysicalZone, RawPtr<PhysicalZone>, &PhysicalZone::m_list_node>;
 };
 
 }