Kernel: Encapsulate the Region's COW map a bit better.

luteresa · May 14, 2019 · 01ffcdf · 01ffcdf
1 parent 2fa5e2b
commit 01ffcdf
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 13 deletions.
diff --git a/AK/Bitmap.h b/AK/Bitmap.h
@@ -45,15 +45,19 @@ class Bitmap {
  byte* data() { return m_data; }
  const byte* data() const { return m_data; }
 
+ void fill(bool value)
+ {
+ memset(m_data, value ? 0xff : 0x00, size_in_bytes());
+ }
+
 private:
  explicit Bitmap(int size, bool default_value)
  : m_size(size)
  , m_owned(true)
  {
  ASSERT(m_size != 0);
- int size_to_allocate = ceil_div(size, 8);
- m_data = reinterpret_cast<byte*>(kmalloc(size_to_allocate));
- memset(m_data, default_value ? 0xff : 0x00, size_to_allocate);
+ m_data = reinterpret_cast<byte*>(kmalloc(size_in_bytes()));
+ fill(default_value);
  }
 
  Bitmap(byte* data, int size)
@@ -63,6 +67,8 @@ class Bitmap {
  {
  }
 
+ int size_in_bytes() const { return ceil_div(m_size, 8); }
+
  byte* m_data { nullptr };
  int m_size { 0 };
  bool m_owned { false };

diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp
@@ -279,7 +279,7 @@ ByteBuffer procfs$pid_vmo(InodeIdentifier identifier)
  auto& physical_page = region->vmo().physical_pages()[i];
  builder.appendf("P%x%s(%u) ",
  physical_page ? physical_page->paddr().get() : 0,
- region->cow_map().get(i) ? "!" : "",
+ region->should_cow(i) ? "!" : "",
  physical_page ? physical_page->retain_count() : 0
  );
  }

diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp
@@ -264,7 +264,7 @@ bool MemoryManager::zero_page(Region& region, unsigned page_index_in_region)
 #ifdef PAGE_FAULT_DEBUG
  dbgprintf(" >> ZERO P%x\n", physical_page->paddr().get());
 #endif
- region.m_cow_map.set(page_index_in_region, false);
+ region.set_should_cow(page_index_in_region, false);
  vmo.physical_pages()[page_index_in_region] = move(physical_page);
  remap_region_page(region, page_index_in_region, true);
  return true;
@@ -278,7 +278,7 @@ bool MemoryManager::copy_on_write(Region& region, unsigned page_index_in_region)
 #ifdef PAGE_FAULT_DEBUG
  dbgprintf(" >> It's a COW page but nobody is sharing it anymore. Remap r/w\n");
 #endif
- region.m_cow_map.set(page_index_in_region, false);
+ region.set_should_cow(page_index_in_region, false);
  remap_region_page(region, page_index_in_region, true);
  return true;
  }
@@ -296,7 +296,7 @@ bool MemoryManager::copy_on_write(Region& region, unsigned page_index_in_region)
  memcpy(dest_ptr, src_ptr, PAGE_SIZE);
  vmo.physical_pages()[page_index_in_region] = move(physical_page);
  unquickmap_page();
- region.m_cow_map.set(page_index_in_region, false);
+ region.set_should_cow(page_index_in_region, false);
  remap_region_page(region, page_index_in_region, true);
  return true;
 }
@@ -379,7 +379,7 @@ PageFaultResponse MemoryManager::handle_page_fault(const PageFault& fault)
  return PageFaultResponse::Continue;
  }
  } else if (fault.is_protection_violation()) {
- if (region->m_cow_map.get(page_index_in_region)) {
+ if (region->should_cow(page_index_in_region)) {
 #ifdef PAGE_FAULT_DEBUG
  dbgprintf("PV(cow) fault in Region{%p}[%u]\n", region, page_index_in_region);
 #endif
@@ -534,7 +534,7 @@ void MemoryManager::remap_region_page(Region& region, unsigned page_index_in_reg
  ASSERT(physical_page);
  pte.set_physical_page_base(physical_page->paddr().get());
  pte.set_present(true); // FIXME: Maybe we should use the is_readable flag here?
- if (region.m_cow_map.get(page_index_in_region))
+ if (region.should_cow(page_index_in_region))
  pte.set_writable(false);
  else
  pte.set_writable(region.is_writable());
@@ -570,7 +570,7 @@ void MemoryManager::map_region_at_address(PageDirectory& page_directory, Region&
  pte.set_physical_page_base(physical_page->paddr().get());
  pte.set_present(true); // FIXME: Maybe we should use the is_readable flag here?
  // FIXME: It seems wrong that the *region* cow map is essentially using *VMO* relative indices.
- if (region.m_cow_map.get(region.first_page_index() + i))
+ if (region.should_cow(region.first_page_index() + i))
  pte.set_writable(false);
  else
  pte.set_writable(region.is_writable());

diff --git a/Kernel/VM/Region.cpp b/Kernel/VM/Region.cpp
@@ -94,8 +94,7 @@ Retained<Region> Region::clone()
  laddr().get());
 #endif
  // Set up a COW region. The parent (this) region becomes COW as well!
- for (size_t i = 0; i < page_count(); ++i)
- m_cow_map.set(i, true);
+ m_cow_map.fill(true);
  MM.remap_region(current->process().page_directory(), *this);
  return adopt(*new Region(laddr(), size(), m_vmo->clone(), m_offset_in_vmo, String(m_name), m_readable, m_writable, true));
 }

diff --git a/Kernel/VM/Region.h b/Kernel/VM/Region.h
@@ -75,7 +75,8 @@ class Region : public Retainable<Region> {
  m_page_directory.clear();
  }
 
- const Bitmap& cow_map() const { return m_cow_map; }
+ bool should_cow(size_t page_index) const { return m_cow_map.get(page_index); }
+ void set_should_cow(size_t page_index, bool cow) { m_cow_map.set(page_index, cow); }
 
  void set_writable(bool b) { m_writable = b; }