[ulib][ralloc] Add a cookie value to RegionAllocator::Region
A uint64_t cookie value has been added to the Region class. By doing it
in the Region class itself rather than the base ralloc_region_t it
allows for continued use of C style struct initializers with GetRegion.
Due to the const nature of returned regions from GetRegion, this cookie
is set immutably as an optional argument to GetRegion. These changes
have the side effect of removing partiy between the C and C++ apis, but
only for the cookie. It is considered an acceptable cost to avoid all
callers using C style struct initialization needing to specify a default
cookie value.
The impact this change will have is an extra 8 bytes of memory per
region pulled out of an allocator. On a Pixelbook there are presently
~30 regions allocated at boot. This is miniscule compared to the benefit
the bookkeeping adds for clients of the library.
Test: region-alloc-test
Change-Id: I3f3eb65e990f94af58f71c6cc72fe36694632a13
diff --git a/system/ulib/region-alloc/include/region-alloc/region-alloc.h b/system/ulib/region-alloc/include/region-alloc/region-alloc.h
index a97456c..3728d40 100644
--- a/system/ulib/region-alloc/include/region-alloc/region-alloc.h
+++ b/system/ulib/region-alloc/include/region-alloc/region-alloc.h
@@ -252,6 +252,7 @@
public fbl::Recyclable<Region> {
public:
using UPtr = fbl::unique_ptr<const Region>;
+ uint64_t GetCookie() const { return cookie_; }
private:
using WAVLTreeNodeState = fbl::WAVLTreeNodeState<Region*>;
@@ -318,6 +319,7 @@
RegionAllocator* owner_;
WAVLTreeNodeState ns_tree_sort_by_base_;
WAVLTreeNodeState ns_tree_sort_by_size_;
+ uint64_t cookie_;
};
class RegionPool : public fbl::RefCounted<RegionPool>,
@@ -428,7 +430,8 @@
// ++ ZX_ERR_INVALID_ARGS : size is zero, or alignment is not a power of two.
// ++ ZX_ERR_NOT_FOUND : No suitable region could be found in the set of
// currently available regions which can satisfy the request.
- zx_status_t GetRegion(uint64_t size, uint64_t alignment, Region::UPtr& out_region)
+ zx_status_t GetRegion(uint64_t size, uint64_t alignment, Region::UPtr& out_region,
+ uint64_t cookie = 0)
__TA_EXCLUDES(alloc_lock_);
// Get a region with a specific location and size out of the set of
@@ -441,31 +444,34 @@
// ++ ZX_ERR_INVALID_ARGS : The size of the requested region is zero.
// ++ ZX_ERR_NOT_FOUND : No suitable region could be found in the set of
// currently available regions which can satisfy the request.
- zx_status_t GetRegion(const ralloc_region_t& requested_region, Region::UPtr& out_region)
- __TA_EXCLUDES(alloc_lock_);;
+ zx_status_t GetRegion(const ralloc_region_t& requested_region, Region::UPtr& out_region,
+ uint64_t cookie = 0) __TA_EXCLUDES(alloc_lock_);;
// Helper which defaults the alignment of a size/alignment based allocation
// to pointer-aligned.
- zx_status_t GetRegion(uint64_t size, Region::UPtr& out_region) __TA_EXCLUDES(alloc_lock_) {
- return GetRegion(size, sizeof(void*), out_region);
+ zx_status_t GetRegion(uint64_t size, Region::UPtr& out_region, uint64_t cookie = 0)
+ __TA_EXCLUDES(alloc_lock_) {
+ return GetRegion(size, sizeof(void*), out_region, cookie);
}
// Helper versions of the GetRegion methods for those who don't care
// about the specific reason for failure (nullptr will be returned on
// failure).
- Region::UPtr GetRegion(uint64_t size, uint64_t alignment) __TA_EXCLUDES(alloc_lock_) {
+ Region::UPtr GetRegion(uint64_t size, uint64_t alignment, uint64_t cookie = 0)
+ __TA_EXCLUDES(alloc_lock_) {
Region::UPtr ret;
- GetRegion(size, alignment, ret);
+ GetRegion(size, alignment, ret, cookie);
return ret;
}
- Region::UPtr GetRegion(uint64_t size) __TA_EXCLUDES(alloc_lock_) {
+ Region::UPtr GetRegion(uint64_t size, uint64_t cookie = 0) __TA_EXCLUDES(alloc_lock_) {
Region::UPtr ret;
- GetRegion(size, ret);
+ GetRegion(size, ret, cookie);
return ret;
}
- Region::UPtr GetRegion(const ralloc_region_t& requested_region) __TA_EXCLUDES(alloc_lock_) {
+ Region::UPtr GetRegion(const ralloc_region_t& requested_region, uint64_t cookie = 0)
+ __TA_EXCLUDES(alloc_lock_) {
Region::UPtr ret;
GetRegion(requested_region, ret);
return ret;
@@ -510,7 +516,8 @@
zx_status_t AllocFromAvailLocked(Region::WAVLTreeSortBySize::iterator source,
Region::UPtr& out_region,
uint64_t base,
- uint64_t size) __TA_REQUIRES(alloc_lock_);
+ uint64_t size,
+ uint64_t cookie) __TA_REQUIRES(alloc_lock_);
bool IntersectsLocked(const Region::WAVLTreeSortByBase& tree,
const ralloc_region_t& region) __TA_REQUIRES(alloc_lock_);
diff --git a/system/ulib/region-alloc/region-alloc.cpp b/system/ulib/region-alloc/region-alloc.cpp
index 5f2cc87..85711cc 100644
--- a/system/ulib/region-alloc/region-alloc.cpp
+++ b/system/ulib/region-alloc/region-alloc.cpp
@@ -275,7 +275,8 @@
zx_status_t RegionAllocator::GetRegion(uint64_t size,
uint64_t alignment,
- Region::UPtr& out_region) {
+ Region::UPtr& out_region,
+ uint64_t cookie) {
fbl::AutoLock alloc_lock(&alloc_lock_);
// Check our RegionPool
@@ -318,11 +319,11 @@
if (!iter.IsValid())
return ZX_ERR_NOT_FOUND;
- return AllocFromAvailLocked(iter, out_region, aligned_base, size);
+ return AllocFromAvailLocked(iter, out_region, aligned_base, size, cookie);
}
zx_status_t RegionAllocator::GetRegion(const ralloc_region_t& requested_region,
- Region::UPtr& out_region) {
+ Region::UPtr& out_region, uint64_t cookie) {
fbl::AutoLock alloc_lock(&alloc_lock_);
// Check our RegionPool
@@ -368,7 +369,7 @@
// allocation request. Get an iterator for the by-size index, then use the
// common AllocFromAvailLocked method to handle the bookkeeping involved.
auto by_size_iter = avail_regions_by_size_.make_iterator(*iter);
- return AllocFromAvailLocked(by_size_iter, out_region, base, size);
+ return AllocFromAvailLocked(by_size_iter, out_region, base, size, cookie);
}
zx_status_t RegionAllocator::AddSubtractSanityCheckLocked(const ralloc_region_t& region) {
@@ -409,7 +410,8 @@
zx_status_t RegionAllocator::AllocFromAvailLocked(Region::WAVLTreeSortBySize::iterator source,
Region::UPtr& out_region,
uint64_t base,
- uint64_t size) {
+ uint64_t size,
+ uint64_t cookie) {
ZX_DEBUG_ASSERT(out_region == nullptr);
ZX_DEBUG_ASSERT(source.IsValid());
ZX_DEBUG_ASSERT(base >= source->base);
@@ -435,6 +437,8 @@
Region* region = avail_regions_by_size_.erase(source);
avail_regions_by_base_.erase(*region);
allocated_regions_by_base_.insert(region);
+
+ region->cookie_ = cookie;
out_region.reset(region);
} else if (!split_before) {
// If we only have to split after, then this region is aligned with what
@@ -454,6 +458,7 @@
avail_regions_by_size_.insert(after_region);
allocated_regions_by_base_.insert(before_region);
+ before_region->cookie_ = cookie;
out_region.reset(before_region);
} else if (!split_after) {
// If we only have to split before, then this region is not aligned
@@ -473,6 +478,7 @@
avail_regions_by_size_.insert(before_region);
allocated_regions_by_base_.insert(after_region);
+ after_region->cookie_ = cookie;
out_region.reset(after_region);
} else {
// Looks like we need to break our region into 3 chunk and return the
@@ -492,6 +498,7 @@
region->base = before_region->base + overhead;
region->size = size;
+ region->cookie_ = cookie;
after_region->base = region->base + region->size;
after_region->size = before_region->size - size - overhead;
before_region->size = overhead;
@@ -501,6 +508,7 @@
avail_regions_by_base_.insert(after_region);
allocated_regions_by_base_.insert(region);
+ region->cookie_ = cookie;
out_region.reset(region);
}
return ZX_OK;
diff --git a/system/utest/region-alloc/region-alloc.cpp b/system/utest/region-alloc/region-alloc.cpp
index 3c6e417..9bacfec 100644
--- a/system/utest/region-alloc/region-alloc.cpp
+++ b/system/utest/region-alloc/region-alloc.cpp
@@ -292,6 +292,40 @@
END_TEST;
}
+static bool ralloc_region_cookie_test() {
+ BEGIN_TEST;
+ RegionAllocator alloc;
+ const uint64_t cookie = 0x636f6f6b6965; // 'cookie'
+
+ auto pool = RegionAllocator::RegionPool::Create(REGION_POOL_MAX_SIZE);
+ ASSERT_EQ(ZX_OK, alloc.SetRegionPool(pool));
+ ASSERT_EQ(ZX_OK, alloc.AddRegion({0u, 1024u}));
+
+ // Check that the default cookie value is 0u.
+ {
+ RegionAllocator::Region::UPtr tmp;
+ EXPECT_EQ(ZX_OK, alloc.GetRegion({ .base = 128, .size = 128}, tmp));
+ EXPECT_EQ(0u, tmp->GetCookie());
+ }
+
+ // Pull the region out with the cookie value set and verify.
+ {
+ RegionAllocator::Region::UPtr tmp;
+ EXPECT_EQ(ZX_OK, alloc.GetRegion({ .base = 128, .size = 128}, tmp, cookie));
+ EXPECT_EQ(cookie, tmp->GetCookie());
+ }
+
+ // Check that with the cookie omitted again that the region
+ // will again have a cookie of 0u
+ {
+ RegionAllocator::Region::UPtr tmp;
+ EXPECT_EQ(ZX_OK, alloc.GetRegion({ .base = 128, .size = 128}, tmp));
+ EXPECT_EQ(0u, tmp->GetCookie());
+ }
+ END_TEST;
+}
+
+
} //namespace
BEGIN_TEST_CASE(ralloc_tests)
@@ -301,4 +335,5 @@
RUN_NAMED_TEST("Add/Overlap", ralloc_add_overlap_test)
RUN_NAMED_TEST("Subtract", ralloc_subtract_test)
RUN_NAMED_TEST("Allocated Walk", ralloc_alloc_walk_test)
+RUN_NAMED_TEST("Cookie Test", ralloc_region_cookie_test)
END_TEST_CASE(ralloc_tests)
