From bf8fd5f42c48f8330f015b57ac68c4d11ed16d73 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Thu, 3 Mar 2022 13:01:54 +0800 Subject: [PATCH 1/2] Heap: fix free bytes calculation for TLSF heap * Closes https://github.com/espressif/esp-idf/issues/8270 --- components/heap/multi_heap.c | 11 +++++++++- components/heap/test/test_malloc_caps.c | 6 ++++-- .../test_multi_heap_host/test_multi_heap.cpp | 20 +++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index 059f6c19af..cf944f010f 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -197,6 +197,7 @@ void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size) void *result = tlsf_malloc(heap->heap_data, size); if(result) { heap->free_bytes -= tlsf_block_size(result); + heap->free_bytes -= tlsf_alloc_overhead(); if (heap->free_bytes < heap->minimum_free_bytes) { heap->minimum_free_bytes = heap->free_bytes; } @@ -217,6 +218,7 @@ void multi_heap_free_impl(multi_heap_handle_t heap, void *p) multi_heap_internal_lock(heap); heap->free_bytes += tlsf_block_size(p); + heap->free_bytes += tlsf_alloc_overhead(); tlsf_free(heap->heap_data, p); multi_heap_internal_unlock(heap); } @@ -239,6 +241,8 @@ void *multi_heap_realloc_impl(multi_heap_handle_t heap, void *p, size_t size) size_t previous_block_size = tlsf_block_size(p); void *result = tlsf_realloc(heap->heap_data, p, size); if(result) { + /* No need to subtract the tlsf_alloc_overhead() as it has already + * been subtracted when allocating the block at first with malloc */ heap->free_bytes += previous_block_size; heap->free_bytes -= tlsf_block_size(result); if (heap->free_bytes < heap->minimum_free_bytes) { @@ -270,6 +274,7 @@ void *multi_heap_aligned_alloc_impl_offs(multi_heap_handle_t heap, size_t size, void *result = tlsf_memalign_offs(heap->heap_data, alignment, size, offset); if(result) { heap->free_bytes -= tlsf_block_size(result); + heap->free_bytes -= tlsf_alloc_overhead(); if(heap->free_bytes < heap->minimum_free_bytes) { heap->minimum_free_bytes = heap->free_bytes; } @@ -361,6 +366,7 @@ static void multi_heap_get_info_tlsf(void* ptr, size_t size, int used, void* use void multi_heap_get_info_impl(multi_heap_handle_t heap, multi_heap_info_t *info) { uint32_t sl_interval; + uint32_t overhead; memset(info, 0, sizeof(multi_heap_info_t)); @@ -370,7 +376,10 @@ void multi_heap_get_info_impl(multi_heap_handle_t heap, multi_heap_info_t *info) multi_heap_internal_lock(heap); tlsf_walk_pool(tlsf_get_pool(heap->heap_data), multi_heap_get_info_tlsf, info); - info->total_allocated_bytes = (heap->pool_size - tlsf_size()) - heap->free_bytes; + /* TLSF has an overhead per block. Calculate the total amoun of overhead, it shall not be + * part of the allocated bytes */ + overhead = info->allocated_blocks * tlsf_alloc_overhead(); + info->total_allocated_bytes = (heap->pool_size - tlsf_size()) - heap->free_bytes - overhead; info->minimum_free_bytes = heap->minimum_free_bytes; info->total_free_bytes = heap->free_bytes; if (info->largest_free_block) { diff --git a/components/heap/test/test_malloc_caps.c b/components/heap/test/test_malloc_caps.c index b78a088847..857e68ac68 100644 --- a/components/heap/test/test_malloc_caps.c +++ b/components/heap/test/test_malloc_caps.c @@ -122,8 +122,10 @@ TEST_CASE("IRAM_8BIT capability test", "[heap]") TEST_ASSERT((((int)ptr)&0xFF000000)==0x40000000); - TEST_ASSERT(heap_caps_get_free_size(MALLOC_CAP_IRAM_8BIT) == (free_size - heap_caps_get_allocated_size(ptr))); - TEST_ASSERT(heap_caps_get_free_size(MALLOC_CAP_32BIT) == (free_size32 - heap_caps_get_allocated_size(ptr))); + /* As the heap allocator may present an overhead for allocated blocks, + * we need to check that the free heap size is now smaller than former free size. */ + TEST_ASSERT(heap_caps_get_free_size(MALLOC_CAP_IRAM_8BIT) <= (free_size - heap_caps_get_allocated_size(ptr))); + TEST_ASSERT(heap_caps_get_free_size(MALLOC_CAP_32BIT) <= (free_size32 - heap_caps_get_allocated_size(ptr))); free(ptr); } diff --git a/components/heap/test_multi_heap_host/test_multi_heap.cpp b/components/heap/test_multi_heap_host/test_multi_heap.cpp index 0a730e1696..e85cceddc6 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -503,3 +503,23 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") printf("[ALIGNED_ALLOC] heap_size after: %d \n", multi_heap_free_size(heap)); REQUIRE((old_size - multi_heap_free_size(heap)) <= leakage); } + +// TLSF has some overhead when allocating blocks, check that overhead +TEST_CASE("multi_heap allocation overhead", "[multi_heap]") +{ + uint8_t heapdata[4 * 1024]; + size_t alloc_size = 256; + multi_heap_handle_t heap = multi_heap_register(heapdata, sizeof(heapdata)); + size_t free_bytes_1 = multi_heap_free_size(heap); + + /* Allocate any amount of data, in any case there will be an overhead */ + void *x = multi_heap_malloc(heap, alloc_size); + + /* free_bytes_2 should be free_bytes_1 - alloc_size - overhead. + * We don't know the value of overhead, let's check that it is non-zero */ + size_t free_bytes_2 = multi_heap_free_size(heap); + REQUIRE( free_bytes_1 > free_bytes_2 ); + REQUIRE( free_bytes_1 - free_bytes_2 > alloc_size ); + + multi_heap_free(heap, x); +} From 751c546a68535df5a71cd61058caeec8f6398412 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Mon, 11 Apr 2022 11:36:52 +0800 Subject: [PATCH 2/2] Heap: fix typos in test and component --- components/heap/multi_heap.c | 2 +- components/heap/test/test_malloc_caps.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index cf944f010f..2c85c1f7de 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -376,7 +376,7 @@ void multi_heap_get_info_impl(multi_heap_handle_t heap, multi_heap_info_t *info) multi_heap_internal_lock(heap); tlsf_walk_pool(tlsf_get_pool(heap->heap_data), multi_heap_get_info_tlsf, info); - /* TLSF has an overhead per block. Calculate the total amoun of overhead, it shall not be + /* TLSF has an overhead per block. Calculate the total amount of overhead, it shall not be * part of the allocated bytes */ overhead = info->allocated_blocks * tlsf_alloc_overhead(); info->total_allocated_bytes = (heap->pool_size - tlsf_size()) - heap->free_bytes - overhead; diff --git a/components/heap/test/test_malloc_caps.c b/components/heap/test/test_malloc_caps.c index 857e68ac68..a2d27050a4 100644 --- a/components/heap/test/test_malloc_caps.c +++ b/components/heap/test/test_malloc_caps.c @@ -123,7 +123,7 @@ TEST_CASE("IRAM_8BIT capability test", "[heap]") TEST_ASSERT((((int)ptr)&0xFF000000)==0x40000000); /* As the heap allocator may present an overhead for allocated blocks, - * we need to check that the free heap size is now smaller than former free size. */ + * we need to check that the free heap size is now smaller or equal to the former free size. */ TEST_ASSERT(heap_caps_get_free_size(MALLOC_CAP_IRAM_8BIT) <= (free_size - heap_caps_get_allocated_size(ptr))); TEST_ASSERT(heap_caps_get_free_size(MALLOC_CAP_32BIT) <= (free_size32 - heap_caps_get_allocated_size(ptr)));