Skip to content

Commit 25886e7

Browse files
Copilotequation314
andcommitted
Change to panic on capacity exceeded instead of silent fallback
Per review feedback, instead of silently falling back to reduced alignment support, now explicitly panic with a clear error message when the bitmap capacity is insufficient for the requested allocation. This provides better error reporting and makes the limitation explicit rather than degrading functionality silently. Added test to verify panic behavior when capacity is exceeded. Co-authored-by: equation314 <11389231+equation314@users.noreply.github.com>
1 parent 5acff77 commit 25886e7

File tree

1 file changed

+43
-19
lines changed

1 file changed

+43
-19
lines changed

src/bitmap.rs

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -65,22 +65,28 @@ impl<const PAGE_SIZE: usize> BaseAllocator for BitmapPageAllocator<PAGE_SIZE> {
6565
let start = crate::align_up(start, PAGE_SIZE);
6666
self.total_pages = (end - start) / PAGE_SIZE;
6767

68-
// Try to align base to MAX_ALIGN_1GB for best alignment support.
69-
// But if this creates a gap that would exceed bitmap capacity,
70-
// use the start address itself as base.
71-
let aligned_base = crate::align_down(start, MAX_ALIGN_1GB);
72-
let gap_pages = (start - aligned_base) / PAGE_SIZE;
73-
74-
if gap_pages + self.total_pages <= BitAllocUsed::CAP {
75-
// Use MAX_ALIGN_1GB aligned base for maximum alignment support
76-
self.base = aligned_base;
77-
self.inner.insert(gap_pages..gap_pages + self.total_pages);
78-
} else {
79-
// Fall back to using start as base to fit within capacity
80-
// This may limit maximum alignment support, but prevents assertion failure
81-
self.base = start;
82-
self.inner.insert(0..self.total_pages);
83-
}
68+
// Calculate the base offset stored in the real [`BitAlloc`] instance.
69+
// The base must be aligned to MAX_ALIGN_1GB to support maximum alignment.
70+
self.base = crate::align_down(start, MAX_ALIGN_1GB);
71+
72+
// Range in bitmap: [start - self.base, start - self.base + total_pages * PAGE_SIZE)
73+
let start_idx = (start - self.base) / PAGE_SIZE;
74+
let end_idx = start_idx + self.total_pages;
75+
76+
// Panic if the bitmap capacity is insufficient for the requested range.
77+
// This can happen when:
78+
// 1. The size is too large for the bitmap capacity
79+
// 2. The start address is not aligned well, creating a large gap
80+
assert!(
81+
end_idx <= BitAllocUsed::CAP,
82+
"bitmap capacity exceeded: need {} pages but CAP is {} (start={:#x}, size={:#x})",
83+
end_idx,
84+
BitAllocUsed::CAP,
85+
start,
86+
size
87+
);
88+
89+
self.inner.insert(start_idx..end_idx);
8490
}
8591

8692
fn add_memory(&mut self, _start: usize, _size: usize) -> AllocResult {
@@ -345,12 +351,13 @@ mod tests {
345351

346352
#[test]
347353
fn test_init_nonzero_start_address() {
348-
// Test with non-zero start address.
354+
// Test with non-zero start address that fits within capacity.
355+
// With BitAlloc1M in test mode (CAP = 1M pages = 4GB), a small offset
356+
// and 4MB allocation should work fine.
349357
let mut allocator = BitmapPageAllocator::<PAGE_SIZE>::new();
350358
let size = 4 * 1024 * 1024; // 4 MB size
351-
let start_addr = 40960; // non-zero address (10 pages)
359+
let start_addr = 40960; // non-zero address (10 pages offset from 0)
352360

353-
// This should not panic anymore with the fix
354361
allocator.init(start_addr, size);
355362

356363
// Verify the allocator is properly initialized
@@ -389,4 +396,21 @@ mod tests {
389396
assert_eq!(addr % (1024 * 1024), 0);
390397
allocator.dealloc_pages(addr, 1);
391398
}
399+
400+
#[test]
401+
#[should_panic(expected = "bitmap capacity exceeded")]
402+
fn test_init_capacity_exceeded() {
403+
// Test that init panics when the required range exceeds bitmap capacity.
404+
// In test mode, BitAlloc1M has CAP = 1M pages = 4GB.
405+
// With a start address that creates a 1GB gap (due to alignment) and
406+
// requesting 4GB allocation, we need 1GB/4KB + 4GB/4KB = 256K + 1M = ~1.25M pages,
407+
// which exceeds the 1M capacity.
408+
let mut allocator = BitmapPageAllocator::<PAGE_SIZE>::new();
409+
let size = 4 * 1024 * 1024 * 1024; // 4 GB - at capacity limit
410+
let start_addr = PAGE_SIZE; // Small offset causes 1GB gap when aligned down to 1GB boundary
411+
412+
// This should panic because start is aligned down to 0, creating gap of 1 page,
413+
// and 4GB = 1M pages, total = 1M + 1 which exceeds CAP of 1M
414+
allocator.init(start_addr, size);
415+
}
392416
}

0 commit comments

Comments
 (0)