bugfix in ram allocator; 2-bit unified bitmap

include/time.h

@@ -62,7 +62,6 @@ struct Timer {
 	Timer    *first;
 	Time      time;
 	TimerFunc func;
-	void *arg;
 };
 
 extern Timer *timer_queue;

src/ram.c

@@ -6,13 +6,6 @@
 
 extern BOOTBOOT bootboot;
 
-// Simple bitmap implementation
-typedef uint64_t bitmap;
-#define BITMAP_SIZE(count) (((count) + 63) & ~63ul)
-#define GET_BIT(bitmap, index) (((bitmap)[(index) / 64] >> ((index) % 64)) & 1)
-#define SET_BIT(bitmap, index) ((bitmap)[(index) / 64] |= 1ul << (index))
-#define CLR_BIT(bitmap, index) ((bitmap)[(index) / 64] &= ~(1ul << (index)))
-
 // unaligned to catch errors
 #define INVALID_FRAME_ADDR 0xBADFul
 #define IS_VALID_FRAME_ADDR(addr) ((addr & 0xfff) == 0)
@@ -51,12 +44,42 @@ struct free_block {
     struct pa next;
 };
 
+// We have a bitmap for each possible block on each level:
+// In the highest level, there is a single bitmap entry (because there can be at most 1 block).
+// In the one below there are 2 entries, in the next one 4, then 8, then 16, ...
+// A block entry has size 2 bits.
+//
+// Consider the following RAM layout: 32 KiB RAM -> 4 levels. When everything is free:
+//   lvl3 [ FREED ]
+//   lvl2 [ INVAL,                      INVAL ]
+//   lvl1 [ INVAL,        INVAL,        INVAL,        INVAL ]
+//   lvl0 [ INVAL, INVAL, INVAL, INVAL, INVAL, INVAL, INVAL, INVAL ]
+// When we allocate 4 KiB (3 splits happen):
+//   lvl3 [ SPLIT ]
+//   lvl2 [ SPLIT,                      FREED ]
+//   lvl1 [ SPLIT,        FREED,        INVAL,        INVAL ]
+//   lvl0 [ ALLOC, FREED, INVAL, INVAL, INVAL, INVAL, INVAL, INVAL ]
+// When we then allocate 8 KiB (no split happens):
+//   lvl3 [ SPLIT ]
+//   lvl2 [ SPLIT,                      FREED ]
+//   lvl1 [ SPLIT,        ALLOC,        INVAL,        INVAL ]
+//   lvl0 [ ALLOC, FREED, INVAL, INVAL, INVAL, INVAL, INVAL, INVAL ]
+// When we then free the first 4 KiB (merge happens):
+//   lvl3 [ SPLIT ]
+//   lvl2 [ SPLIT,                      FREED ]
+//   lvl1 [ FREED,        ALLOC,        INVAL,        INVAL ]
+//   lvl0 [ INVAL, INVAL, INVAL, INVAL, INVAL, INVAL, INVAL, INVAL ]
+enum block_state {
+    BLOCK_STATE_INVALID = 0,
+    BLOCK_STATE_FREE = 1,
+    BLOCK_STATE_ALLOCATED = 2,
+    BLOCK_STATE_SPLIT = 3,
+};
+
 struct ram_layer {
     struct pa first_free;
     // this is a bitmap *
-    struct pa avail_bitmap;
-    // this is a bitmap *
-    struct pa coherent_bitmap;
+    struct pa bitmap;
 };
 
 // levels are the size of the blocks
@@ -149,6 +172,7 @@ freelist_insert(unsigned level, struct pa addr)
 static void
 freelist_remove(unsigned level, struct pa addr)
 {
+    ASSERT(IS_VALID_FRAME_ADDR(pa_to_value(addr)));
     struct free_block *block = pa_to_pointer(addr);
     ASSERT(block->magic == FREE_BLOCK_MAGIC);
 
@@ -157,61 +181,30 @@ freelist_remove(unsigned level, struct pa addr)
         prev->next = block->next;
     } else {
         // this block was the first in the list.
+        ASSERT(ram_layers[level].first_free.value == addr.value);
         ram_layers[level].first_free = block->next;
     }
+
     if (IS_VALID_FRAME_ADDR(pa_to_value(block->next))) {
         struct free_block *next = pa_to_pointer(block->next);
         next->prev = block->prev;
     }
 }
 
-static inline bool
-is_block_free(unsigned level, struct pa addr)
-{
+static inline enum block_state
+block_state_get(unsigned level, struct pa addr) {
     uint64_t index = pa_to_value(addr) >> (PG_SHIFT + level);
-    return GET_BIT((bitmap *)pa_to_pointer(ram_layers[level].avail_bitmap), index);
+    uint8_t *bm = (uint8_t*)pa_to_pointer(ram_layers[level].bitmap);
+    uint8_t shift = (index & 3ul) << 1;
+    return (bm[index>>2] >> shift) & 0x3;
 }
 
-static inline bool
-is_block_coherent(unsigned level, struct pa addr)
-{
+static inline void
+block_state_set(unsigned level, struct pa addr, enum block_state state) {
     uint64_t index = pa_to_value(addr) >> (PG_SHIFT + level);
-    return GET_BIT((bitmap *)pa_to_pointer(ram_layers[level].coherent_bitmap), index);
-}
-
-static void
-set_block_alloced(unsigned level, struct pa addr)
-{
-    struct ram_layer *layer = &ram_layers[level];
-    uint64_t index = pa_to_value(addr) >> (PG_SHIFT + level);
-
-    // Remove from bitmap of available blocks
-    ASSERT(GET_BIT((bitmap *)pa_to_pointer(layer->avail_bitmap), index) == 1);
-    CLR_BIT((bitmap *)pa_to_pointer(layer->avail_bitmap), index);
-}
-
-static void
-set_block_free(unsigned level, struct pa addr)
-{
-    struct ram_layer *layer = &ram_layers[level];
-    uint64_t index = pa_to_value(addr) >> (PG_SHIFT + level);
-
-    // Add to bitmap of available blocks
-    ASSERT(GET_BIT((bitmap *)pa_to_pointer(layer->avail_bitmap), index) == 0);
-    SET_BIT((bitmap *)pa_to_pointer(layer->avail_bitmap), index);
-}
-
-static void
-set_block_coherency(unsigned level, struct pa addr, bool coherent)
-{
-    struct ram_layer *layer = &ram_layers[level];
-    uint64_t index = pa_to_value(addr) >> (PG_SHIFT + level);
-
-    if (coherent) {
-        SET_BIT((bitmap *)pa_to_pointer(layer->coherent_bitmap), index);
-    } else {
-        CLR_BIT((bitmap *)pa_to_pointer(layer->coherent_bitmap), index);
-    }
+    uint8_t *bm = (uint8_t*)pa_to_pointer(ram_layers[level].bitmap);
+    uint8_t shift = (index & 3ul) << 1;
+    bm[index>>2] = (bm[index>>2] & ~(3u << shift)) | ((unsigned)state << shift);
 }
 
 static bool
@@ -235,20 +228,20 @@ alloc_block(struct pa *addr_out, unsigned magnitude)
     while (level > magnitude) {
         ASSERT(IS_VALID_FRAME_ADDR(pa_to_value(ram_layers[level].first_free)));
 
-        // Mark this block as allocated because it will have been split
+        // Mark this block as split
         struct pa addr = ram_layers[level].first_free;
         freelist_remove(level, addr);
-        set_block_alloced(level, addr);
+        block_state_set(level, addr, BLOCK_STATE_SPLIT);
 
         // Mark the left child as free
         freelist_insert(level - 1, addr);
-        set_block_free(level - 1, addr);
+        block_state_set(level - 1, addr, BLOCK_STATE_FREE);
 
         // Mark the right child as free
         struct pa buddy = pa_from_value(pa_to_value(addr) ^ (1ul << (PG_SHIFT + (level - 1))));
         ASSERT(pa_to_value(buddy) > pa_to_value(addr));
         freelist_insert(level - 1, buddy);
-        set_block_free(level - 1, buddy);
+        block_state_set(level - 1, buddy, BLOCK_STATE_FREE);
 
         // Traverse into left child (by simply keeping the address the same)
         level--;
@@ -258,7 +251,7 @@ alloc_block(struct pa *addr_out, unsigned magnitude)
     ASSERT(IS_VALID_FRAME_ADDR(pa_to_value(ram_layers[level].first_free)));
     *addr_out = ram_layers[level].first_free;
     freelist_remove(level, *addr_out);
-    set_block_alloced(level, *addr_out);
+    block_state_set(level, *addr_out, BLOCK_STATE_ALLOCATED);
 
     ASSERT(global_ram_info.bytes_free >= NUM_BYTES_COVERED_IN_LEVEL(level));
     global_ram_info.bytes_free -= NUM_BYTES_COVERED_IN_LEVEL(level);
@@ -273,7 +266,7 @@ free_block(struct pa addr, unsigned magnitude)
 
     // Release the block we found
     freelist_insert(magnitude, addr);
-    set_block_free(magnitude, addr);
+    block_state_set(magnitude, addr, BLOCK_STATE_FREE);
 
     global_ram_info.bytes_free += NUM_BYTES_COVERED_IN_LEVEL(magnitude);
     ASSERT(global_ram_info.bytes_free <= global_ram_info.bytes_total);
@@ -284,17 +277,23 @@ free_block(struct pa addr, unsigned magnitude)
         struct pa joined = pa_from_value(pa_to_value(addr) & pa_to_value(buddy));
 
         // Stop if our buddy isn't free
-        if (!is_block_free(level, buddy)) {
+        enum block_state buddy_state = block_state_get(level, buddy);
+        if (buddy_state != BLOCK_STATE_FREE) {
             break;
         }
 
         // Remove both blocks from free list
         freelist_remove(level, addr);
         freelist_remove(level, buddy);
+        block_state_set(level, addr, BLOCK_STATE_INVALID);
+        block_state_set(level, buddy, BLOCK_STATE_INVALID);
 
         // Put the combined block onto the free list
         freelist_insert(level + 1, joined);
-        set_block_free(level + 1, joined);
+        block_state_set(level + 1, joined, BLOCK_STATE_FREE);
+
+        // we continue with the joined block
+        addr = joined;
     }
 }
 
@@ -303,40 +302,31 @@ get_allocation_magnitude(struct pa addr, unsigned *magnitude)
 {
     ASSERT(IS_VALID_FRAME_ADDR(pa_to_value(addr)));
 
-    // Descend levels until we find the whole coherent (unfragmented) allocated block
-    unsigned level = ram_num_levels;
-    while (level--) {
-        if (is_block_free(level, addr)) {
-            return false;
-        }
-        if (is_block_coherent(level, addr)) {
+    // Ascend and find the allocated block
+    unsigned level = 0;
+    while (level < ram_num_levels) {
+        enum block_state state = block_state_get(level, addr);
+        if (state == BLOCK_STATE_ALLOCATED) {
             *magnitude = level;
             return true;
         }
+        // the block cannot be split, because then it would have been found
+        // on a lower level already.
+        ASSERT(state != BLOCK_STATE_SPLIT);
+        // it also cannot be free because we expect it to be allocated.
+        ASSERT(state != BLOCK_STATE_FREE);
+        level++;
     }
 
     return false;
 }
 
-bool
-ram_alloc_frame(struct ppn *ppn_out, enum frame_size size)
-{
-    CHECK_INIT;
-    ASSERT(ppn_out != NULL);
-
-    unsigned magnitude;
-    switch (size) {
-        case RAM_PAGE_NORMAL: magnitude =  0; break;
-        case RAM_PAGE_LARGE:  magnitude =  9; break;
-        case RAM_PAGE_HUGE:   magnitude = 18; break;
-        default: UNREACHABLE();
-    }
-
+static bool
+ram_alloc_with_magnitude(struct ppn *ppn_out, unsigned magnitude) {
     struct pa addr;
     if (!alloc_block(&addr, magnitude)) {
         return false;
     }
-    set_block_coherency(magnitude, addr, true);
 
     *ppn_out = ppn_from_aligned_pa(addr);
 
@@ -359,6 +349,23 @@ ram_alloc_frame(struct ppn *ppn_out, enum frame_size size)
     return true;
 }
 
+bool
+ram_alloc_frame(struct ppn *ppn_out, enum frame_size size)
+{
+    CHECK_INIT;
+    ASSERT(ppn_out != NULL);
+
+    unsigned magnitude;
+    switch (size) {
+        case RAM_PAGE_NORMAL: magnitude =  0; break;
+        case RAM_PAGE_LARGE:  magnitude =  9; break;
+        case RAM_PAGE_HUGE:   magnitude = 18; break;
+        default: UNREACHABLE();
+    }
+
+    return ram_alloc_with_magnitude(ppn_out, magnitude);
+}
+
 bool
 ram_alloc_frame_zeroed(struct ppn *ppn_out, enum frame_size size) {
     CHECK_INIT;
@@ -386,30 +393,7 @@ ram_alloc_buffer(struct ppn *ppn_out, struct ram_buffer_requirements req)
     uint64_t next_size = next_pow2(req.size);
     unsigned magnitude = __builtin_ffsl(next_size) - 1 - PG_SHIFT;
 
-    struct pa addr;
-    if (!alloc_block(&addr, magnitude)) {
-        return false;
-    }
-    set_block_coherency(magnitude, addr, true);
-
-    *ppn_out = ppn_from_aligned_pa(addr);
-
-#ifdef RAM_DEBUG_TRACK_ALLOCED_RANGES
-    struct mem_range this_range = {
-        .start = addr,
-        .size = next_size
-    };
-    // check that this allocation is in a valid area
-    bool sane_range = cover_contains_range(&bootboot_free_cover, this_range);
-    if (!sane_range) {
-        PANIC("allocator returns memory that doesn't belong to it!");
-    }
-    // check that this allocation doesn't intersect with previous ones
-    ASSERT(!cover_intersects_range(&debug_allocated_cover, this_range));
-    cover_add(&debug_allocated_cover, this_range);
-#endif
-
-    return true;
+    return ram_alloc_with_magnitude(ppn_out, magnitude);
 }
 
 bool
@@ -435,7 +419,6 @@ ram_free(struct ppn ppn)
         PANIC("Attempting to free physical address which is not base of a known allocation.");
     }
     free_block(addr, magnitude);
-    set_block_coherency(magnitude, addr, false);
 
 #ifdef RAM_DEBUG_TRACK_ALLOCED_RANGES
     struct mem_range this_range = {
@@ -493,17 +476,16 @@ ram_init(void)
     DEBUG_PRINTF("addrLimit = %lu\n", addrLimit);
     DEBUG_PRINTF("ram_num_levels = %u\n", ram_num_levels);
 
-    // Allocate bitmaps for all levels; Initialize all memory as allocated
+    // Allocate bitmaps for all levels
     for (unsigned level = 0; level < ram_num_levels; level++) {
         ram_layers[level].first_free = pa_from_value(INVALID_FRAME_ADDR);
 
-        size_t bitmapSize = BITMAP_SIZE(1ul << (ram_num_levels - level - 1));
+        // ex.: 6 levels: bitmap sizes [32 16 8 5 2 1] -> bitmap byte sizes [8 4 2 1 1 1]
+        size_t bitmap_size_bytes = (1ul << (ram_num_levels - level - 1)) >> 2;
+        if (bitmap_size_bytes == 0) bitmap_size_bytes = 1;
 
-        ASSERT(cover_steal(&bootboot_free_cover, bitmapSize, &ram_layers[level].avail_bitmap));
-        memset(pa_to_pointer(ram_layers[level].avail_bitmap), 0x00, bitmapSize);
-
-        ASSERT(cover_steal(&bootboot_free_cover, bitmapSize, &ram_layers[level].coherent_bitmap));
-        memset(pa_to_pointer(ram_layers[level].coherent_bitmap), 0x00, bitmapSize);
+        ASSERT(cover_steal(&bootboot_free_cover, bitmap_size_bytes, &ram_layers[level].bitmap));
+        memset(pa_to_pointer(ram_layers[level].bitmap), 0x00, bitmap_size_bytes);
     }
 
     // align all segments to page size