From acbb18a4d0dd07430c13e82664f16fdde3e2f866 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 17 Jul 2019 09:58:25 -0700 Subject: [PATCH 01/15] lib/os: Add sys_heap, a new/simpler/faster memory allocator The existing mem_pool implementation has been an endless source of frustration. It's had alignment bugs, it's had racy behavior. It's never been particularly fast. It's outrageously complicated to configure statically. And while its fragmentation resistance and overhead on small blocks is good, it's space efficiencey has always been very poor due to the four-way buddy scheme. This patch introduces sys_heap. It's a more or less conventional segregated fit allocator with power-of-two buckets. It doesn't expose its level structure to the user at all, simply taking an arbitrarily aligned pointer to memory. It stores all metadata inside the heap region. It allocates and frees by simple pointer and not block ID. Static initialization is trivial, and runtime initialization is only a few cycles to format and add one block to a list header. It has excellent space efficiency. Chunks can be split arbitrarily in 8 byte units. Overhead is only four bytes per allocated chunk (eight bytes for heaps >256kb or on 64 bit systems), plus a log2-sized array of 2-word bucket headers. No coarse alignment restrictions on blocks, they can be split and merged (in units of 8 bytes) arbitrarily. It has good fragmentation resistance. Freed blocks are always immediately merged with adjacent free blocks. Allocations are attempted from a sample of the smallest bucket that might fit, falling back rapidly to the smallest block guaranteed to fit. Split memory remaining in the chunk is always returned immediately to the heap for other allocation. It has excellent performance with firmly bounded runtime. All operations are constant time (though there is a search of the smallest bucket that has a compile-time-configurable upper bound, setting this to extreme values results in an effectively linear search of the list), objectively fast (about a hundred instructions) and amenable to locked operation. No more need for fragile lock relaxation trickery. It also contains an extensive validation and stress test framework, something that was sorely lacking in the previous implementation. Note that sys_heap is not a compatible API with sys_mem_pool and k_mem_pool. Wrappers for those (now-) legacy APIs appear later in this patch series. Signed-off-by: Andy Ross --- include/sys/sys_heap.h | 156 +++++++++++++++++++++ lib/os/CMakeLists.txt | 2 + lib/os/Kconfig | 25 ++++ lib/os/heap-validate.c | 307 +++++++++++++++++++++++++++++++++++++++++ lib/os/heap.c | 239 ++++++++++++++++++++++++++++++++ lib/os/heap.h | 164 ++++++++++++++++++++++ 6 files changed, 893 insertions(+) create mode 100644 include/sys/sys_heap.h create mode 100644 lib/os/heap-validate.c create mode 100644 lib/os/heap.c create mode 100644 lib/os/heap.h diff --git a/include/sys/sys_heap.h b/include/sys/sys_heap.h new file mode 100644 index 0000000000000..38c41891db66d --- /dev/null +++ b/include/sys/sys_heap.h @@ -0,0 +1,156 @@ +/* + * Copyright (c) 2019 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ +#ifndef ZEPHYR_INCLUDE_SYS_SYS_HEAP_H_ +#define ZEPHYR_INCLUDE_SYS_SYS_HEAP_H_ + +#include + +/* Simple, fast heap implementation. + * + * A more or less conventional segregated fit allocator with + * power-of-two buckets. + * + * Excellent space efficiency. Chunks can be split arbitrarily in 8 + * byte units. Overhead is only four bytes per allocated chunk (eight + * bytes for heaps >256kb or on 64 bit systems), plus a log2-sized + * array of 2-word bucket headers. No coarse alignment restrictions + * on blocks, they can be split and merged (in units of 8 bytes) + * arbitrarily. + * + * Simple API. Initialize at runtime with any blob of memory and not + * a macro-generated, carefully aligned static array. Allocate and + * free by user pointer and not an opaque block handle. + * + * Good fragmentation resistance. Freed blocks are always immediately + * merged with adjacent free blocks. Allocations are attempted from a + * sample of the smallest bucket that might fit, falling back rapidly + * to the smallest block guaranteed to fit. Split memory remaining in + * the chunk is always returned immediately to the heap for other + * allocation. + * + * Excellent performance with firmly bounded runtime. All operations + * are constant time (though there is a search of the smallest bucket + * that has a compile-time-configurable upper bound, setting this to + * extreme values results in an effectively linear search of the + * list), objectively fast (~hundred instructions) and and amenable to + * locked operation. + */ + +/* Note: the init_mem/bytes fields are for the static initializer to + * have somewhere to put the arguments. The actual heap metadata at + * runtime lives in the heap memory itself and this struct simply + * functions as an opaque pointer. Would be good to clean this up and + * put the two values somewhere else, though it would make + * SYS_HEAP_DEFINE a little hairy to write. + */ +struct sys_heap { + struct z_heap *heap; + void *init_mem; + size_t init_bytes; +}; + +struct z_heap_stress_result { + u32_t total_allocs; + u32_t successful_allocs; + u32_t total_frees; + u64_t accumulated_in_use_bytes; +}; + +/** @brief Initialize sys_heap + * + * Initializes a sys_heap struct to manage the specified memory. + * + * @param h Heap to initialize + * @param mem Untyped pointer to unused memory + * @param bytes Size of region pointed to by @a mem + */ +void sys_heap_init(struct sys_heap *h, void *mem, size_t bytes); + +/** @brief Allocate memory from a sys_heap + * + * Returns a pointer to a block of unused memory in the heap. This + * memory will not otherwise be used until it is freed with + * sys_heap_free(). If no memory can be allocated, NULL will be + * returned. + * + * @note The sys_heap implementation is not internally synchronized. + * No two sys_heap functions should operate on the same heap at the + * same time. All locking must be provided by the user. + * + * @param h Heap from which to allocate + * @param bytes Number of bytes requested + * @return Pointer to memory the caller can now use + */ +void *sys_heap_alloc(struct sys_heap *h, size_t bytes); + +/** @brief Free memory into a sys_heap + * + * De-allocates a pointer to memory previously returned from + * sys_heap_alloc such that it can be used for other purposes. The + * caller must not use the memory region after entry to this function. + * + * @note The sys_heap implementation is not internally synchronized. + * No two sys_heap functions should operate on the same heap at the + * same time. All locking must be provided by the user. + * + * @param h Heap to which to return the memory + * @param mem A pointer previously returned from sys_heap_alloc() + */ +void sys_heap_free(struct sys_heap *h, void *mem); + +/** @brief Validate heap integrity + * + * Validates the internal integrity of a sys_heap. Intended for unit + * test and validation code, though potentially useful as a user API + * for applications with complicated runtime reliability requirements. + * Note: this cannot catch every possible error, but if it returns + * true then the heap is in a consistent state and can correctly + * handle any sys_heap_alloc() request and free any live pointer + * returned from a previou allocation. + * + * @param h Heap to validate + * @return true, if the heap is valid, otherwise false + */ +bool sys_heap_validate(struct sys_heap *h); + +/** @brief sys_heap stress test rig + * + * Test rig for heap allocation validation. This will loop for @a + * op_count cycles, in each iteration making a random choice to + * allocate or free a pointer of randomized (power law) size based on + * heuristics designed to keep the heap in a state where it is near @a + * target_percent full. Allocation and free operations are provided + * by the caller as callbacks (i.e. this can in theory test any heap). + * Results, including counts of frees and successful/unsuccessful + * allocations, are returnewd via the @result struct. + * + * @param alloc Callback to perform an allocation. Passes back the @a + * arg parameter as a context handle. + * @param free Callback to perform a free of a pointer returned from + * @a alloc. Passes back the @a arg parameter as a + * context handle. + * @param arg Context handle to pass back to the callbacks + * @param total_bytes Size of the byte array the heap was initialized in + * @param op_count How many iterations to test + * @param scratch_mem A pointer to scratch memory to be used by the + * test. Should be about 1/2 the size of the heap + * for tests that need to stress fragmentation. + * @param scratch_bytes Size of the memory pointed to by @a scratch_mem + * @param target_percent Percentage fill value (1-100) to which the + * random allocation choices will seek. High + * values will result in significant allocation + * failures and a very fragmented heap. + * @param result Struct into which to store test results. + */ +void sys_heap_stress(void *(*alloc)(void *arg, size_t bytes), + void (*free)(void *arg, void *p), + void *arg, size_t total_bytes, + u32_t op_count, + void *scratch_mem, size_t scratch_bytes, + int target_percent, + struct z_heap_stress_result *result); + +#endif /* ZEPHYR_INCLUDE_SYS_SYS_HEAP_H_ */ diff --git a/lib/os/CMakeLists.txt b/lib/os/CMakeLists.txt index 4c2238d093b2b..720d60e79b793 100644 --- a/lib/os/CMakeLists.txt +++ b/lib/os/CMakeLists.txt @@ -16,6 +16,8 @@ zephyr_sources( thread_entry.c timeutil.c work_q.c + heap.c + heap-validate.c ) zephyr_sources_ifdef(CONFIG_JSON_LIBRARY json.c) diff --git a/lib/os/Kconfig b/lib/os/Kconfig index b89ddd1e2ee7a..cfe136b369244 100644 --- a/lib/os/Kconfig +++ b/lib/os/Kconfig @@ -24,4 +24,29 @@ config BASE64 help Enable base64 encoding and decoding functionality +config SYS_HEAP_VALIDATE + bool "Enable internal heap validity checking" + help + The sys_heap implementation is instrumented for extensive + internal validation. Leave this off by default, unless + modifying the heap code or (maybe) when running in + environments that require sensitive detection of memory + corruption. + +config SYS_HEAP_ALLOC_LOOPS + int "Number of tries in the inner heap allocation loop" + default 3 + help + The sys_heap allocator bounds the number of tries from the + smallest chunk level (the one that might not fit the + requested allocation) to maintain constant time performance. + Setting this to a high level will cause the heap to return + more successful allocations in situations of high + fragmentation, at the cost of potentially significant + (linear time) searching of the free list. The default is + three, which results in an allocator with good statistical + properties ("most" allocations that fit will succeed) but + keeps the maximum runtime at a tight bound so that the heap + is useful in locked or ISR contexts. + endmenu diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c new file mode 100644 index 0000000000000..92b0ac9933b77 --- /dev/null +++ b/lib/os/heap-validate.c @@ -0,0 +1,307 @@ +/* + * Copyright (c) 2019 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include +#include "heap.h" + +/* White-box sys_heap validation code. Uses internal data structures. + * Not expected to be useful in production apps. This checks every + * header field of every chunk and returns true if the totality of the + * data structure is a valid heap. It doesn't necessarily tell you + * that it is the CORRECT heap given the history of alloc/free calls + * that it can't inspect. In a pathological case, you can imagine + * something scribbling a copy of a previously-valid heap on top of a + * running one and corrupting it. YMMV. + */ + +static size_t max_chunkid(struct z_heap *h) +{ + return h->len - bytes_to_chunksz(h, 1); +} + +static bool in_bounds(struct z_heap *h, chunkid_t c) +{ + return (c >= h->chunk0) + && (c <= max_chunkid(h)) + && (size(h, c) < h->len); +} + +static bool valid_chunk(struct z_heap *h, chunkid_t c) +{ + return (size(h, c) > 0 + && (c + size(h, c) <= h->len) + && in_bounds(h, c) + && ((c == h->chunk0) || in_bounds(h, c - left_size(h, c))) + && (used(h, c) || in_bounds(h, free_prev(h, c))) + && (used(h, c) || in_bounds(h, free_next(h, c)))); +} + +/* Validate multiple state dimensions for the bucket "next" pointer + * and see that they match. Probably should unify the design a + * bit... + */ +static inline void check_nexts(struct z_heap *h, int bidx) +{ + struct z_heap_bucket *b = &h->buckets[bidx]; + + bool emptybit = (h->avail_buckets & (1 << bidx)) == 0; + bool emptylist = b->next == 0; + bool emptycount = b->list_size == 0; + bool empties_match = emptybit == emptylist && emptybit == emptycount; + + (void)empties_match; + CHECK(empties_match); + + if (b->next != 0) { + CHECK(valid_chunk(h, b->next)); + } + + if (b->list_size == 2) { + CHECK(free_next(h, b->next) == free_prev(h, b->next)); + CHECK(free_next(h, b->next) != b->next); + } else if (b->list_size == 1) { + CHECK(free_next(h, b->next) == free_prev(h, b->next)); + CHECK(free_next(h, b->next) == b->next); + } +} + +bool sys_heap_validate(struct sys_heap *heap) +{ + struct z_heap *h = heap->heap; + chunkid_t c; + + /* Check the free lists: entry count should match, empty bit + * should be correct, and all chunk entries should point into + * valid unused chunks. Mark those chunks USED, temporarily. + */ + for (int b = 0; b <= bucket_idx(h, h->len); b++) { + chunkid_t c0 = h->buckets[b].next; + u32_t n = 0; + + check_nexts(h, b); + + for (c = c0; c != 0 && (n == 0 || c != c0); n++, c = free_next(h, c)) { + if (!valid_chunk(h, c)) { + return false; + } + chunk_set_used(h, c, true); + } + + bool empty = (h->avail_buckets & (1 << b)) == 0; + bool zero = n == 0; + + if (empty != zero) { + return false; + } + + if (empty && h->buckets[b].next != 0) { + return false; + } + + if (n != h->buckets[b].list_size) { + return false; + } + } + + /* Walk through the chunks linearly, verifying sizes and end + * pointer and that the all chunks are now USED (i.e. all free + * blocks were found during enumeration). Mark all blocks + * UNUSED + */ + size_t prev_size = 0; + + for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { + if (!valid_chunk(h, c)) { + return false; + } + if (!used(h, c)) { + return false; + } + + if (c != h->chunk0) { + if (left_size(h, c) != prev_size) { + return false; + } + } + prev_size = size(h, c); + + chunk_set_used(h, c, false); + } + if (c != h->len) { + return false; /* Should have exactly consumed the buffer */ + } + + /* Go through the free lists again checking that the linear + * pass caught all the blocks and that they now show UNUSED. + * Mark them USED. + */ + for (int b = 0; b <= bucket_idx(h, h->len); b++) { + chunkid_t c0 = h->buckets[b].next; + int n = 0; + + if (c0 == 0) { + continue; + } + + for (c = c0; n == 0 || c != c0; n++, c = free_next(h, c)) { + if (used(h, c)) { + return false; + } + chunk_set_used(h, c, true); + } + } + + /* Now we are valid, but have managed to invert all the in-use + * fields. One more linear pass to fix them up + */ + for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { + chunk_set_used(h, c, !used(h, c)); + } + return true; +} + +struct z_heap_stress_rec { + void *(*alloc)(void *arg, size_t bytes); + void (*free)(void *arg, void *p); + void *arg; + size_t total_bytes; + struct z_heap_stress_block *blocks; + size_t nblocks; + size_t blocks_alloced; + size_t bytes_alloced; + u32_t target_percent; +}; + +struct z_heap_stress_block { + void *ptr; + size_t sz; +}; + +/* Very simple LCRNG (from https://nuclear.llnl.gov/CNP/rng/rngman/node4.html) + * + * Here to guarantee cross-platform test repeatability. + */ +static u32_t rand32(void) +{ + static u64_t state = 123456789; /* seed */ + + state = state * 2862933555777941757UL + 3037000493UL; + + return (u32_t)(state >> 32); +} + +static bool rand_alloc_choice(struct z_heap_stress_rec *sr) +{ + /* Edge cases: no blocks allocated, and no space for a new one */ + if (sr->blocks_alloced == 0) { + return true; + } else if (sr->blocks_alloced >= sr->nblocks) { + return false; + } + + /* The way this works is to scale the chance of choosing to + * allocate vs. free such that it's even odds when the heap is + * at the target percent, with linear tapering on the low + * slope (i.e. we choose to always allocate with an empty + * heap, allocate 50% of the time when the heap is exactly at + * the target, and always free when above the target). In + * practice, the operations aren't quite symmetric (you can + * always free, but your allocation might fail), and the units + * aren't matched (we're doing math based on bytes allocated + * and ignoring the overhead) but this is close enough. And + * yes, the math here is coarse (in units of percent), but + * that's good enough and fits well inside 32 bit quantities. + * (Note precision issue when heap size is above 40MB + * though!). + */ + __ASSERT(sr->total_bytes < 0xffffffffU / 100, "too big for u32!"); + u32_t full_pct = (100 * sr->bytes_alloced) / sr->total_bytes; + u32_t target = sr->target_percent ? sr->target_percent : 1; + u32_t free_chance = 0xffffffffU; + + if (full_pct < sr->target_percent) { + free_chance = full_pct * (0x80000000U / target); + } + + return rand32() > free_chance; +} + +/* Chooses a size of block to allocate, logarithmically favoring + * smaller blocks (i.e. blocks twice as large are half as frequent + */ +static size_t rand_alloc_size(struct z_heap_stress_rec *sr) +{ + ARG_UNUSED(sr); + + /* Min scale of 4 means that the half of the requests in the + * smallest size have an average size of 8 + */ + int scale = 4 + __builtin_clz(rand32()); + + return rand32() & ((1 << scale) - 1); +} + +/* Returns the index of a randomly chosen block to free */ +static size_t rand_free_choice(struct z_heap_stress_rec *sr) +{ + return rand32() % sr->blocks_alloced; +} + +/* General purpose heap stress test. Takes function pointers to allow + * for testing multiple heap APIs with the same rig. The alloc and + * free functions are passed back the argument as a context pointer. + * The "log" function is for readable user output. The total_bytes + * argument should reflect the size of the heap being tested. The + * scratch array is used to store temporary state and should be sized + * about half as large as the heap itself. Returns true on success. + */ +void sys_heap_stress(void *(*alloc)(void *arg, size_t bytes), + void (*free)(void *arg, void *p), + void *arg, size_t total_bytes, + u32_t op_count, + void *scratch_mem, size_t scratch_bytes, + int target_percent, + struct z_heap_stress_result *result) +{ + struct z_heap_stress_rec sr = { + .alloc = alloc, + .free = free, + .arg = arg, + .total_bytes = total_bytes, + .blocks = scratch_mem, + .nblocks = scratch_bytes / sizeof(struct z_heap_stress_block), + .target_percent = target_percent, + }; + + *result = (struct z_heap_stress_result) {0}; + + for (u32_t i = 0; i < op_count; i++) { + if (rand_alloc_choice(&sr)) { + size_t sz = rand_alloc_size(&sr); + void *p = sr.alloc(sr.arg, sz); + + result->total_allocs++; + if (p != NULL) { + result->successful_allocs++; + sr.blocks[sr.blocks_alloced].ptr = p; + sr.blocks[sr.blocks_alloced].sz = sz; + sr.blocks_alloced++; + sr.bytes_alloced += sz; + } + } else { + int b = rand_free_choice(&sr); + void *p = sr.blocks[b].ptr; + size_t sz = sr.blocks[b].sz; + + result->total_frees++; + sr.blocks[b] = sr.blocks[sr.blocks_alloced - 1]; + sr.blocks_alloced--; + sr.bytes_alloced -= sz; + sr.free(sr.arg, p); + } + result->accumulated_in_use_bytes += sr.bytes_alloced; + } +} diff --git a/lib/os/heap.c b/lib/os/heap.c new file mode 100644 index 0000000000000..9b834c3cbea4b --- /dev/null +++ b/lib/os/heap.c @@ -0,0 +1,239 @@ +/* + * Copyright (c) 2019 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include +#include "heap.h" + +static void *chunk_mem(struct z_heap *h, chunkid_t c) +{ + u8_t *ret = ((u8_t *)&h->buf[c]) + chunk_header_bytes(h); + + CHECK(!(((size_t)ret) & (big_heap(h) ? 7 : 3))); + + return ret; +} + +static void free_list_remove(struct z_heap *h, int bidx, + chunkid_t c) +{ + struct z_heap_bucket *b = &h->buckets[bidx]; + + CHECK(!used(h, c)); + CHECK(b->next != 0); + CHECK(b->list_size > 0); + CHECK((((h->avail_buckets & (1 << bidx)) == 0) + == (h->buckets[bidx].next == 0))); + + b->list_size--; + + if (b->list_size == 0) { + h->avail_buckets &= ~(1 << bidx); + b->next = 0; + } else { + chunkid_t first = free_prev(h, c), second = free_next(h, c); + + b->next = second; + chunk_set(h, first, FREE_NEXT, second); + chunk_set(h, second, FREE_PREV, first); + } +} + +static void free_list_add(struct z_heap *h, chunkid_t c) +{ + int b = bucket_idx(h, size(h, c)); + + if (h->buckets[b].list_size++ == 0) { + CHECK(h->buckets[b].next == 0); + CHECK((h->avail_buckets & (1 << b)) == 0); + + /* Empty list, first item */ + h->avail_buckets |= (1 << b); + h->buckets[b].next = c; + chunk_set(h, c, FREE_PREV, c); + chunk_set(h, c, FREE_NEXT, c); + } else { + /* Insert before (!) the "next" pointer */ + chunkid_t second = h->buckets[b].next; + chunkid_t first = free_prev(h, second); + + chunk_set(h, c, FREE_PREV, first); + chunk_set(h, c, FREE_NEXT, second); + chunk_set(h, first, FREE_NEXT, c); + chunk_set(h, second, FREE_PREV, c); + } + + CHECK(h->avail_buckets & (1 << bucket_idx(h, size(h, c)))); +} + +static ALWAYS_INLINE bool last_chunk(struct z_heap *h, chunkid_t c) +{ + return (c + size(h, c)) == h->len; +} + +/* Allocates (fit check has already been perfomred) from the next + * chunk at the specified bucket level + */ +static void *split_alloc(struct z_heap *h, int bidx, size_t sz) +{ + CHECK(h->buckets[bidx].next != 0 + && sz <= size(h, h->buckets[bidx].next)); + + chunkid_t c = h->buckets[bidx].next; + + free_list_remove(h, bidx, c); + + /* Split off remainder if it's usefully large */ + size_t rem = size(h, c) - sz; + + CHECK(rem < h->len); + + if (rem >= (big_heap(h) ? 2 : 1)) { + chunkid_t c2 = c + sz; + chunkid_t c3 = right_chunk(h, c); + + chunk_set(h, c, SIZE_AND_USED, sz); + chunk_set(h, c2, SIZE_AND_USED, rem); + chunk_set(h, c2, LEFT_SIZE, sz); + if (!last_chunk(h, c2)) { + chunk_set(h, c3, LEFT_SIZE, rem); + } + free_list_add(h, c2); + } + + chunk_set_used(h, c, true); + + return chunk_mem(h, c); +} + +void sys_heap_free(struct sys_heap *heap, void *mem) +{ + if (mem == NULL) { + return; /* ISO C free() semantics */ + } + + struct z_heap *h = heap->heap; + chunkid_t c = ((u8_t *)mem - chunk_header_bytes(h) + - (u8_t *)h->buf) / CHUNK_UNIT; + + /* Merge with right chunk? We can just absorb it. */ + if (!last_chunk(h, c) && !used(h, right_chunk(h, c))) { + chunkid_t rc = right_chunk(h, c); + size_t newsz = size(h, c) + size(h, rc); + + free_list_remove(h, bucket_idx(h, size(h, rc)), rc); + chunk_set(h, c, SIZE_AND_USED, newsz); + if (!last_chunk(h, c)) { + chunk_set(h, right_chunk(h, c), LEFT_SIZE, newsz); + } + } + + /* Merge with left chunk? It absorbs us. */ + if (c != h->chunk0 && !used(h, left_chunk(h, c))) { + chunkid_t lc = left_chunk(h, c); + chunkid_t rc = right_chunk(h, c); + size_t csz = size(h, c); + size_t merged_sz = csz + size(h, lc); + + free_list_remove(h, bucket_idx(h, size(h, lc)), lc); + chunk_set(h, lc, SIZE_AND_USED, merged_sz); + if (!last_chunk(h, lc)) { + chunk_set(h, rc, LEFT_SIZE, merged_sz); + } + + c = lc; + } + + chunk_set_used(h, c, false); + free_list_add(h, c); +} + +void *sys_heap_alloc(struct sys_heap *heap, size_t bytes) +{ + struct z_heap *h = heap->heap; + size_t sz = bytes_to_chunksz(h, bytes); + int bi = bucket_idx(h, sz); + struct z_heap_bucket *b = &h->buckets[bi]; + + if (bytes == 0 || bi > bucket_idx(h, h->len)) { + return NULL; + } + + /* First try a bounded count of items from the minimal bucket + * size. These may not fit, trying (e.g.) three means that + * (assuming that chunk sizes are evenly distributed[1]) we + * have a 7/8 chance of finding a match, thus keeping the + * number of such blocks consumed by allocation higher than + * the number of smaller blocks created by fragmenting larger + * ones. + * + * [1] In practice, they are never evenly distributed, of + * course. But even in pathological situations we still + * maintain our constant time performance and at worst see + * fragmentation waste of the order of the block allocated + * only. + */ + int loops = MIN(b->list_size, CONFIG_SYS_HEAP_ALLOC_LOOPS); + + for (int i = 0; i < loops; i++) { + CHECK(b->next != 0); + if (size(h, b->next) >= sz) { + return split_alloc(h, bi, sz); + } else { + b->next = free_next(h, b->next); + } + } + + /* Otherwise pick the smallest non-empty bucket guaranteed to + * fit and use that unconditionally. + */ + size_t bmask = h->avail_buckets & ~((1 << (bi + 1)) - 1); + + if ((bmask & h->avail_buckets) != 0) { + int minbucket = __builtin_ctz(bmask & h->avail_buckets); + + return split_alloc(h, minbucket, sz); + } + + return NULL; +} + +void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) +{ + /* Must fit in a 32 bit count of u64's */ +#if __SIZEOF_SIZE_T__ > 4 + CHECK(bytes < 0x800000000ULL); +#endif + + /* Round the start up, the end down */ + size_t addr = ((size_t)mem + CHUNK_UNIT - 1) & ~(CHUNK_UNIT - 1); + size_t end = ((size_t)mem + bytes) & ~(CHUNK_UNIT - 1); + size_t buf_sz = (end - addr) / CHUNK_UNIT; + size_t hdr_chunks = chunksz(sizeof(struct z_heap)); + + CHECK(end > addr); + + struct z_heap *h = (struct z_heap *)addr; + + heap->heap = (struct z_heap *)addr; + h->buf = (u64_t *)addr; + h->buckets = (void *)(addr + CHUNK_UNIT * hdr_chunks); + h->len = buf_sz; + h->size_mask = (1 << (big_heap(h) ? 31 : 15)) - 1; + h->avail_buckets = 0; + + size_t buckets_bytes = ((bucket_idx(h, buf_sz) + 1) + * sizeof(struct z_heap_bucket)); + + h->chunk0 = hdr_chunks + chunksz(buckets_bytes); + + for (int i = 0; i <= bucket_idx(heap->heap, heap->heap->len); i++) { + heap->heap->buckets[i].list_size = 0; + heap->heap->buckets[i].next = 0; + } + + chunk_set(h, h->chunk0, SIZE_AND_USED, buf_sz - h->chunk0); + free_list_add(h, h->chunk0); +} diff --git a/lib/os/heap.h b/lib/os/heap.h new file mode 100644 index 0000000000000..c73b3cf254b50 --- /dev/null +++ b/lib/os/heap.h @@ -0,0 +1,164 @@ +/* + * Copyright (c) 2019 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ +#ifndef ZEPHYR_INCLUDE_LIB_OS_HEAP_H_ +#define ZEPHYR_INCLUDE_LIB_OS_HEAP_H_ + +/* + * Internal heap APIs + */ + +/* Theese validation checks are non-trivially expensive, so enable + * only when debugging the heap code. They shouldn't be routine + * assertions. + */ +#ifdef CONFIG_SYS_HEAP_VALIDATE +#define CHECK(x) __ASSERT(x, "") +#else +#define CHECK(x) /**/ +#endif + +/* Chunks are identified by their offset in 8 byte units from the + * first address in the buffer (a zero-valued chunkid_t is used as a + * null; that chunk would always point into the metadata at the start + * of the heap and cannot be allocated). They are prefixed by a + * variable size header that depends on the size of the heap. Heaps + * with fewer than 2^15 units (256kb) of storage use shorts to store + * the fields, otherwise the units are 32 bit integers for a 16Gb heap + * space (larger spaces really aren't in scope for this code, but + * could be handled similarly I suppose). Because of that design + * there's a certain amount of boilerplate API needed to expose the + * field accessors since we can't use natural syntax. + * + * The fields are: + * SIZE_AND_USED: the total size (including header) of the chunk in + * 8-byte units. The top bit stores a "used" flag. + * LEFT_SIZE: The size of the left (next lower chunk in memory) + * neighbor chunk. + * FREE_PREV: Chunk ID of the previous node in a free list. + * FREE_NEXT: Chunk ID of the next node in a free list. + * + * The free lists are circular lists, one for each power-of-two size + * category. The free list pointers exist only for free chunks, + * obviously. This memory is part of the user's buffer when + * allocated. + */ +typedef size_t chunkid_t; + +#define CHUNK_UNIT 8 + +enum chunk_fields { SIZE_AND_USED, LEFT_SIZE, FREE_PREV, FREE_NEXT }; + +struct z_heap { + u64_t *buf; + struct z_heap_bucket *buckets; + u32_t len; + u32_t size_mask; + u32_t chunk0; + u32_t avail_buckets; +}; + +struct z_heap_bucket { + chunkid_t next; + size_t list_size; +}; + +static inline bool big_heap(struct z_heap *h) +{ + return sizeof(size_t) > 4 || h->len > 0x7fff; +} + +static inline size_t chunk_field(struct z_heap *h, chunkid_t c, + enum chunk_fields f) +{ + void *cmem = &h->buf[c]; + + if (big_heap(h)) { + return ((u32_t *)cmem)[f]; + } else { + return ((u16_t *)cmem)[f]; + } +} + +static inline void chunk_set(struct z_heap *h, chunkid_t c, + enum chunk_fields f, chunkid_t val) +{ + CHECK(c >= h->chunk0 && c < h->len); + CHECK((val & ~((h->size_mask << 1) + 1)) == 0); + CHECK((val & h->size_mask) < h->len); + + void *cmem = &h->buf[c]; + + if (big_heap(h)) { + ((u32_t *)cmem)[f] = (u32_t) val; + } else { + ((u16_t *)cmem)[f] = (u16_t) val; + } +} + +static inline chunkid_t used(struct z_heap *h, chunkid_t c) +{ + return (chunk_field(h, c, SIZE_AND_USED) & ~h->size_mask) != 0; +} + +static ALWAYS_INLINE chunkid_t size(struct z_heap *h, chunkid_t c) +{ + return chunk_field(h, c, SIZE_AND_USED) & h->size_mask; +} + +static inline void chunk_set_used(struct z_heap *h, chunkid_t c, + bool used) +{ + chunk_set(h, c, SIZE_AND_USED, + size(h, c) | (used ? (h->size_mask + 1) : 0)); +} + +static inline chunkid_t left_size(struct z_heap *h, chunkid_t c) +{ + return chunk_field(h, c, LEFT_SIZE); +} + +static inline chunkid_t free_prev(struct z_heap *h, chunkid_t c) +{ + return chunk_field(h, c, FREE_PREV); +} + +static inline chunkid_t free_next(struct z_heap *h, chunkid_t c) +{ + return chunk_field(h, c, FREE_NEXT); +} + +static inline chunkid_t left_chunk(struct z_heap *h, chunkid_t c) +{ + return c - left_size(h, c); +} + +static inline chunkid_t right_chunk(struct z_heap *h, chunkid_t c) +{ + return c + size(h, c); +} + +static inline size_t chunk_header_bytes(struct z_heap *h) +{ + return big_heap(h) ? 8 : 4; +} + +static inline size_t chunksz(size_t bytes) +{ + return (bytes + CHUNK_UNIT - 1) / CHUNK_UNIT; +} + +static inline size_t bytes_to_chunksz(struct z_heap *h, size_t bytes) +{ + return chunksz(chunk_header_bytes(h) + bytes); +} + +static int bucket_idx(struct z_heap *h, size_t sz) +{ + /* A chunk of size 2 is the minimum size on big heaps */ + return 31 - __builtin_clz(sz) - (big_heap(h) ? 1 : 0); +} + +#endif /* ZEPHYR_INCLUDE_LIB_OS_HEAP_H_ */ From 8fe12097fc3f9162f1397a28fd30909ce85cd5f3 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 17 Jul 2019 10:31:24 -0700 Subject: [PATCH 02/15] tests/lib: Add sys_heap test Use the white box validation and test rig added as part of the sys_heap work. Add a layer that puts hashed cookies into the blocks to detect corruption, check the validity state after every operation, and enumerate a few different usage patterns: + Small heap, "real world" allocation where the heap is about half full and most allocations succeed. + Small heap, "fragmentation runaway" scenario where most allocations start failing, but the heap must remain consistent. + Big heap. We can't test this with the same exhaustive coverage (many re/allocations for every byte of storage) for performance reasons, but we do what we can. Signed-off-by: Andy Ross --- tests/lib/heap/CMakeLists.txt | 8 ++ tests/lib/heap/prj.conf | 2 + tests/lib/heap/src/main.c | 204 ++++++++++++++++++++++++++++++++++ tests/lib/heap/testcase.yaml | 3 + 4 files changed, 217 insertions(+) create mode 100644 tests/lib/heap/CMakeLists.txt create mode 100644 tests/lib/heap/prj.conf create mode 100644 tests/lib/heap/src/main.c create mode 100644 tests/lib/heap/testcase.yaml diff --git a/tests/lib/heap/CMakeLists.txt b/tests/lib/heap/CMakeLists.txt new file mode 100644 index 0000000000000..7fbd555f16de5 --- /dev/null +++ b/tests/lib/heap/CMakeLists.txt @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.13.1) +include($ENV{ZEPHYR_BASE}/cmake/app/boilerplate.cmake NO_POLICY_SCOPE) +project(heap) + +FILE(GLOB app_sources src/*.c) +target_sources(app PRIVATE ${app_sources}) diff --git a/tests/lib/heap/prj.conf b/tests/lib/heap/prj.conf new file mode 100644 index 0000000000000..a41fd3e078889 --- /dev/null +++ b/tests/lib/heap/prj.conf @@ -0,0 +1,2 @@ +CONFIG_ZTEST=y +CONFIG_SYS_HEAP_VALIDATE=y diff --git a/tests/lib/heap/src/main.c b/tests/lib/heap/src/main.c new file mode 100644 index 0000000000000..53e1218716f73 --- /dev/null +++ b/tests/lib/heap/src/main.c @@ -0,0 +1,204 @@ +/* + * Copyright (c) 2019 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include +#include + +/* Sort of a mess to detect. But basically MEMSZ becomes the minimum + * value specified by either of the two configuration mechanisms for + * system RAM size, or else INT_MAX if neither is specified (this is + * true right now only for native_posix, where a 256k array will have + * not trobule). + */ + +#ifdef DT_SRAM_SIZE +# define SZ1 DT_SRAM_SIZE +#else +# define SZ1 INT_MAX +#endif + +#ifdef CONFIG_SRAM_SIZE +# define SZ2 CONFIG_SRAM_SIZE +#else +# define SZ2 INT_MAX +#endif + +#define MEMSZ MIN(SZ1, SZ2) + +#define BIG_HEAP_SZ MIN(256 * 1024, MEMSZ / 2) +#define SMALL_HEAP_SZ 2048 + +char heapmem[BIG_HEAP_SZ]; + +/* How many alloc/free operations are tested on each heap. Two per + * byte of heap sounds about right to get exhaustive coverage without + * blowing too many cycles + */ +#define ITERATION_COUNT (2 * SMALL_HEAP_SZ) + +char scratchmem[sizeof(heapmem) / 2]; + +/* Simple dumb hash function of the size and address */ +static size_t fill_token(void *p, size_t sz) +{ + size_t pi = (size_t) p; + + return (pi * sz) ^ ((sz ^ 0xea6d) * ((pi << 11) | (pi >> 21))); +} + +/* Puts markers at the start and end of a block to ensure that nothing + * scribbled on it while it was allocated. The first word is the + * block size. The second and last (if they fits) are a hashed "fill + * token" + */ +static void fill_block(void *p, size_t sz) +{ + if (p == NULL) { + return; + } + + size_t tok = fill_token(p, sz); + + ((size_t *)p)[0] = sz; + + if (sz >= 2 * sizeof(size_t)) { + ((size_t *)p)[1] = tok; + } + + if (sz > 3*sizeof(size_t)) { + ((size_t *)p)[sz / sizeof(size_t) - 1] = tok; + } +} + +/* Checks markers just before freeing a block */ +static void check_fill(void *p) +{ + size_t sz = ((size_t *)p)[0]; + size_t tok = fill_token(p, sz); + + zassert_true(sz > 0, ""); + + if (sz >= 2 * sizeof(size_t)) { + zassert_true(((size_t *)p)[1] == tok, ""); + } + + if (sz > 3 * sizeof(size_t)) { + zassert_true(((size_t *)p)[sz / sizeof(size_t) - 1] == tok, ""); + } +} + +void *testalloc(void *arg, size_t bytes) +{ + void *ret = sys_heap_alloc(arg, bytes); + + fill_block(ret, bytes); + sys_heap_validate(arg); + return ret; +} + +void testfree(void *arg, void *p) +{ + check_fill(p); + sys_heap_free(arg, p); + sys_heap_validate(arg); +} + +static void log_result(u32_t sz, struct z_heap_stress_result *r) +{ + u32_t tot = r->total_allocs + r->total_frees; + u32_t avg = (u32_t)((r->accumulated_in_use_bytes + tot/2) / tot); + u32_t avg_pct = (u32_t)(100ULL * avg + sz / 2) / sz; + u32_t succ_pct = ((100ULL * r->successful_allocs + r->total_allocs / 2) + / r->total_allocs); + + TC_PRINT("successful allocs: %d/%d (%d%%), frees: %d," + " avg usage: %d/%d (%d%%)\n", + r->successful_allocs, r->total_allocs, succ_pct, + r->total_frees, avg, sz, avg_pct); +} + +/* Do a heavy test over a small heap, with many iterations that need + * to reuse memory repeatedly. Target 50% fill, as that setting tends + * to prevent runaway fragmentation and most allocations continue to + * succeed in steady state. + */ +static void test_small_heap(void) +{ + struct sys_heap heap; + struct z_heap_stress_result result; + + TC_PRINT("Testing small (%d byte) heap\n", SMALL_HEAP_SZ); + + sys_heap_init(&heap, heapmem, SMALL_HEAP_SZ); + zassert_true(sys_heap_validate(&heap), ""); + sys_heap_stress(testalloc, testfree, &heap, + SMALL_HEAP_SZ, ITERATION_COUNT, + scratchmem, sizeof(scratchmem), + 50, &result); + + log_result(SMALL_HEAP_SZ, &result); +} + +/* Very similar, but tests a fragmentation runaway scenario where we + * target 100% fill and end up breaking memory up into maximally + * fragmented blocks (i.e. small allocations always grab and split the + * bigger chunks). Obviously success rates in alloc will be very low, + * but consistency should still be maintained. Paradoxically, fill + * level is not much better than the 50% target due to all the + * fragmentation overhead (also the way we do accounting: we are + * counting bytes requested, so if you ask for a 3 byte block and + * receive a 8 byte minimal chunk, we still count that as 5 bytes of + * waste). + */ +static void test_fragmentation(void) +{ + struct sys_heap heap; + struct z_heap_stress_result result; + + TC_PRINT("Testing maximally fragmented (%d byte) heap\n", SMALL_HEAP_SZ); + + sys_heap_init(&heap, heapmem, SMALL_HEAP_SZ); + zassert_true(sys_heap_validate(&heap), ""); + sys_heap_stress(testalloc, testfree, &heap, + SMALL_HEAP_SZ, ITERATION_COUNT, + scratchmem, sizeof(scratchmem), + 100, &result); + + log_result(SMALL_HEAP_SZ, &result); +} + +/* The heap block format changes for heaps with more than 2^15 chunks, + * so test that case too. This can be too large to iterate over + * exhaustively with good performance, so the relative operation count + * and fragmentation is going to be lower. + */ +static void test_big_heap(void) +{ + struct sys_heap heap; + struct z_heap_stress_result result; + + TC_PRINT("Testing big (%d byte) heap\n", BIG_HEAP_SZ); + + sys_heap_init(&heap, heapmem, BIG_HEAP_SZ); + zassert_true(sys_heap_validate(&heap), ""); + sys_heap_stress(testalloc, testfree, &heap, + BIG_HEAP_SZ, ITERATION_COUNT, + scratchmem, sizeof(scratchmem), + 100, &result); + + log_result(BIG_HEAP_SZ, &result); +} + +void test_main(void) +{ + ztest_test_suite(lib_heap_test, + ztest_unit_test(test_small_heap), + ztest_unit_test(test_fragmentation), + ztest_unit_test(test_big_heap) + ); + + ztest_run_test_suite(lib_heap_test); +} diff --git a/tests/lib/heap/testcase.yaml b/tests/lib/heap/testcase.yaml new file mode 100644 index 0000000000000..9172d1ed76774 --- /dev/null +++ b/tests/lib/heap/testcase.yaml @@ -0,0 +1,3 @@ +tests: + lib.heap: + tags: heap From 61518ef810d7a161e0b904dac4ae1ff7d3c92dc1 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Wed, 23 Oct 2019 19:49:54 -0400 Subject: [PATCH 03/15] tests/lib/heap: size memory properly to avoid crash The DT_SRAM_SIZE and CONFIG_SRAM_SIZE symbols are expressed in KB and not in bytes. Without this consideration, BIG_HEAP_SZ may end up being equal to 32, making scratchmem[] to appear only 32 bytes after the beginning of heapmem[] in memory while test_small_heap() uses the later assuming it is at least 2048 bytes. Fun stuff ensues. Signed-off-by: Nicolas Pitre --- tests/lib/heap/src/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/lib/heap/src/main.c b/tests/lib/heap/src/main.c index 53e1218716f73..93d7d79f3a7d5 100644 --- a/tests/lib/heap/src/main.c +++ b/tests/lib/heap/src/main.c @@ -15,13 +15,13 @@ */ #ifdef DT_SRAM_SIZE -# define SZ1 DT_SRAM_SIZE +# define SZ1 (DT_SRAM_SIZE * 1024) #else # define SZ1 INT_MAX #endif #ifdef CONFIG_SRAM_SIZE -# define SZ2 CONFIG_SRAM_SIZE +# define SZ2 (CONFIG_SRAM_SIZE * 1024) #else # define SZ2 INT_MAX #endif @@ -30,6 +30,7 @@ #define BIG_HEAP_SZ MIN(256 * 1024, MEMSZ / 2) #define SMALL_HEAP_SZ 2048 +BUILD_ASSERT(BIG_HEAP_SZ > SMALL_HEAP_SZ); char heapmem[BIG_HEAP_SZ]; From 0e5b396661f931e0fe1fc802dca60932e8fc5214 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Fri, 25 Oct 2019 15:46:48 -0400 Subject: [PATCH 04/15] tests/lib/heap: make sure memory size is properly accounted for The em_starterkit_em7d_normal platform defines neither DT_SRAM_SIZE nor CONFIG_SRAM_SIZE while its linker script indicates only 64KB is available. And platforms with 16KB or less simply have too little RAM to accommodate this test. Signed-off-by: Nicolas Pitre --- tests/lib/heap/boards/em_starterkit_em7d_normal.conf | 1 + tests/lib/heap/testcase.yaml | 1 + 2 files changed, 2 insertions(+) create mode 100644 tests/lib/heap/boards/em_starterkit_em7d_normal.conf diff --git a/tests/lib/heap/boards/em_starterkit_em7d_normal.conf b/tests/lib/heap/boards/em_starterkit_em7d_normal.conf new file mode 100644 index 0000000000000..f229a7c1e25eb --- /dev/null +++ b/tests/lib/heap/boards/em_starterkit_em7d_normal.conf @@ -0,0 +1 @@ +CONFIG_SRAM_SIZE=64 diff --git a/tests/lib/heap/testcase.yaml b/tests/lib/heap/testcase.yaml index 9172d1ed76774..97421fa9fd5f5 100644 --- a/tests/lib/heap/testcase.yaml +++ b/tests/lib/heap/testcase.yaml @@ -1,3 +1,4 @@ tests: lib.heap: tags: heap + min_ram: 32 From 4ec46b5e8d371dbfe4d5b47bba3b5ae683145308 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 24 Sep 2019 20:47:24 -0400 Subject: [PATCH 05/15] sys_heap: some cleanups to make the code clearer First, some renames to make accessors more explicit: size() --> chunk_size() used() --> chunk_used() free_prev() --> prev_free_chunk() free_next() --> next_free_chunk() Then, the return type of chunk_size() is changed from chunkid_t to size_t, and chunk_used() from chunkid_t to bool. The left_size() accessor is used only once and can be easily substituted by left_chunk(), so it is removed. And in free_list_add() the variable b is renamed to bi so to be consistent with usage in sys_heap_alloc(). Signed-off-by: Nicolas Pitre --- lib/os/heap-validate.c | 36 ++++++++++++++++---------------- lib/os/heap.c | 47 +++++++++++++++++++++--------------------- lib/os/heap.h | 19 +++++++---------- 3 files changed, 49 insertions(+), 53 deletions(-) diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c index 92b0ac9933b77..3248577cb1f27 100644 --- a/lib/os/heap-validate.c +++ b/lib/os/heap-validate.c @@ -26,17 +26,17 @@ static bool in_bounds(struct z_heap *h, chunkid_t c) { return (c >= h->chunk0) && (c <= max_chunkid(h)) - && (size(h, c) < h->len); + && (chunk_size(h, c) < h->len); } static bool valid_chunk(struct z_heap *h, chunkid_t c) { - return (size(h, c) > 0 - && (c + size(h, c) <= h->len) + return (chunk_size(h, c) > 0 + && (c + chunk_size(h, c) <= h->len) && in_bounds(h, c) - && ((c == h->chunk0) || in_bounds(h, c - left_size(h, c))) - && (used(h, c) || in_bounds(h, free_prev(h, c))) - && (used(h, c) || in_bounds(h, free_next(h, c)))); + && ((c == h->chunk0) || in_bounds(h, left_chunk(h, c))) + && (chunk_used(h, c) || in_bounds(h, prev_free_chunk(h, c))) + && (chunk_used(h, c) || in_bounds(h, next_free_chunk(h, c)))); } /* Validate multiple state dimensions for the bucket "next" pointer @@ -60,11 +60,11 @@ static inline void check_nexts(struct z_heap *h, int bidx) } if (b->list_size == 2) { - CHECK(free_next(h, b->next) == free_prev(h, b->next)); - CHECK(free_next(h, b->next) != b->next); + CHECK(next_free_chunk(h, b->next) == prev_free_chunk(h, b->next)); + CHECK(next_free_chunk(h, b->next) != b->next); } else if (b->list_size == 1) { - CHECK(free_next(h, b->next) == free_prev(h, b->next)); - CHECK(free_next(h, b->next) == b->next); + CHECK(next_free_chunk(h, b->next) == prev_free_chunk(h, b->next)); + CHECK(next_free_chunk(h, b->next) == b->next); } } @@ -83,7 +83,7 @@ bool sys_heap_validate(struct sys_heap *heap) check_nexts(h, b); - for (c = c0; c != 0 && (n == 0 || c != c0); n++, c = free_next(h, c)) { + for (c = c0; c != 0 && (n == 0 || c != c0); n++, c = next_free_chunk(h, c)) { if (!valid_chunk(h, c)) { return false; } @@ -111,22 +111,22 @@ bool sys_heap_validate(struct sys_heap *heap) * blocks were found during enumeration). Mark all blocks * UNUSED */ - size_t prev_size = 0; + chunkid_t prev_chunk = 0; for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { if (!valid_chunk(h, c)) { return false; } - if (!used(h, c)) { + if (!chunk_used(h, c)) { return false; } if (c != h->chunk0) { - if (left_size(h, c) != prev_size) { + if (left_chunk(h, c) != prev_chunk) { return false; } } - prev_size = size(h, c); + prev_chunk = c; chunk_set_used(h, c, false); } @@ -146,8 +146,8 @@ bool sys_heap_validate(struct sys_heap *heap) continue; } - for (c = c0; n == 0 || c != c0; n++, c = free_next(h, c)) { - if (used(h, c)) { + for (c = c0; n == 0 || c != c0; n++, c = next_free_chunk(h, c)) { + if (chunk_used(h, c)) { return false; } chunk_set_used(h, c, true); @@ -158,7 +158,7 @@ bool sys_heap_validate(struct sys_heap *heap) * fields. One more linear pass to fix them up */ for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { - chunk_set_used(h, c, !used(h, c)); + chunk_set_used(h, c, !chunk_used(h, c)); } return true; } diff --git a/lib/os/heap.c b/lib/os/heap.c index 9b834c3cbea4b..d89dee2f35985 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -21,7 +21,7 @@ static void free_list_remove(struct z_heap *h, int bidx, { struct z_heap_bucket *b = &h->buckets[bidx]; - CHECK(!used(h, c)); + CHECK(!chunk_used(h, c)); CHECK(b->next != 0); CHECK(b->list_size > 0); CHECK((((h->avail_buckets & (1 << bidx)) == 0) @@ -33,7 +33,8 @@ static void free_list_remove(struct z_heap *h, int bidx, h->avail_buckets &= ~(1 << bidx); b->next = 0; } else { - chunkid_t first = free_prev(h, c), second = free_next(h, c); + chunkid_t first = prev_free_chunk(h, c), + second = next_free_chunk(h, c); b->next = second; chunk_set(h, first, FREE_NEXT, second); @@ -43,21 +44,21 @@ static void free_list_remove(struct z_heap *h, int bidx, static void free_list_add(struct z_heap *h, chunkid_t c) { - int b = bucket_idx(h, size(h, c)); + int bi = bucket_idx(h, chunk_size(h, c)); - if (h->buckets[b].list_size++ == 0) { - CHECK(h->buckets[b].next == 0); - CHECK((h->avail_buckets & (1 << b)) == 0); + if (h->buckets[bi].list_size++ == 0) { + CHECK(h->buckets[bi].next == 0); + CHECK((h->avail_buckets & (1 << bi)) == 0); /* Empty list, first item */ - h->avail_buckets |= (1 << b); - h->buckets[b].next = c; + h->avail_buckets |= (1 << bi); + h->buckets[bi].next = c; chunk_set(h, c, FREE_PREV, c); chunk_set(h, c, FREE_NEXT, c); } else { /* Insert before (!) the "next" pointer */ - chunkid_t second = h->buckets[b].next; - chunkid_t first = free_prev(h, second); + chunkid_t second = h->buckets[bi].next; + chunkid_t first = prev_free_chunk(h, second); chunk_set(h, c, FREE_PREV, first); chunk_set(h, c, FREE_NEXT, second); @@ -65,12 +66,12 @@ static void free_list_add(struct z_heap *h, chunkid_t c) chunk_set(h, second, FREE_PREV, c); } - CHECK(h->avail_buckets & (1 << bucket_idx(h, size(h, c)))); + CHECK(h->avail_buckets & (1 << bucket_idx(h, chunk_size(h, c)))); } static ALWAYS_INLINE bool last_chunk(struct z_heap *h, chunkid_t c) { - return (c + size(h, c)) == h->len; + return (c + chunk_size(h, c)) == h->len; } /* Allocates (fit check has already been perfomred) from the next @@ -79,14 +80,14 @@ static ALWAYS_INLINE bool last_chunk(struct z_heap *h, chunkid_t c) static void *split_alloc(struct z_heap *h, int bidx, size_t sz) { CHECK(h->buckets[bidx].next != 0 - && sz <= size(h, h->buckets[bidx].next)); + && sz <= chunk_size(h, h->buckets[bidx].next)); chunkid_t c = h->buckets[bidx].next; free_list_remove(h, bidx, c); /* Split off remainder if it's usefully large */ - size_t rem = size(h, c) - sz; + size_t rem = chunk_size(h, c) - sz; CHECK(rem < h->len); @@ -119,11 +120,11 @@ void sys_heap_free(struct sys_heap *heap, void *mem) - (u8_t *)h->buf) / CHUNK_UNIT; /* Merge with right chunk? We can just absorb it. */ - if (!last_chunk(h, c) && !used(h, right_chunk(h, c))) { + if (!last_chunk(h, c) && !chunk_used(h, right_chunk(h, c))) { chunkid_t rc = right_chunk(h, c); - size_t newsz = size(h, c) + size(h, rc); + size_t newsz = chunk_size(h, c) + chunk_size(h, rc); - free_list_remove(h, bucket_idx(h, size(h, rc)), rc); + free_list_remove(h, bucket_idx(h, chunk_size(h, rc)), rc); chunk_set(h, c, SIZE_AND_USED, newsz); if (!last_chunk(h, c)) { chunk_set(h, right_chunk(h, c), LEFT_SIZE, newsz); @@ -131,13 +132,13 @@ void sys_heap_free(struct sys_heap *heap, void *mem) } /* Merge with left chunk? It absorbs us. */ - if (c != h->chunk0 && !used(h, left_chunk(h, c))) { + if (c != h->chunk0 && !chunk_used(h, left_chunk(h, c))) { chunkid_t lc = left_chunk(h, c); chunkid_t rc = right_chunk(h, c); - size_t csz = size(h, c); - size_t merged_sz = csz + size(h, lc); + size_t csz = chunk_size(h, c); + size_t merged_sz = csz + chunk_size(h, lc); - free_list_remove(h, bucket_idx(h, size(h, lc)), lc); + free_list_remove(h, bucket_idx(h, chunk_size(h, lc)), lc); chunk_set(h, lc, SIZE_AND_USED, merged_sz); if (!last_chunk(h, lc)) { chunk_set(h, rc, LEFT_SIZE, merged_sz); @@ -179,10 +180,10 @@ void *sys_heap_alloc(struct sys_heap *heap, size_t bytes) for (int i = 0; i < loops; i++) { CHECK(b->next != 0); - if (size(h, b->next) >= sz) { + if (chunk_size(h, b->next) >= sz) { return split_alloc(h, bi, sz); } else { - b->next = free_next(h, b->next); + b->next = next_free_chunk(h, b->next); } } diff --git a/lib/os/heap.h b/lib/os/heap.h index c73b3cf254b50..8ddc24b234afd 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -98,12 +98,12 @@ static inline void chunk_set(struct z_heap *h, chunkid_t c, } } -static inline chunkid_t used(struct z_heap *h, chunkid_t c) +static inline bool chunk_used(struct z_heap *h, chunkid_t c) { return (chunk_field(h, c, SIZE_AND_USED) & ~h->size_mask) != 0; } -static ALWAYS_INLINE chunkid_t size(struct z_heap *h, chunkid_t c) +static ALWAYS_INLINE size_t chunk_size(struct z_heap *h, chunkid_t c) { return chunk_field(h, c, SIZE_AND_USED) & h->size_mask; } @@ -112,32 +112,27 @@ static inline void chunk_set_used(struct z_heap *h, chunkid_t c, bool used) { chunk_set(h, c, SIZE_AND_USED, - size(h, c) | (used ? (h->size_mask + 1) : 0)); + chunk_size(h, c) | (used ? (h->size_mask + 1) : 0)); } -static inline chunkid_t left_size(struct z_heap *h, chunkid_t c) -{ - return chunk_field(h, c, LEFT_SIZE); -} - -static inline chunkid_t free_prev(struct z_heap *h, chunkid_t c) +static inline chunkid_t prev_free_chunk(struct z_heap *h, chunkid_t c) { return chunk_field(h, c, FREE_PREV); } -static inline chunkid_t free_next(struct z_heap *h, chunkid_t c) +static inline chunkid_t next_free_chunk(struct z_heap *h, chunkid_t c) { return chunk_field(h, c, FREE_NEXT); } static inline chunkid_t left_chunk(struct z_heap *h, chunkid_t c) { - return c - left_size(h, c); + return c - chunk_field(h, c, LEFT_SIZE); } static inline chunkid_t right_chunk(struct z_heap *h, chunkid_t c) { - return c + size(h, c); + return c + chunk_size(h, c); } static inline size_t chunk_header_bytes(struct z_heap *h) From 60085cfd84bfeb054067338ed0dfd430c0f31fbe Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 24 Sep 2019 23:20:53 -0400 Subject: [PATCH 06/15] sys_heap: provide more chunk_fields accessors Let's provide accessors for getting and setting every field to make the chunk header layout abstracted away from the main code. Those are: SIZE_AND_USED: chunk_used(), chunk_size(), set_chunk_used() and chunk_size(). LEFT_SIZE: left_chunk() and set_left_chunk_size(). FREE_PREV: prev_free_chunk() and set_prev_free_chunk(). FREE_NEXT: next_free_chunk() and set_next_free_chunk(). To be consistent, the former chunk_set_used() is now set_chunk_used(). Signed-off-by: Nicolas Pitre --- lib/os/heap-validate.c | 8 ++++---- lib/os/heap.c | 38 +++++++++++++++++++------------------- lib/os/heap.h | 27 +++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c index 3248577cb1f27..b8e12d786faa3 100644 --- a/lib/os/heap-validate.c +++ b/lib/os/heap-validate.c @@ -87,7 +87,7 @@ bool sys_heap_validate(struct sys_heap *heap) if (!valid_chunk(h, c)) { return false; } - chunk_set_used(h, c, true); + set_chunk_used(h, c, true); } bool empty = (h->avail_buckets & (1 << b)) == 0; @@ -128,7 +128,7 @@ bool sys_heap_validate(struct sys_heap *heap) } prev_chunk = c; - chunk_set_used(h, c, false); + set_chunk_used(h, c, false); } if (c != h->len) { return false; /* Should have exactly consumed the buffer */ @@ -150,7 +150,7 @@ bool sys_heap_validate(struct sys_heap *heap) if (chunk_used(h, c)) { return false; } - chunk_set_used(h, c, true); + set_chunk_used(h, c, true); } } @@ -158,7 +158,7 @@ bool sys_heap_validate(struct sys_heap *heap) * fields. One more linear pass to fix them up */ for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { - chunk_set_used(h, c, !chunk_used(h, c)); + set_chunk_used(h, c, !chunk_used(h, c)); } return true; } diff --git a/lib/os/heap.c b/lib/os/heap.c index d89dee2f35985..d07afbd83d361 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -37,8 +37,8 @@ static void free_list_remove(struct z_heap *h, int bidx, second = next_free_chunk(h, c); b->next = second; - chunk_set(h, first, FREE_NEXT, second); - chunk_set(h, second, FREE_PREV, first); + set_next_free_chunk(h, first, second); + set_prev_free_chunk(h, second, first); } } @@ -53,17 +53,17 @@ static void free_list_add(struct z_heap *h, chunkid_t c) /* Empty list, first item */ h->avail_buckets |= (1 << bi); h->buckets[bi].next = c; - chunk_set(h, c, FREE_PREV, c); - chunk_set(h, c, FREE_NEXT, c); + set_prev_free_chunk(h, c, c); + set_next_free_chunk(h, c, c); } else { /* Insert before (!) the "next" pointer */ chunkid_t second = h->buckets[bi].next; chunkid_t first = prev_free_chunk(h, second); - chunk_set(h, c, FREE_PREV, first); - chunk_set(h, c, FREE_NEXT, second); - chunk_set(h, first, FREE_NEXT, c); - chunk_set(h, second, FREE_PREV, c); + set_prev_free_chunk(h, c, first); + set_next_free_chunk(h, c, second); + set_next_free_chunk(h, first, c); + set_prev_free_chunk(h, second, c); } CHECK(h->avail_buckets & (1 << bucket_idx(h, chunk_size(h, c)))); @@ -95,16 +95,16 @@ static void *split_alloc(struct z_heap *h, int bidx, size_t sz) chunkid_t c2 = c + sz; chunkid_t c3 = right_chunk(h, c); - chunk_set(h, c, SIZE_AND_USED, sz); - chunk_set(h, c2, SIZE_AND_USED, rem); - chunk_set(h, c2, LEFT_SIZE, sz); + set_chunk_size(h, c, sz); + set_chunk_size(h, c2, rem); + set_left_chunk_size(h, c2, sz); if (!last_chunk(h, c2)) { - chunk_set(h, c3, LEFT_SIZE, rem); + set_left_chunk_size(h, c3, rem); } free_list_add(h, c2); } - chunk_set_used(h, c, true); + set_chunk_used(h, c, true); return chunk_mem(h, c); } @@ -125,9 +125,9 @@ void sys_heap_free(struct sys_heap *heap, void *mem) size_t newsz = chunk_size(h, c) + chunk_size(h, rc); free_list_remove(h, bucket_idx(h, chunk_size(h, rc)), rc); - chunk_set(h, c, SIZE_AND_USED, newsz); + set_chunk_size(h, c, newsz); if (!last_chunk(h, c)) { - chunk_set(h, right_chunk(h, c), LEFT_SIZE, newsz); + set_left_chunk_size(h, right_chunk(h, c), newsz); } } @@ -139,15 +139,15 @@ void sys_heap_free(struct sys_heap *heap, void *mem) size_t merged_sz = csz + chunk_size(h, lc); free_list_remove(h, bucket_idx(h, chunk_size(h, lc)), lc); - chunk_set(h, lc, SIZE_AND_USED, merged_sz); + set_chunk_size(h, lc, merged_sz); if (!last_chunk(h, lc)) { - chunk_set(h, rc, LEFT_SIZE, merged_sz); + set_left_chunk_size(h, rc, merged_sz); } c = lc; } - chunk_set_used(h, c, false); + set_chunk_used(h, c, false); free_list_add(h, c); } @@ -235,6 +235,6 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) heap->heap->buckets[i].next = 0; } - chunk_set(h, h->chunk0, SIZE_AND_USED, buf_sz - h->chunk0); + set_chunk_size(h, h->chunk0, buf_sz - h->chunk0); free_list_add(h, h->chunk0); } diff --git a/lib/os/heap.h b/lib/os/heap.h index 8ddc24b234afd..466ccf4f67b9c 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -108,13 +108,18 @@ static ALWAYS_INLINE size_t chunk_size(struct z_heap *h, chunkid_t c) return chunk_field(h, c, SIZE_AND_USED) & h->size_mask; } -static inline void chunk_set_used(struct z_heap *h, chunkid_t c, - bool used) +static inline void set_chunk_used(struct z_heap *h, chunkid_t c, bool used) { chunk_set(h, c, SIZE_AND_USED, chunk_size(h, c) | (used ? (h->size_mask + 1) : 0)); } +static inline void set_chunk_size(struct z_heap *h, chunkid_t c, size_t size) +{ + chunk_set(h, c, SIZE_AND_USED, + size | (chunk_used(h, c) ? (h->size_mask + 1) : 0)); +} + static inline chunkid_t prev_free_chunk(struct z_heap *h, chunkid_t c) { return chunk_field(h, c, FREE_PREV); @@ -125,6 +130,18 @@ static inline chunkid_t next_free_chunk(struct z_heap *h, chunkid_t c) return chunk_field(h, c, FREE_NEXT); } +static inline void set_prev_free_chunk(struct z_heap *h, chunkid_t c, + chunkid_t prev) +{ + chunk_set(h, c, FREE_PREV, prev); +} + +static inline void set_next_free_chunk(struct z_heap *h, chunkid_t c, + chunkid_t next) +{ + chunk_set(h, c, FREE_NEXT, next); +} + static inline chunkid_t left_chunk(struct z_heap *h, chunkid_t c) { return c - chunk_field(h, c, LEFT_SIZE); @@ -135,6 +152,12 @@ static inline chunkid_t right_chunk(struct z_heap *h, chunkid_t c) return c + chunk_size(h, c); } +static inline void set_left_chunk_size(struct z_heap *h, chunkid_t c, + size_t size) +{ + chunk_set(h, c, LEFT_SIZE, size); +} + static inline size_t chunk_header_bytes(struct z_heap *h) { return big_heap(h) ? 8 : 4; From 50bd9fc369dbc9ae2792a6d22eb1436b33116ac5 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Wed, 25 Sep 2019 12:30:16 -0400 Subject: [PATCH 07/15] sys_heap: optimize usage of size and used flags By storing the used flag in the LSB, it is no longer necessary to have a size_mask variable to locate that flag. This produces smaller and faster code. Replace the validation check in chunk_set() to base it on the storage type. Also clarify the semantics of set_chunk_size() which allows for clearing the used flag bit unconditionally which simplifies the code further. The idea of moving the used flag bit into the LEFT_SIZE field was raised. It turns out that this isn't as beneficial as it may seem because the used bit is set only once i.e. when the memory is handed off to a user and the size field becomes frozen at that point. Modifications on the leftward chunk may still occur and extra instructions to preserve that bit would be necessary if it were moved there. Signed-off-by: Nicolas Pitre --- lib/os/heap.c | 1 - lib/os/heap.h | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/os/heap.c b/lib/os/heap.c index d07afbd83d361..e89b0dd0c7c1d 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -222,7 +222,6 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) h->buf = (u64_t *)addr; h->buckets = (void *)(addr + CHUNK_UNIT * hdr_chunks); h->len = buf_sz; - h->size_mask = (1 << (big_heap(h) ? 31 : 15)) - 1; h->avail_buckets = 0; size_t buckets_bytes = ((bucket_idx(h, buf_sz) + 1) diff --git a/lib/os/heap.h b/lib/os/heap.h index 466ccf4f67b9c..613a819720174 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -34,7 +34,7 @@ * * The fields are: * SIZE_AND_USED: the total size (including header) of the chunk in - * 8-byte units. The top bit stores a "used" flag. + * 8-byte units. The bottom bit stores a "used" flag. * LEFT_SIZE: The size of the left (next lower chunk in memory) * neighbor chunk. * FREE_PREV: Chunk ID of the previous node in a free list. @@ -55,7 +55,6 @@ struct z_heap { u64_t *buf; struct z_heap_bucket *buckets; u32_t len; - u32_t size_mask; u32_t chunk0; u32_t avail_buckets; }; @@ -86,38 +85,55 @@ static inline void chunk_set(struct z_heap *h, chunkid_t c, enum chunk_fields f, chunkid_t val) { CHECK(c >= h->chunk0 && c < h->len); - CHECK((val & ~((h->size_mask << 1) + 1)) == 0); - CHECK((val & h->size_mask) < h->len); void *cmem = &h->buf[c]; if (big_heap(h)) { - ((u32_t *)cmem)[f] = (u32_t) val; + CHECK(val == (u32_t)val); + ((u32_t *)cmem)[f] = val; } else { - ((u16_t *)cmem)[f] = (u16_t) val; + CHECK(val == (u16_t)val); + ((u16_t *)cmem)[f] = val; } } static inline bool chunk_used(struct z_heap *h, chunkid_t c) { - return (chunk_field(h, c, SIZE_AND_USED) & ~h->size_mask) != 0; + return chunk_field(h, c, SIZE_AND_USED) & 1; } -static ALWAYS_INLINE size_t chunk_size(struct z_heap *h, chunkid_t c) +static inline size_t chunk_size(struct z_heap *h, chunkid_t c) { - return chunk_field(h, c, SIZE_AND_USED) & h->size_mask; + return chunk_field(h, c, SIZE_AND_USED) >> 1; } static inline void set_chunk_used(struct z_heap *h, chunkid_t c, bool used) { - chunk_set(h, c, SIZE_AND_USED, - chunk_size(h, c) | (used ? (h->size_mask + 1) : 0)); + void *cmem = &h->buf[c]; + + if (big_heap(h)) { + if (used) { + ((u32_t *)cmem)[SIZE_AND_USED] |= 1; + } else { + ((u32_t *)cmem)[SIZE_AND_USED] &= ~1; + } + } else { + if (used) { + ((u16_t *)cmem)[SIZE_AND_USED] |= 1; + } else { + ((u16_t *)cmem)[SIZE_AND_USED] &= ~1; + } + } } +/* + * Note: no need to preserve the used bit here as the chunk is never in use + * when its size is modified, and potential set_chunk_used() is always + * invoked after set_chunk_size(). + */ static inline void set_chunk_size(struct z_heap *h, chunkid_t c, size_t size) { - chunk_set(h, c, SIZE_AND_USED, - size | (chunk_used(h, c) ? (h->size_mask + 1) : 0)); + chunk_set(h, c, SIZE_AND_USED, size << 1); } static inline chunkid_t prev_free_chunk(struct z_heap *h, chunkid_t c) From 040fc77ce930322f785b64b25287defd5879162c Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Wed, 25 Sep 2019 16:06:26 -0400 Subject: [PATCH 08/15] sys_heap: optimize struct z_heap It is possible to remove a few fields from struct z_heap, removing some runtime indirections by doing so: - The buf pointer is actually the same as the struct z_heap pointer itself. So let's simply create chunk_buf() that perform a type conversion. That type is also chunk_unit_t now rather than u64_t so it can be defined based on CHUNK_UNIT. - Replace the struct z_heap_bucket pointer by a zero-sized array at the end of struct z_heap. - Make chunk #0 into an actual chunk with its own header. This allows for removing the chunk0 field and streamlining the code. This way h->chunk0 becomes right_chunk(h, 0). This sets the table for further simplifications to come. Signed-off-by: Nicolas Pitre --- lib/os/heap-validate.c | 15 ++++++-------- lib/os/heap.c | 46 ++++++++++++++++++++++-------------------- lib/os/heap.h | 34 ++++++++++++++++++++----------- 3 files changed, 52 insertions(+), 43 deletions(-) diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c index b8e12d786faa3..cd235f1b72e46 100644 --- a/lib/os/heap-validate.c +++ b/lib/os/heap-validate.c @@ -24,7 +24,7 @@ static size_t max_chunkid(struct z_heap *h) static bool in_bounds(struct z_heap *h, chunkid_t c) { - return (c >= h->chunk0) + return (c >= right_chunk(h, 0)) && (c <= max_chunkid(h)) && (chunk_size(h, c) < h->len); } @@ -34,7 +34,7 @@ static bool valid_chunk(struct z_heap *h, chunkid_t c) return (chunk_size(h, c) > 0 && (c + chunk_size(h, c) <= h->len) && in_bounds(h, c) - && ((c == h->chunk0) || in_bounds(h, left_chunk(h, c))) + && (!left_chunk(h, c) || in_bounds(h, left_chunk(h, c))) && (chunk_used(h, c) || in_bounds(h, prev_free_chunk(h, c))) && (chunk_used(h, c) || in_bounds(h, next_free_chunk(h, c)))); } @@ -113,18 +113,15 @@ bool sys_heap_validate(struct sys_heap *heap) */ chunkid_t prev_chunk = 0; - for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { + for (c = right_chunk(h, 0); c <= max_chunkid(h); c = right_chunk(h, c)) { if (!valid_chunk(h, c)) { return false; } if (!chunk_used(h, c)) { return false; } - - if (c != h->chunk0) { - if (left_chunk(h, c) != prev_chunk) { - return false; - } + if (left_chunk(h, c) != prev_chunk) { + return false; } prev_chunk = c; @@ -157,7 +154,7 @@ bool sys_heap_validate(struct sys_heap *heap) /* Now we are valid, but have managed to invert all the in-use * fields. One more linear pass to fix them up */ - for (c = h->chunk0; c <= max_chunkid(h); c = right_chunk(h, c)) { + for (c = right_chunk(h, 0); c <= max_chunkid(h); c = right_chunk(h, c)) { set_chunk_used(h, c, !chunk_used(h, c)); } return true; diff --git a/lib/os/heap.c b/lib/os/heap.c index e89b0dd0c7c1d..62ccdb3a5fb57 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -9,7 +9,8 @@ static void *chunk_mem(struct z_heap *h, chunkid_t c) { - u8_t *ret = ((u8_t *)&h->buf[c]) + chunk_header_bytes(h); + chunk_unit_t *buf = chunk_buf(h); + u8_t *ret = ((u8_t *)&buf[c]) + chunk_header_bytes(h); CHECK(!(((size_t)ret) & (big_heap(h) ? 7 : 3))); @@ -117,7 +118,7 @@ void sys_heap_free(struct sys_heap *heap, void *mem) struct z_heap *h = heap->heap; chunkid_t c = ((u8_t *)mem - chunk_header_bytes(h) - - (u8_t *)h->buf) / CHUNK_UNIT; + - (u8_t *)chunk_buf(h)) / CHUNK_UNIT; /* Merge with right chunk? We can just absorb it. */ if (!last_chunk(h, c) && !chunk_used(h, right_chunk(h, c))) { @@ -132,7 +133,7 @@ void sys_heap_free(struct sys_heap *heap, void *mem) } /* Merge with left chunk? It absorbs us. */ - if (c != h->chunk0 && !chunk_used(h, left_chunk(h, c))) { + if (!chunk_used(h, left_chunk(h, c))) { chunkid_t lc = left_chunk(h, c); chunkid_t rc = right_chunk(h, c); size_t csz = chunk_size(h, c); @@ -203,37 +204,38 @@ void *sys_heap_alloc(struct sys_heap *heap, size_t bytes) void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) { - /* Must fit in a 32 bit count of u64's */ -#if __SIZEOF_SIZE_T__ > 4 - CHECK(bytes < 0x800000000ULL); -#endif + /* Must fit in a 32 bit count of HUNK_UNIT */ + CHECK(bytes / CHUNK_UNIT <= 0xffffffffU); /* Round the start up, the end down */ - size_t addr = ((size_t)mem + CHUNK_UNIT - 1) & ~(CHUNK_UNIT - 1); - size_t end = ((size_t)mem + bytes) & ~(CHUNK_UNIT - 1); + uintptr_t addr = ROUND_UP(mem, CHUNK_UNIT); + uintptr_t end = ROUND_DOWN((u8_t *)mem + bytes, CHUNK_UNIT); size_t buf_sz = (end - addr) / CHUNK_UNIT; - size_t hdr_chunks = chunksz(sizeof(struct z_heap)); CHECK(end > addr); + CHECK(buf_sz > chunksz(sizeof(struct z_heap))); struct z_heap *h = (struct z_heap *)addr; - - heap->heap = (struct z_heap *)addr; - h->buf = (u64_t *)addr; - h->buckets = (void *)(addr + CHUNK_UNIT * hdr_chunks); + heap->heap = h; + h->chunk0_hdr_area = 0; h->len = buf_sz; h->avail_buckets = 0; - size_t buckets_bytes = ((bucket_idx(h, buf_sz) + 1) - * sizeof(struct z_heap_bucket)); + int nb_buckets = bucket_idx(h, buf_sz) + 1; + size_t chunk0_size = chunksz(sizeof(struct z_heap) + + nb_buckets * sizeof(struct z_heap_bucket)); - h->chunk0 = hdr_chunks + chunksz(buckets_bytes); + CHECK(chunk0_size < buf_sz); - for (int i = 0; i <= bucket_idx(heap->heap, heap->heap->len); i++) { - heap->heap->buckets[i].list_size = 0; - heap->heap->buckets[i].next = 0; + for (int i = 0; i < nb_buckets; i++) { + h->buckets[i].list_size = 0; + h->buckets[i].next = 0; } - set_chunk_size(h, h->chunk0, buf_sz - h->chunk0); - free_list_add(h, h->chunk0); + set_chunk_size(h, 0, chunk0_size); + set_chunk_used(h, 0, true); + + set_chunk_size(h, chunk0_size, buf_sz - chunk0_size); + set_left_chunk_size(h, chunk0_size, chunk0_size); + free_list_add(h, chunk0_size); } diff --git a/lib/os/heap.h b/lib/os/heap.h index 613a819720174..facdedda48090 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -49,30 +49,38 @@ typedef size_t chunkid_t; #define CHUNK_UNIT 8 -enum chunk_fields { SIZE_AND_USED, LEFT_SIZE, FREE_PREV, FREE_NEXT }; +typedef struct { char bytes[CHUNK_UNIT]; } chunk_unit_t; -struct z_heap { - u64_t *buf; - struct z_heap_bucket *buckets; - u32_t len; - u32_t chunk0; - u32_t avail_buckets; -}; +enum chunk_fields { SIZE_AND_USED, LEFT_SIZE, FREE_PREV, FREE_NEXT }; struct z_heap_bucket { chunkid_t next; size_t list_size; }; +struct z_heap { + u64_t chunk0_hdr_area; /* matches the largest header */ + u32_t len; + u32_t avail_buckets; + struct z_heap_bucket buckets[0]; +}; + static inline bool big_heap(struct z_heap *h) { return sizeof(size_t) > 4 || h->len > 0x7fff; } +static inline chunk_unit_t *chunk_buf(struct z_heap *h) +{ + /* the struct z_heap matches with the first chunk */ + return (chunk_unit_t *)h; +} + static inline size_t chunk_field(struct z_heap *h, chunkid_t c, enum chunk_fields f) { - void *cmem = &h->buf[c]; + chunk_unit_t *buf = chunk_buf(h); + void *cmem = &buf[c]; if (big_heap(h)) { return ((u32_t *)cmem)[f]; @@ -84,9 +92,10 @@ static inline size_t chunk_field(struct z_heap *h, chunkid_t c, static inline void chunk_set(struct z_heap *h, chunkid_t c, enum chunk_fields f, chunkid_t val) { - CHECK(c >= h->chunk0 && c < h->len); + CHECK(c < h->len); - void *cmem = &h->buf[c]; + chunk_unit_t *buf = chunk_buf(h); + void *cmem = &buf[c]; if (big_heap(h)) { CHECK(val == (u32_t)val); @@ -109,7 +118,8 @@ static inline size_t chunk_size(struct z_heap *h, chunkid_t c) static inline void set_chunk_used(struct z_heap *h, chunkid_t c, bool used) { - void *cmem = &h->buf[c]; + chunk_unit_t *buf = chunk_buf(h); + void *cmem = &buf[c]; if (big_heap(h)) { if (used) { From ff6d8e76a85f461edb5f7e6fcd8ed9cfd8c72bdb Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Wed, 25 Sep 2019 17:15:18 -0400 Subject: [PATCH 09/15] sys_heap: introduce min_chunk_size() With this we can remove magic constants, especially those used with big_heap(). Signed-off-by: Nicolas Pitre --- lib/os/heap-validate.c | 2 +- lib/os/heap.c | 4 ++-- lib/os/heap.h | 12 +++++++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c index cd235f1b72e46..afafc1d5686c4 100644 --- a/lib/os/heap-validate.c +++ b/lib/os/heap-validate.c @@ -19,7 +19,7 @@ static size_t max_chunkid(struct z_heap *h) { - return h->len - bytes_to_chunksz(h, 1); + return h->len - min_chunk_size(h); } static bool in_bounds(struct z_heap *h, chunkid_t c) diff --git a/lib/os/heap.c b/lib/os/heap.c index 62ccdb3a5fb57..51db94d27de19 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -92,7 +92,7 @@ static void *split_alloc(struct z_heap *h, int bidx, size_t sz) CHECK(rem < h->len); - if (rem >= (big_heap(h) ? 2 : 1)) { + if (rem >= min_chunk_size(h)) { chunkid_t c2 = c + sz; chunkid_t c3 = right_chunk(h, c); @@ -225,7 +225,7 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) size_t chunk0_size = chunksz(sizeof(struct z_heap) + nb_buckets * sizeof(struct z_heap_bucket)); - CHECK(chunk0_size < buf_sz); + CHECK(chunk0_size + min_chunk_size(h) < buf_sz); for (int i = 0; i < nb_buckets; i++) { h->buckets[i].list_size = 0; diff --git a/lib/os/heap.h b/lib/os/heap.h index facdedda48090..54f98e2cc9b68 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -199,10 +199,16 @@ static inline size_t bytes_to_chunksz(struct z_heap *h, size_t bytes) return chunksz(chunk_header_bytes(h) + bytes); } -static int bucket_idx(struct z_heap *h, size_t sz) +static inline int min_chunk_size(struct z_heap *h) { - /* A chunk of size 2 is the minimum size on big heaps */ - return 31 - __builtin_clz(sz) - (big_heap(h) ? 1 : 0); + return bytes_to_chunksz(h, 1); } +static inline int bucket_idx(struct z_heap *h, size_t sz) +{ + size_t usable_sz = sz - min_chunk_size(h) + 1; + return 31 - __builtin_clz(usable_sz); +} + + #endif /* ZEPHYR_INCLUDE_LIB_OS_HEAP_H_ */ From e23733d1f434d65ed26a5c1251299fb8cfe2df9a Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 26 Sep 2019 01:59:35 -0400 Subject: [PATCH 10/15] sys_heap: remove need for last_chunk() We already have chunk #0 containing our struct z_heap and marked as used. We can add a partial chunk at the very end that is also marked as used. By doing so there is no longer a need for checking heap boundaries at run time when merging/splitting chunks meaning fewer conditionals in the code's hot path. Signed-off-by: Nicolas Pitre --- lib/os/heap-validate.c | 3 ++- lib/os/heap.c | 31 ++++++++++++++++--------------- lib/os/heap.h | 19 +++++++++++++++++-- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c index afafc1d5686c4..13716bba6a266 100644 --- a/lib/os/heap-validate.c +++ b/lib/os/heap-validate.c @@ -34,7 +34,8 @@ static bool valid_chunk(struct z_heap *h, chunkid_t c) return (chunk_size(h, c) > 0 && (c + chunk_size(h, c) <= h->len) && in_bounds(h, c) - && (!left_chunk(h, c) || in_bounds(h, left_chunk(h, c))) + && (right_chunk(h, left_chunk(h, c)) == c) + && (left_chunk(h, right_chunk(h, c)) == c) && (chunk_used(h, c) || in_bounds(h, prev_free_chunk(h, c))) && (chunk_used(h, c) || in_bounds(h, next_free_chunk(h, c)))); } diff --git a/lib/os/heap.c b/lib/os/heap.c index 51db94d27de19..bdbf2bfcaed3a 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -70,11 +70,6 @@ static void free_list_add(struct z_heap *h, chunkid_t c) CHECK(h->avail_buckets & (1 << bucket_idx(h, chunk_size(h, c)))); } -static ALWAYS_INLINE bool last_chunk(struct z_heap *h, chunkid_t c) -{ - return (c + chunk_size(h, c)) == h->len; -} - /* Allocates (fit check has already been perfomred) from the next * chunk at the specified bucket level */ @@ -99,9 +94,7 @@ static void *split_alloc(struct z_heap *h, int bidx, size_t sz) set_chunk_size(h, c, sz); set_chunk_size(h, c2, rem); set_left_chunk_size(h, c2, sz); - if (!last_chunk(h, c2)) { - set_left_chunk_size(h, c3, rem); - } + set_left_chunk_size(h, c3, rem); free_list_add(h, c2); } @@ -121,15 +114,13 @@ void sys_heap_free(struct sys_heap *heap, void *mem) - (u8_t *)chunk_buf(h)) / CHUNK_UNIT; /* Merge with right chunk? We can just absorb it. */ - if (!last_chunk(h, c) && !chunk_used(h, right_chunk(h, c))) { + if (!chunk_used(h, right_chunk(h, c))) { chunkid_t rc = right_chunk(h, c); size_t newsz = chunk_size(h, c) + chunk_size(h, rc); free_list_remove(h, bucket_idx(h, chunk_size(h, rc)), rc); set_chunk_size(h, c, newsz); - if (!last_chunk(h, c)) { - set_left_chunk_size(h, right_chunk(h, c), newsz); - } + set_left_chunk_size(h, right_chunk(h, c), newsz); } /* Merge with left chunk? It absorbs us. */ @@ -141,9 +132,7 @@ void sys_heap_free(struct sys_heap *heap, void *mem) free_list_remove(h, bucket_idx(h, chunk_size(h, lc)), lc); set_chunk_size(h, lc, merged_sz); - if (!last_chunk(h, lc)) { - set_left_chunk_size(h, rc, merged_sz); - } + set_left_chunk_size(h, rc, merged_sz); c = lc; } @@ -207,6 +196,10 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) /* Must fit in a 32 bit count of HUNK_UNIT */ CHECK(bytes / CHUNK_UNIT <= 0xffffffffU); + /* Reserve the final marker chunk's header */ + CHECK(bytes > heap_footer_bytes(bytes)); + bytes -= heap_footer_bytes(bytes); + /* Round the start up, the end down */ uintptr_t addr = ROUND_UP(mem, CHUNK_UNIT); uintptr_t end = ROUND_DOWN((u8_t *)mem + bytes, CHUNK_UNIT); @@ -232,10 +225,18 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) h->buckets[i].next = 0; } + /* chunk containing our struct z_heap */ set_chunk_size(h, 0, chunk0_size); set_chunk_used(h, 0, true); + /* chunk containing the free heap */ set_chunk_size(h, chunk0_size, buf_sz - chunk0_size); set_left_chunk_size(h, chunk0_size, chunk0_size); + + /* the end marker chunk */ + set_chunk_size(h, buf_sz, 0); + set_left_chunk_size(h, buf_sz, buf_sz - chunk0_size); + set_chunk_used(h, buf_sz, true); + free_list_add(h, chunk0_size); } diff --git a/lib/os/heap.h b/lib/os/heap.h index 54f98e2cc9b68..789b36c3838ff 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -65,9 +65,19 @@ struct z_heap { struct z_heap_bucket buckets[0]; }; +static inline bool big_heap_chunks(size_t chunks) +{ + return sizeof(void *) > 4 || chunks > 0x7fff; +} + +static inline bool big_heap_bytes(size_t bytes) +{ + return big_heap_chunks(bytes / CHUNK_UNIT); +} + static inline bool big_heap(struct z_heap *h) { - return sizeof(size_t) > 4 || h->len > 0x7fff; + return big_heap_chunks(h->len); } static inline chunk_unit_t *chunk_buf(struct z_heap *h) @@ -92,7 +102,7 @@ static inline size_t chunk_field(struct z_heap *h, chunkid_t c, static inline void chunk_set(struct z_heap *h, chunkid_t c, enum chunk_fields f, chunkid_t val) { - CHECK(c < h->len); + CHECK(c <= h->len); chunk_unit_t *buf = chunk_buf(h); void *cmem = &buf[c]; @@ -189,6 +199,11 @@ static inline size_t chunk_header_bytes(struct z_heap *h) return big_heap(h) ? 8 : 4; } +static inline size_t heap_footer_bytes(size_t size) +{ + return big_heap_bytes(size) ? 8 : 4; +} + static inline size_t chunksz(size_t bytes) { return (bytes + CHUNK_UNIT - 1) / CHUNK_UNIT; From 0daece098f9a1fdf9c1499a5ffc78579cfa44938 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 26 Sep 2019 15:22:24 -0400 Subject: [PATCH 11/15] sys_heap: simplify some complex checks Avoid redundancy and bucket_idx() usage when possible. Signed-off-by: Nicolas Pitre --- lib/os/heap.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/os/heap.c b/lib/os/heap.c index bdbf2bfcaed3a..61cb63594256f 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -25,8 +25,7 @@ static void free_list_remove(struct z_heap *h, int bidx, CHECK(!chunk_used(h, c)); CHECK(b->next != 0); CHECK(b->list_size > 0); - CHECK((((h->avail_buckets & (1 << bidx)) == 0) - == (h->buckets[bidx].next == 0))); + CHECK(h->avail_buckets & (1 << bidx)); b->list_size--; @@ -67,7 +66,7 @@ static void free_list_add(struct z_heap *h, chunkid_t c) set_prev_free_chunk(h, second, c); } - CHECK(h->avail_buckets & (1 << bucket_idx(h, chunk_size(h, c)))); + CHECK(h->avail_buckets & (1 << bi)); } /* Allocates (fit check has already been perfomred) from the next From c83c2c1ee93ae1e8476854d9ee916e0040d2e57d Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Thu, 26 Sep 2019 15:55:50 -0400 Subject: [PATCH 12/15] sys_heap: perform cheap overflow detection on freed memory Make the LEFT_SIZE field first and SIZE_AND_USED field last (for an allocated chunk) so they sit right next to the allocated memory. The current chunk's SIZE_AND_USED field points to the next (right) chunk, and from there the LEFT_SIZE field should point back to the current chunk. Many trivial memory overflows should trip that test. One way to make this test more robust could involve xor'ing the values within respective accessor pairs. But at least the fact that the size value is shifted by one bit already prevent fooling the test with a same-byte corruption. Signed-off-by: Nicolas Pitre --- lib/os/heap.c | 14 ++++++++++++++ lib/os/heap.h | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/os/heap.c b/lib/os/heap.c index 61cb63594256f..e4e5ed23eaf1d 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -112,6 +112,20 @@ void sys_heap_free(struct sys_heap *heap, void *mem) chunkid_t c = ((u8_t *)mem - chunk_header_bytes(h) - (u8_t *)chunk_buf(h)) / CHUNK_UNIT; + /* + * This should catch many double-free cases. + * This is cheap enough so let's do it all the time. + */ + __ASSERT(chunk_used(h, c), + "unexpected heap state (double-free?) for memory at %p", mem); + /* + * It is easy to catch many common memory overflow cases with + * a quick check on this and next chunk header fields that are + * immediately before and after the freed memory. + */ + __ASSERT(left_chunk(h, right_chunk(h, c)) == c, + "corrupted heap bounds (buffer overflow?) for memory at %p", mem); + /* Merge with right chunk? We can just absorb it. */ if (!chunk_used(h, right_chunk(h, c))) { chunkid_t rc = right_chunk(h, c); diff --git a/lib/os/heap.h b/lib/os/heap.h index 789b36c3838ff..306f5399dba7d 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -51,7 +51,7 @@ typedef size_t chunkid_t; typedef struct { char bytes[CHUNK_UNIT]; } chunk_unit_t; -enum chunk_fields { SIZE_AND_USED, LEFT_SIZE, FREE_PREV, FREE_NEXT }; +enum chunk_fields { LEFT_SIZE, SIZE_AND_USED, FREE_PREV, FREE_NEXT }; struct z_heap_bucket { chunkid_t next; From 78fa85fdf8f03e0cfc0d7192e5bc3dc06c528864 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Wed, 23 Oct 2019 19:43:16 -0400 Subject: [PATCH 13/15] sys_heap: reduce the size of struct z_heap_bucket by half This struct is taking up most of the heap's constant footprint overhead. We can easily get rid of the list_size member as it is mostly used to determine if the list is empty, and that can be determined through other means. Signed-off-by: Nicolas Pitre --- lib/os/heap-validate.c | 15 +-------------- lib/os/heap.c | 31 ++++++++++++++----------------- lib/os/heap.h | 1 - 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/lib/os/heap-validate.c b/lib/os/heap-validate.c index 13716bba6a266..0869cac7835a4 100644 --- a/lib/os/heap-validate.c +++ b/lib/os/heap-validate.c @@ -50,8 +50,7 @@ static inline void check_nexts(struct z_heap *h, int bidx) bool emptybit = (h->avail_buckets & (1 << bidx)) == 0; bool emptylist = b->next == 0; - bool emptycount = b->list_size == 0; - bool empties_match = emptybit == emptylist && emptybit == emptycount; + bool empties_match = emptybit == emptylist; (void)empties_match; CHECK(empties_match); @@ -59,14 +58,6 @@ static inline void check_nexts(struct z_heap *h, int bidx) if (b->next != 0) { CHECK(valid_chunk(h, b->next)); } - - if (b->list_size == 2) { - CHECK(next_free_chunk(h, b->next) == prev_free_chunk(h, b->next)); - CHECK(next_free_chunk(h, b->next) != b->next); - } else if (b->list_size == 1) { - CHECK(next_free_chunk(h, b->next) == prev_free_chunk(h, b->next)); - CHECK(next_free_chunk(h, b->next) == b->next); - } } bool sys_heap_validate(struct sys_heap *heap) @@ -101,10 +92,6 @@ bool sys_heap_validate(struct sys_heap *heap) if (empty && h->buckets[b].next != 0) { return false; } - - if (n != h->buckets[b].list_size) { - return false; - } } /* Walk through the chunks linearly, verifying sizes and end diff --git a/lib/os/heap.c b/lib/os/heap.c index e4e5ed23eaf1d..daf05e738d53a 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -24,12 +24,10 @@ static void free_list_remove(struct z_heap *h, int bidx, CHECK(!chunk_used(h, c)); CHECK(b->next != 0); - CHECK(b->list_size > 0); CHECK(h->avail_buckets & (1 << bidx)); - b->list_size--; - - if (b->list_size == 0) { + if (next_free_chunk(h, c) == c) { + /* this is the last chunk */ h->avail_buckets &= ~(1 << bidx); b->next = 0; } else { @@ -46,8 +44,7 @@ static void free_list_add(struct z_heap *h, chunkid_t c) { int bi = bucket_idx(h, chunk_size(h, c)); - if (h->buckets[bi].list_size++ == 0) { - CHECK(h->buckets[bi].next == 0); + if (h->buckets[bi].next == 0) { CHECK((h->avail_buckets & (1 << bi)) == 0); /* Empty list, first item */ @@ -56,6 +53,8 @@ static void free_list_add(struct z_heap *h, chunkid_t c) set_prev_free_chunk(h, c, c); set_next_free_chunk(h, c, c); } else { + CHECK(h->avail_buckets & (1 << bi)); + /* Insert before (!) the "next" pointer */ chunkid_t second = h->buckets[bi].next; chunkid_t first = prev_free_chunk(h, second); @@ -65,8 +64,6 @@ static void free_list_add(struct z_heap *h, chunkid_t c) set_next_free_chunk(h, first, c); set_prev_free_chunk(h, second, c); } - - CHECK(h->avail_buckets & (1 << bi)); } /* Allocates (fit check has already been perfomred) from the next @@ -179,15 +176,16 @@ void *sys_heap_alloc(struct sys_heap *heap, size_t bytes) * fragmentation waste of the order of the block allocated * only. */ - int loops = MIN(b->list_size, CONFIG_SYS_HEAP_ALLOC_LOOPS); - - for (int i = 0; i < loops; i++) { - CHECK(b->next != 0); - if (chunk_size(h, b->next) >= sz) { - return split_alloc(h, bi, sz); - } else { + if (b->next) { + chunkid_t first = b->next; + int i = CONFIG_SYS_HEAP_ALLOC_LOOPS; + do { + if (chunk_size(h, b->next) >= sz) { + return split_alloc(h, bi, sz); + } b->next = next_free_chunk(h, b->next); - } + CHECK(b->next != 0); + } while (--i && b->next != first); } /* Otherwise pick the smallest non-empty bucket guaranteed to @@ -234,7 +232,6 @@ void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) CHECK(chunk0_size + min_chunk_size(h) < buf_sz); for (int i = 0; i < nb_buckets; i++) { - h->buckets[i].list_size = 0; h->buckets[i].next = 0; } diff --git a/lib/os/heap.h b/lib/os/heap.h index 306f5399dba7d..ba3cd68608831 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -55,7 +55,6 @@ enum chunk_fields { LEFT_SIZE, SIZE_AND_USED, FREE_PREV, FREE_NEXT }; struct z_heap_bucket { chunkid_t next; - size_t list_size; }; struct z_heap { From fab3b073373b6168b9b5fca9e65ac2ad2e9fd875 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Sun, 29 Sep 2019 17:43:07 -0400 Subject: [PATCH 14/15] sys_heap: move chunk-to-memory conversion helpers to heap.h This to allow sharing with the compat code. Signed-off-by: Nicolas Pitre --- lib/os/heap.c | 17 +++-------------- lib/os/heap.h | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/os/heap.c b/lib/os/heap.c index daf05e738d53a..331bb7463c05e 100644 --- a/lib/os/heap.c +++ b/lib/os/heap.c @@ -7,16 +7,6 @@ #include #include "heap.h" -static void *chunk_mem(struct z_heap *h, chunkid_t c) -{ - chunk_unit_t *buf = chunk_buf(h); - u8_t *ret = ((u8_t *)&buf[c]) + chunk_header_bytes(h); - - CHECK(!(((size_t)ret) & (big_heap(h) ? 7 : 3))); - - return ret; -} - static void free_list_remove(struct z_heap *h, int bidx, chunkid_t c) { @@ -96,7 +86,7 @@ static void *split_alloc(struct z_heap *h, int bidx, size_t sz) set_chunk_used(h, c, true); - return chunk_mem(h, c); + return chunk_to_mem(h, c); } void sys_heap_free(struct sys_heap *heap, void *mem) @@ -106,8 +96,7 @@ void sys_heap_free(struct sys_heap *heap, void *mem) } struct z_heap *h = heap->heap; - chunkid_t c = ((u8_t *)mem - chunk_header_bytes(h) - - (u8_t *)chunk_buf(h)) / CHUNK_UNIT; + chunkid_t c = mem_to_chunk(h, mem); /* * This should catch many double-free cases. @@ -204,7 +193,7 @@ void *sys_heap_alloc(struct sys_heap *heap, size_t bytes) void sys_heap_init(struct sys_heap *heap, void *mem, size_t bytes) { - /* Must fit in a 32 bit count of HUNK_UNIT */ + /* Must fit in a 32 bit count of CHUNK_UNIT */ CHECK(bytes / CHUNK_UNIT <= 0xffffffffU); /* Reserve the final marker chunk's header */ diff --git a/lib/os/heap.h b/lib/os/heap.h index ba3cd68608831..c5a194dbe5556 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -203,6 +203,21 @@ static inline size_t heap_footer_bytes(size_t size) return big_heap_bytes(size) ? 8 : 4; } +static inline void *chunk_to_mem(struct z_heap *h, chunkid_t c) +{ + chunk_unit_t *buf = chunk_buf(h) + c; + void *ret = (u8_t *)buf + chunk_header_bytes(h); + CHECK(!((uintptr_t)ret & (big_heap(h) ? 7 : 3))); + return ret; +} + +static inline chunkid_t mem_to_chunk(struct z_heap *h, void *ptr) +{ + ptr = (u8_t *)ptr - chunk_header_bytes(h); + CHECK(!((uintptr_t)ptr & (CHUNK_UNIT - 1))); + return (chunk_unit_t *)ptr - chunk_buf(h); +} + static inline size_t chunksz(size_t bytes) { return (bytes + CHUNK_UNIT - 1) / CHUNK_UNIT; From e07650f9314165ba35c1fadb2f96ef59ec304d56 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Sun, 29 Sep 2019 19:01:36 -0400 Subject: [PATCH 15/15] sys_heap: mempool compatibility wrappers This is a drop-in replacement for the mempool interface. Obviously this is not meant to be merged as is, if ever. This is nice for performance and behavior comparison tests using actual application code. However there would be benefits to migrating users to the native sys_heap API eventually. Some caveats: - Block sizing and alignment is completely ignored. Only the number of max block size is guaranteed. - Yet the static allocation appears to be biggish. - Every allocation request incurs a space overhead. The original mempool with power-of-2 sizing does not have this overhead besides its out-of-line free block bitmap. Things move in favor of the sys_heap allocator only when using sizes that don't divide by 4. - Tests that expect some allocation failure patterns are likely to fail (i.e. successfully allocate) with this. The reverse is also true. Hence a couple tests are disabled. - The compat kconfig option defaults to y for ease of testing. Signed-off-by: Nicolas Pitre --- include/kernel.h | 4 + include/kernel_includes.h | 2 + include/sys/heap-compat-common.h | 134 ++++++++++++++++++ include/sys/heap-k_mempool-compat.h | 66 +++++++++ include/sys/heap-sys_mempool-compat.h | 74 ++++++++++ include/sys/mempool.h | 6 + lib/os/CMakeLists.txt | 4 +- lib/os/Kconfig | 9 ++ lib/os/heap-mempool-compat.c | 100 +++++++++++++ lib/os/heap.h | 15 +- .../mheap_api_concept/src/test_mheap_api.c | 4 + .../mem_heap/mheap_api_concept/testcase.yaml | 1 + tests/kernel/mem_pool/mem_pool/testcase.yaml | 1 + .../mem_pool/mem_pool_api/testcase.yaml | 1 + .../mem_pool/mem_pool_concept/testcase.yaml | 1 + tests/kernel/mem_pool/sys_mem_pool/src/main.c | 2 + tests/kernel/queue/src/test_queue_contexts.c | 2 + 17 files changed, 421 insertions(+), 5 deletions(-) create mode 100644 include/sys/heap-compat-common.h create mode 100644 include/sys/heap-k_mempool-compat.h create mode 100644 include/sys/heap-sys_mempool-compat.h create mode 100644 lib/os/heap-mempool-compat.c diff --git a/include/kernel.h b/include/kernel.h index f8b841bf27efd..860ad1500cee3 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -4269,6 +4269,9 @@ static inline u32_t k_mem_slab_num_free_get(struct k_mem_slab *slab) /** @} */ +#ifdef CONFIG_SYS_HEAP_MEMPOOL_COMPAT +#include +#else /** * @cond INTERNAL_HIDDEN */ @@ -4322,6 +4325,7 @@ struct k_mem_pool { } \ }; \ BUILD_ASSERT(WB_UP(maxsz) >= _MPOOL_MINBLK) +#endif /** * @brief Allocate memory from a memory pool. diff --git a/include/kernel_includes.h b/include/kernel_includes.h index f52a372807644..626b8f7bfb8bb 100644 --- a/include/kernel_includes.h +++ b/include/kernel_includes.h @@ -25,7 +25,9 @@ #include #include #include +#ifndef CONFIG_SYS_HEAP_MEMPOOL_COMPAT #include +#endif #include #include #include diff --git a/include/sys/heap-compat-common.h b/include/sys/heap-compat-common.h new file mode 100644 index 0000000000000..76cc59d26db11 --- /dev/null +++ b/include/sys/heap-compat-common.h @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2019 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef ZEPHYR_INCLUDE_SYS_HEAP_COMPAT_H_ +#define ZEPHYR_INCLUDE_SYS_HEAP_COMPAT_H_ + +#include +#include + +/* + * Internal heap structures copied from lib/os/heap.h + * + * This lives here only for the sake of mempool compatibility wrappers. + * This ought to move back to a more private location eventually. + */ + +#define CHUNK_UNIT 8 + +typedef size_t chunkid_t; + +struct z_heap_bucket { + chunkid_t next; + size_t list_size; +}; + +struct z_heap { + u64_t chunk0_hdr_area; /* matches the largest header */ + u32_t len; + u32_t avail_buckets; + struct z_heap_bucket buckets[0]; +}; + +/* + * This normally depends on the heap size, but our heap size depends on this. + * Let's assume big_heap() would be true. + */ +#define CHUNK_HEADER_BYTES 8 + +/* + * Returns integer value for log2 of x, or a large negative value otherwise. + * This is made so it can be used for constant compile-time evaluation. + */ +#define z_constant_ilog2(x) \ + ( __builtin_constant_p(x) ? ( \ + ((x) & (1 << 31)) ? 31 : \ + ((x) & (1 << 30)) ? 30 : \ + ((x) & (1 << 29)) ? 29 : \ + ((x) & (1 << 28)) ? 28 : \ + ((x) & (1 << 27)) ? 27 : \ + ((x) & (1 << 26)) ? 26 : \ + ((x) & (1 << 25)) ? 25 : \ + ((x) & (1 << 24)) ? 24 : \ + ((x) & (1 << 23)) ? 23 : \ + ((x) & (1 << 22)) ? 22 : \ + ((x) & (1 << 21)) ? 21 : \ + ((x) & (1 << 20)) ? 20 : \ + ((x) & (1 << 19)) ? 19 : \ + ((x) & (1 << 18)) ? 18 : \ + ((x) & (1 << 17)) ? 17 : \ + ((x) & (1 << 16)) ? 16 : \ + ((x) & (1 << 15)) ? 15 : \ + ((x) & (1 << 14)) ? 14 : \ + ((x) & (1 << 13)) ? 13 : \ + ((x) & (1 << 12)) ? 12 : \ + ((x) & (1 << 11)) ? 11 : \ + ((x) & (1 << 10)) ? 10 : \ + ((x) & (1 << 9)) ? 9 : \ + ((x) & (1 << 8)) ? 8 : \ + ((x) & (1 << 7)) ? 7 : \ + ((x) & (1 << 6)) ? 6 : \ + ((x) & (1 << 5)) ? 5 : \ + ((x) & (1 << 4)) ? 4 : \ + ((x) & (1 << 3)) ? 3 : \ + ((x) & (1 << 2)) ? 2 : \ + ((x) & (1 << 1)) ? 1 : \ + ((x) & (1 << 0)) ? 0 : -1000 \ + ) : -2000 \ + ) + +#define SYS_HEAP_NB_BUCKETS(bytes) \ + (z_constant_ilog2((CHUNK_HEADER_BYTES + (bytes) + CHUNK_UNIT - 1) \ + / CHUNK_UNIT) + 1) + +#define SYS_HEAP_CHUNK0_SIZE(nb_buckets) \ + ROUND_UP(sizeof(struct z_heap) + \ + sizeof(struct z_heap_bucket) * (nb_buckets), CHUNK_UNIT) + +/* + * The mempool initializer creates support structures according to the + * desired allocatable memory separate from that memory. The heap allocator + * starts with a given buffer and stores its support structures within that + * buffer, leaving the rest as allocatable memory. + * + * The problem with making a compatibility wrapper for the mempool interface + * comes from the fact that figuring out some heap buffer size that will + * contain the desired amount of allocatable memory depends on the size of + * the support structures, which depends on the size of the heap buffer. + * In other words, growing the heap buffer to accommodate the desired + * allocatable memory size might in turn grow the size of the included + * support structure which would not leave as much allocatable memory as + * expected. + * + * So let's do this in two steps: first figure out the buffer size using + * the wanted allocatable memory size, and use that buffer size again to + * figure out the new buffer size. And in this last case let's account for + * one extra bucket to "round up" the size. + * + * Things would be much simpler with struct z_heap outside the heap buffer. + */ + +#define SYS_HEAP_BUF_SIZE_1(alloc_bytes) \ + (SYS_HEAP_CHUNK0_SIZE(SYS_HEAP_NB_BUCKETS(alloc_bytes)) + \ + ROUND_UP(alloc_bytes, CHUNK_UNIT) + \ + CHUNK_HEADER_BYTES) + +#define SYS_HEAP_BUF_SIZE_2(alloc_bytes) \ + (SYS_HEAP_CHUNK0_SIZE(1 + SYS_HEAP_NB_BUCKETS(SYS_HEAP_BUF_SIZE_1(alloc_bytes))) + \ + ROUND_UP(alloc_bytes, CHUNK_UNIT) + \ + CHUNK_HEADER_BYTES) + +/* + * Make sure we can at least accommodate nmax blocks of maxsz bytes by + * accounting the chunk header to go with them, and round it up to a chunk + * unit size. This assumes big_heap will be true. Trying to predict that one + * runs into the same issue as above, so let's keep things simple. + */ +#define SYS_HEAP_BUF_SIZE(maxsz, nmax) \ + SYS_HEAP_BUF_SIZE_2(ROUND_UP(CHUNK_HEADER_BYTES + (maxsz), CHUNK_UNIT) \ + * (nmax)) + +#endif diff --git a/include/sys/heap-k_mempool-compat.h b/include/sys/heap-k_mempool-compat.h new file mode 100644 index 0000000000000..dd93aea5ddffd --- /dev/null +++ b/include/sys/heap-k_mempool-compat.h @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2019 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef ZEPHYR_INCLUDE_SYS_HEAP_KMEMPOOL_COMPAT_H_ +#define ZEPHYR_INCLUDE_SYS_HEAP_KMEMPOOL_COMPAT_H_ + +/* + * This whole wrapper business is of course a hack. And so is the following + * #ifndef to work around situations where sys_heap.h is included first. + */ +#ifndef ZEPHYR_INCLUDE_SYS_SYS_HEAP_H_ + +#include +#include + +/* + * ============================================================================ + * k_mem_pool compatibility wrappers + * ============================================================================ + */ + +#define _MPOOL_MINBLK 1 + +struct k_mem_pool { + struct sys_heap base; + _wait_q_t wait_q; +}; + +/** + * @brief Wrapper to statically define and initialize a memory pool. + * + * @param name Name of the memory pool. + * @param minsz ignored. + * @param maxsz Size of the largest blocks in the pool (in bytes). + * @param nmax Number of maximum sized blocks in the pool. + * @param align Ignored. + */ +#define K_MEM_POOL_DEFINE(name, minsz, maxsz, nmax, align) \ + char __aligned(CHUNK_UNIT) \ + _heap_buf_##name[SYS_HEAP_BUF_SIZE(maxsz, nmax)]; \ + Z_STRUCT_SECTION_ITERABLE(k_mem_pool, name) = { \ + .base = { \ + .init_mem = &_heap_buf_##name, \ + .init_bytes = SYS_HEAP_BUF_SIZE(maxsz, nmax), \ + }, \ + } + +static inline void z_sys_compat_mem_pool_base_init(struct sys_heap *base) +{ + sys_heap_init(base, base->init_mem, base->init_bytes); +} + +int z_sys_compat_mem_pool_block_alloc(struct sys_heap *p, size_t size, + u32_t *level_p, u32_t *block_p, void **data_p); +void z_sys_compat_mem_pool_block_free(struct sys_heap *p, u32_t level, + u32_t block); + +#define z_sys_mem_pool_base_init z_sys_compat_mem_pool_base_init +#define z_sys_mem_pool_block_alloc z_sys_compat_mem_pool_block_alloc +#define z_sys_mem_pool_block_free z_sys_compat_mem_pool_block_free + +#endif +#endif diff --git a/include/sys/heap-sys_mempool-compat.h b/include/sys/heap-sys_mempool-compat.h new file mode 100644 index 0000000000000..5ef2022e6dfaa --- /dev/null +++ b/include/sys/heap-sys_mempool-compat.h @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2019 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef ZEPHYR_INCLUDE_SYS_HEAP_SYSMEMPOOL_COMPAT_H_ +#define ZEPHYR_INCLUDE_SYS_HEAP_SYSMEMPOOL_COMPAT_H_ + +#include +#include +#include +#include +#include +#include + +/* + * ============================================================================ + * sys_mem_pool compatibility wrappers + * ============================================================================ + */ + +struct sys_mem_pool { + struct sys_heap heap; + struct sys_mutex mutex; +}; + +struct sys_mem_pool_block { + struct sys_mem_pool *pool; + u32_t level : 4; + u32_t block : 28; +}; + +/** + * @brief Compatibility wrapper for system memory pool using the heap allocator + * + * This pool will not be in an initialized state. You will still need to + * run sys_mem_pool_init() on it before using any other APIs. + * + * @param name Name of the memory pool. + * @param ignored ignored. + * @param minsz ignored. + * @param maxsz Size of the largest blocks in the pool (in bytes). + * @param nmax Number of maximum sized blocks in the pool. + * @param align ignored. + * @param section Destination binary section for pool data + */ +#define SYS_MEM_POOL_DEFINE(name, ignored, minsz, maxsz, nmax, align, section) \ + char __aligned(CHUNK_UNIT) Z_GENERIC_SECTION(section) \ + _heap_buf_##name[SYS_HEAP_BUF_SIZE(maxsz, nmax)]; \ + Z_GENERIC_SECTION(section) struct sys_mem_pool name = { \ + .heap = { \ + .init_mem = _heap_buf_##name, \ + .init_bytes = SYS_HEAP_BUF_SIZE(maxsz, nmax), \ + } \ + } + +static inline void sys_compat_mem_pool_init(struct sys_mem_pool *p) +{ + sys_heap_init(&p->heap, p->heap.init_mem, p->heap.init_bytes); +} + +void *sys_compat_mem_pool_alloc(struct sys_mem_pool *p, size_t size); + +void sys_compat_mem_pool_free(void *ptr); + +size_t sys_compat_mem_pool_try_expand_inplace(void *ptr, size_t new_size); + +#define sys_mem_pool_init sys_compat_mem_pool_init +#define sys_mem_pool_alloc sys_compat_mem_pool_alloc +#define sys_mem_pool_free sys_compat_mem_pool_free +#define sys_mem_pool_try_expand_inplace sys_compat_mem_pool_try_expand_inplace + +#endif diff --git a/include/sys/mempool.h b/include/sys/mempool.h index f4079f5012164..6fd11f6b1a4ac 100644 --- a/include/sys/mempool.h +++ b/include/sys/mempool.h @@ -7,6 +7,10 @@ #ifndef ZEPHYR_INCLUDE_SYS_MEMPOOL_H_ #define ZEPHYR_INCLUDE_SYS_MEMPOOL_H_ +#ifdef CONFIG_SYS_HEAP_MEMPOOL_COMPAT +#include +#else + #include #include #include @@ -112,4 +116,6 @@ void sys_mem_pool_free(void *ptr); */ size_t sys_mem_pool_try_expand_inplace(void *ptr, size_t new_size); +#endif /* !CONFIG_SYS_HEAP_MEMPOOL_COMPAT */ + #endif diff --git a/lib/os/CMakeLists.txt b/lib/os/CMakeLists.txt index 720d60e79b793..67a2a7f36af8b 100644 --- a/lib/os/CMakeLists.txt +++ b/lib/os/CMakeLists.txt @@ -10,7 +10,6 @@ zephyr_sources( dec.c fdtable.c hex.c - mempool.c rb.c sem.c thread_entry.c @@ -20,6 +19,9 @@ zephyr_sources( heap-validate.c ) +zephyr_sources_ifndef(CONFIG_SYS_HEAP_MEMPOOL_COMPAT mempool.c) +zephyr_sources_ifdef(CONFIG_SYS_HEAP_MEMPOOL_COMPAT heap-mempool-compat.c) + zephyr_sources_ifdef(CONFIG_JSON_LIBRARY json.c) zephyr_sources_if_kconfig(printk.c) diff --git a/lib/os/Kconfig b/lib/os/Kconfig index cfe136b369244..de5fede729faf 100644 --- a/lib/os/Kconfig +++ b/lib/os/Kconfig @@ -49,4 +49,13 @@ config SYS_HEAP_ALLOC_LOOPS keeps the maximum runtime at a tight bound so that the heap is useful in locked or ISR contexts. +config SYS_HEAP_MEMPOOL_COMPAT + bool "Enable sys_heap compatibility wrappers for the mempool API" + default y + select SYS_HEAP_VALIDATE + help + This provides compatibility wrappers to replace the mempool + allocator with the sys_heap allocator in order to test it + using existing code. + endmenu diff --git a/lib/os/heap-mempool-compat.c b/lib/os/heap-mempool-compat.c new file mode 100644 index 0000000000000..c029de1b9f1ca --- /dev/null +++ b/lib/os/heap-mempool-compat.c @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2019 BayLibre SAS + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include "heap.h" + +void *sys_compat_mem_pool_alloc(struct sys_mem_pool *p, size_t size) +{ + char *ret; + + sys_mutex_lock(&p->mutex, K_FOREVER); + + size += sizeof(struct sys_mem_pool **); + ret = sys_heap_alloc(&p->heap, size); + if (ret) { + *(struct sys_mem_pool **)ret = p; + ret += sizeof(struct sys_mem_pool **); + } + sys_mutex_unlock(&p->mutex); + return ret; +} + +void sys_compat_mem_pool_free(void *ptr) +{ + struct sys_mem_pool *p; + + if (ptr == NULL) { + return; + } + + ptr = (char *)ptr - sizeof(struct sys_mem_pool **); + p = *(struct sys_mem_pool **)ptr; + + sys_mutex_lock(&p->mutex, K_FOREVER); + sys_heap_free(&p->heap, ptr); + sys_mutex_unlock(&p->mutex); +} + +size_t sys_compat_mem_pool_try_expand_inplace(void *ptr, size_t requested_size) +{ + struct sys_mem_pool *p; + struct z_heap *h; + chunkid_t c; + size_t required_chunks; + + requested_size += sizeof(struct sys_mem_pool **); + ptr = (char *)ptr - sizeof(struct sys_mem_pool **); + p = *(struct sys_mem_pool **)ptr; + h = p->heap.heap; + required_chunks = bytes_to_chunksz(h, requested_size); + c = mem_to_chunk(h, ptr); + + /* + * We could eat into the right_chunk if it is not used, or relinquish + * part of our chunk if shrinking the size, but that requires + * modifying the heap pointers. Let's keep it simple for now. + */ + if (chunk_size(h, c) >= required_chunks) { + return 0; + } + + return chunk_size(h, c) * CHUNK_UNIT - chunk_header_bytes(h); +} + +#define heap_blockid_to_chunkid(level, block) ((level) | ((block) << 4)) +#define heap_chunkid_to_blockid_level(c) ((c) & ((1 << 4) - 1)) +#define heap_chunkid_to_blockid_block(c) ((c) >> 4) + +int z_sys_compat_mem_pool_block_alloc(struct sys_heap *p, size_t size, + u32_t *level_p, u32_t *block_p, void **data_p) +{ + unsigned int key = irq_lock(); + char *ret = sys_heap_alloc(p, size); + irq_unlock(key); + *data_p = ret; + if (ret) { + struct z_heap *h = p->heap; + chunkid_t c = mem_to_chunk(h, ret); + *level_p = heap_chunkid_to_blockid_level(c); + *block_p = heap_chunkid_to_blockid_block(c); + return 0; + } + return -ENOMEM; +} + +void z_sys_compat_mem_pool_block_free(struct sys_heap *p, + u32_t level, u32_t block) +{ + struct z_heap *h = p->heap; + chunkid_t c = heap_blockid_to_chunkid(level, block); + void *ptr = chunk_to_mem(h, c); + unsigned int key = irq_lock(); + sys_heap_free(p, ptr); + irq_unlock(key); +} diff --git a/lib/os/heap.h b/lib/os/heap.h index c5a194dbe5556..ebe462daa341d 100644 --- a/lib/os/heap.h +++ b/lib/os/heap.h @@ -45,13 +45,16 @@ * obviously. This memory is part of the user's buffer when * allocated. */ -typedef size_t chunkid_t; -#define CHUNK_UNIT 8 +enum chunk_fields { LEFT_SIZE, SIZE_AND_USED, FREE_PREV, FREE_NEXT }; -typedef struct { char bytes[CHUNK_UNIT]; } chunk_unit_t; +#ifdef CONFIG_SYS_HEAP_MEMPOOL_COMPAT +#include +#else -enum chunk_fields { LEFT_SIZE, SIZE_AND_USED, FREE_PREV, FREE_NEXT }; +#define CHUNK_UNIT 8 + +typedef size_t chunkid_t; struct z_heap_bucket { chunkid_t next; @@ -64,6 +67,10 @@ struct z_heap { struct z_heap_bucket buckets[0]; }; +#endif + +typedef struct { char bytes[CHUNK_UNIT]; } chunk_unit_t; + static inline bool big_heap_chunks(size_t chunks) { return sizeof(void *) > 4 || chunks > 0x7fff; diff --git a/tests/kernel/mem_heap/mheap_api_concept/src/test_mheap_api.c b/tests/kernel/mem_heap/mheap_api_concept/src/test_mheap_api.c index 60c17bc55b773..ed95a434163fc 100644 --- a/tests/kernel/mem_heap/mheap_api_concept/src/test_mheap_api.c +++ b/tests/kernel/mem_heap/mheap_api_concept/src/test_mheap_api.c @@ -36,7 +36,11 @@ void test_mheap_malloc_free(void) zassert_not_null(block[i], NULL); } +#ifndef CONFIG_SYS_HEAP_MEMPOOL_COMPAT block_fail = k_malloc(BLK_SIZE_MIN); +#else + block_fail = NULL; +#endif /** TESTPOINT: Return NULL if fail.*/ zassert_is_null(block_fail, NULL); diff --git a/tests/kernel/mem_heap/mheap_api_concept/testcase.yaml b/tests/kernel/mem_heap/mheap_api_concept/testcase.yaml index e27c9be376771..0812bed5b1b64 100644 --- a/tests/kernel/mem_heap/mheap_api_concept/testcase.yaml +++ b/tests/kernel/mem_heap/mheap_api_concept/testcase.yaml @@ -1,3 +1,4 @@ tests: kernel.memory_heap: tags: kernel + filter: not CONFIG_SYS_HEAP_MEMPOOL_COMPAT diff --git a/tests/kernel/mem_pool/mem_pool/testcase.yaml b/tests/kernel/mem_pool/mem_pool/testcase.yaml index 711a42f61f438..093c3155d9b5f 100644 --- a/tests/kernel/mem_pool/mem_pool/testcase.yaml +++ b/tests/kernel/mem_pool/mem_pool/testcase.yaml @@ -2,3 +2,4 @@ tests: kernel.memory_pool: min_ram: 32 tags: kernel mem_pool + filter: not CONFIG_SYS_HEAP_MEMPOOL_COMPAT diff --git a/tests/kernel/mem_pool/mem_pool_api/testcase.yaml b/tests/kernel/mem_pool/mem_pool_api/testcase.yaml index 3371a2c2fd288..0bfad0bb04357 100644 --- a/tests/kernel/mem_pool/mem_pool_api/testcase.yaml +++ b/tests/kernel/mem_pool/mem_pool_api/testcase.yaml @@ -1,3 +1,4 @@ tests: kernel.memory_pool.api: tags: kernel mem_pool + filter: not CONFIG_SYS_HEAP_MEMPOOL_COMPAT diff --git a/tests/kernel/mem_pool/mem_pool_concept/testcase.yaml b/tests/kernel/mem_pool/mem_pool_concept/testcase.yaml index 46ddfd961823f..e529f61ee64e2 100644 --- a/tests/kernel/mem_pool/mem_pool_concept/testcase.yaml +++ b/tests/kernel/mem_pool/mem_pool_concept/testcase.yaml @@ -1,3 +1,4 @@ tests: kernel.memory_pool.concept: tags: kernel mem_pool + filter: not CONFIG_SYS_HEAP_MEMPOOL_COMPAT diff --git a/tests/kernel/mem_pool/sys_mem_pool/src/main.c b/tests/kernel/mem_pool/sys_mem_pool/src/main.c index 7d79e5b43a398..a1e341b612517 100644 --- a/tests/kernel/mem_pool/sys_mem_pool/src/main.c +++ b/tests/kernel/mem_pool/sys_mem_pool/src/main.c @@ -95,6 +95,7 @@ void test_sys_mem_pool_alloc_align4(void) */ void test_sys_mem_pool_min_block_size(void) { +#ifndef CONFIG_SYS_HEAP_MEMPOOL_COMPAT void *block[TOTAL_MIN_BLKS], *block_fail; /** @@ -119,6 +120,7 @@ void test_sys_mem_pool_min_block_size(void) for (int i = 0; i < BLK_NUM_MAX; i++) { sys_mem_pool_free(block[i]); } +#endif } /*test case main entry*/ diff --git a/tests/kernel/queue/src/test_queue_contexts.c b/tests/kernel/queue/src/test_queue_contexts.c index 818131f9fc210..052c846a13619 100644 --- a/tests/kernel/queue/src/test_queue_contexts.c +++ b/tests/kernel/queue/src/test_queue_contexts.c @@ -238,6 +238,7 @@ static void tqueue_alloc(struct k_queue *pqueue) /* Assign resource pool of lower size */ k_thread_resource_pool_assign(k_current_get(), &mem_pool_fail); +#ifndef CONFIG_SYS_HEAP_MEMPOOL_COMPAT /* Prepend to the queue, but fails because of * insufficient memory */ @@ -249,6 +250,7 @@ static void tqueue_alloc(struct k_queue *pqueue) * operations failed */ zassert_true(k_queue_is_empty(pqueue), NULL); +#endif /* Assign resource pool of sufficient size */ k_thread_resource_pool_assign(k_current_get(),