From 78b72a57f85c167fa743a699189f725a729d44f1 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 20 Nov 2020 10:39:25 +0100 Subject: [PATCH 1/6] core: optimise ALIGN_UP() to only use 1 division No need for two divisions for an alignment macro, one is enough. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/sof/common.h b/src/include/sof/common.h index 359f908e9f97..0fac6f049606 100644 --- a/src/include/sof/common.h +++ b/src/include/sof/common.h @@ -12,7 +12,7 @@ /* Align the number to the nearest alignment value */ #define IS_ALIGNED(size, alignment) ((size) % (alignment) == 0) #define ALIGN_UP(size, alignment) \ - ((size) + (((alignment) - ((size) % (alignment))) % (alignment))) + ((size) + (alignment) - 1 - ((size) + (alignment) - 1) % (alignment)) #define ALIGN_DOWN(size, alignment) \ ((size) - ((size) % (alignment))) #define ALIGN ALIGN_UP From 2257af57427d879549c1b8cf39521116651fda77 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 20 Nov 2020 10:43:59 +0100 Subject: [PATCH 2/6] alloc: simplify align_ptr() Simplify the align_ptr() function by re-using an existing ALIGN() macro. Signed-off-by: Guennadi Liakhovetski --- src/lib/alloc.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/lib/alloc.c b/src/lib/alloc.c index af12515982c7..6092c819e1d3 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -190,17 +190,14 @@ static void *rmalloc_sys(struct mm_heap *heap, uint32_t flags, int caps, size_t static void *align_ptr(struct mm_heap *heap, uint32_t alignment, void *ptr, struct block_hdr *hdr) { - int mod_align = 0; - /* Save unaligned ptr to block hdr */ hdr->unaligned_ptr = ptr; /* If ptr is not already aligned we calculate alignment shift */ - if (alignment && (uintptr_t)ptr % alignment) - mod_align = alignment - ((uintptr_t)ptr % alignment); + if (!alignment) + return ptr; - /* Calculate aligned pointer */ - return (char *)ptr + mod_align; + return (void *)ALIGN((uintptr_t)ptr, alignment); } /* allocate single block */ From 2ee22c629fddd84ce983e7202ffd99acaf942467 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 20 Nov 2020 13:17:19 +0100 Subject: [PATCH 3/6] alloc: remove unused "caps" parameter alloc_block() and alloc_cont_blocks() don't use their "caps" parameter, remote it. Signed-off-by: Guennadi Liakhovetski --- src/lib/alloc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/alloc.c b/src/lib/alloc.c index 6092c819e1d3..6ee6a09e5868 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -202,7 +202,7 @@ static void *align_ptr(struct mm_heap *heap, uint32_t alignment, /* allocate single block */ static void *alloc_block(struct mm_heap *heap, int level, - uint32_t caps, uint32_t alignment) + uint32_t alignment) { struct block_map *map = &heap->map[level]; struct block_hdr *hdr; @@ -240,7 +240,7 @@ static void *alloc_block(struct mm_heap *heap, int level, /* allocates continuous blocks */ static void *alloc_cont_blocks(struct mm_heap *heap, int level, - uint32_t caps, size_t bytes, uint32_t alignment) + size_t bytes, uint32_t alignment) { struct block_map *map = &heap->map[level]; struct block_hdr *hdr; @@ -416,7 +416,7 @@ static void *get_ptr_from_heap(struct mm_heap *heap, uint32_t flags, } /* free block space exists */ - ptr = alloc_block(heap, i, caps, alignment); + ptr = alloc_block(heap, i, alignment); platform_shared_commit(map, sizeof(*map)); @@ -801,7 +801,7 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, platform_shared_commit(map, sizeof(*map)); /* found: grab a block */ - ptr = alloc_block(heap, i, caps, alignment); + ptr = alloc_block(heap, i, alignment); break; } temp_bytes = bytes; @@ -825,7 +825,7 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, /* allocate if block size is smaller than request */ if (heap->size >= bytes && map->block_size < bytes) { - ptr = alloc_cont_blocks(heap, i, caps, + ptr = alloc_cont_blocks(heap, i, bytes, alignment); if (ptr) { platform_shared_commit(map, From 76a2e6569c35bfd7f6107b3d3ced2fbb5e9f23af Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 20 Nov 2020 13:20:43 +0100 Subject: [PATCH 4/6] alloc: fix the general one-buffer memory allocation case The condition "size + alignment <= block_size" for allocating memory from a signle buffer is sufficient but not precise enough. For example if we want to allocate 20 bytes with 64-byte alignment, a 32-byte buffer *might* be sufficient if it's suitably aligned. Fix the algorithm to account for such cases. Signed-off-by: Guennadi Liakhovetski --- src/lib/alloc.c | 104 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 74 insertions(+), 30 deletions(-) diff --git a/src/lib/alloc.c b/src/lib/alloc.c index 6ee6a09e5868..5cf9fc51ca65 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -201,18 +201,21 @@ static void *align_ptr(struct mm_heap *heap, uint32_t alignment, } /* allocate single block */ -static void *alloc_block(struct mm_heap *heap, int level, - uint32_t alignment) +static void *alloc_block_index(struct mm_heap *heap, int level, + uint32_t alignment, int index) { struct block_map *map = &heap->map[level]; struct block_hdr *hdr; void *ptr; int i; - hdr = &map->block[map->first_free]; + if (index < 0) + index = map->first_free; map->free_count--; - ptr = (void *)(map->base + map->first_free * map->block_size); + + hdr = &map->block[index]; + ptr = (void *)(map->base + index * map->block_size); ptr = align_ptr(heap, alignment, ptr, hdr); hdr->size = 1; @@ -221,15 +224,16 @@ static void *alloc_block(struct mm_heap *heap, int level, heap->info.used += map->block_size; heap->info.free -= map->block_size; - /* find next free */ - for (i = map->first_free; i < map->count; ++i) { - hdr = &map->block[i]; + if (index == map->first_free) + /* find next free */ + for (i = map->first_free; i < map->count; ++i) { + hdr = &map->block[i]; - if (hdr->used == 0) { - map->first_free = i; - break; + if (hdr->used == 0) { + map->first_free = i; + break; + } } - } platform_shared_commit(map->block, sizeof(*map->block) * map->count); platform_shared_commit(map, sizeof(*map)); @@ -238,6 +242,12 @@ static void *alloc_block(struct mm_heap *heap, int level, return ptr; } +static void *alloc_block(struct mm_heap *heap, int level, + uint32_t alignment) +{ + return alloc_block_index(heap, level, alignment, -1); +} + /* allocates continuous blocks */ static void *alloc_cont_blocks(struct mm_heap *heap, int level, size_t bytes, uint32_t alignment) @@ -777,45 +787,79 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, uint32_t caps, size_t bytes, uint32_t alignment) { struct block_map *map; - int i, temp_bytes = bytes; + unsigned int i, j, temp_bytes = bytes; void *ptr = NULL; /* Only allow alignment as a power of 2 */ if ((alignment & (alignment - 1)) != 0) panic(SOF_IPC_PANIC_MEM); + /* + * There are several cases when a memory allocation request can be + * satisfied with one buffer: + * 1. allocate 30 bytes 256-byte aligned from 32 byte buffers. 1 buffer + * is enough but you have to find a suitable one. + * 2. allocate 20 bytes 256-byte aligned from 0x180 byte buffers. 1 + * buffer is always enough. + * 3. allocate 200 bytes 256-byte aligned from 0x180 byte buffers. 1 + * buffer is enough, but not every buffer is suitable. + */ + /* will request fit in single block */ - for (i = 0; i < heap->blocks; i++) { - map = &heap->map[i]; + for (i = 0, map = heap->map; i < heap->blocks; i++, map++) { + struct block_hdr *hdr; + uintptr_t free_start; - /* size of requested buffer is adjusted for alignment purposes - * we check if first free block is already aligned if not - * we need to allocate bigger size for alignment - */ - if ((map->base + (map->block_size * map->first_free)) - % alignment) - temp_bytes += alignment; + if (map->block_size < bytes || !map->free_count) + continue; - /* Check if blocks are big enough and at least one is free */ - if (map->block_size >= temp_bytes && map->free_count) { + if (!alignment) { platform_shared_commit(map, sizeof(*map)); /* found: grab a block */ ptr = alloc_block(heap, i, alignment); break; } - temp_bytes = bytes; - platform_shared_commit(map, sizeof(*map)); - } + /* + * Usually block sizes are a power of 2 and all blocks are + * respectively aligned. But it's also possible to have + * non-power of 2 sized blocks, e.g. to optimize for typical + * ALSA allocations a map with 0x180 byte buffers can be used. + * For performance reasons we could first check the power-of-2 + * case. This can be added as an optimization later. + */ + for (j = map->first_free, hdr = map->block + j, + free_start = map->base + map->block_size * j; + j < map->count; + j++, hdr++, free_start += map->block_size) { + uintptr_t aligned; - /* size of requested buffer is adjusted for alignment purposes - * since we span more blocks we have to assume worst case scenario - */ - bytes += alignment; + if (hdr->used) + continue; + + aligned = ALIGN(free_start, alignment); + + if (aligned + bytes > free_start + map->block_size) + continue; + + /* Found, alloc_block_index() cannot fail */ + ptr = alloc_block_index(heap, i, alignment, j); + temp_bytes += aligned - free_start; + break; + } + + if (ptr) + break; + } /* request spans > 1 block */ if (!ptr) { + /* size of requested buffer is adjusted for alignment purposes + * since we span more blocks we have to assume worst case scenario + */ + bytes += alignment; + /* * Find the best block size for request. We know, that we failed * to find a single large enough block, so, skip those. From 97a65721541cd0068ad26034385a04a0340faabd Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 20 Nov 2020 14:06:48 +0100 Subject: [PATCH 5/6] alloc: no need to check the same condition repeatedly alloc_heap_buffer() is trying to allocate memory from a single heap, no need to check its size on each iteration of an internal loop over maps. Signed-off-by: Guennadi Liakhovetski --- src/lib/alloc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib/alloc.c b/src/lib/alloc.c index 5cf9fc51ca65..c8755eeca9e0 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -790,6 +790,9 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, unsigned int i, j, temp_bytes = bytes; void *ptr = NULL; + if (heap->size < bytes) + return NULL; + /* Only allow alignment as a power of 2 */ if ((alignment & (alignment - 1)) != 0) panic(SOF_IPC_PANIC_MEM); @@ -868,7 +871,7 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, map = &heap->map[i]; /* allocate if block size is smaller than request */ - if (heap->size >= bytes && map->block_size < bytes) { + if (map->block_size < bytes) { ptr = alloc_cont_blocks(heap, i, bytes, alignment); if (ptr) { @@ -887,8 +890,6 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, bzero(ptr, temp_bytes); #endif - platform_shared_commit(heap, sizeof(*heap)); - return ptr; } @@ -909,6 +910,7 @@ static void *_balloc_unlocked(uint32_t flags, uint32_t caps, size_t bytes, break; ptr = alloc_heap_buffer(heap, flags, caps, bytes, alignment); + platform_shared_commit(heap, sizeof(*heap)); if (ptr) break; From e832a6e377c09e5ee8d1cfe76930d89072313511 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 20 Nov 2020 14:40:39 +0100 Subject: [PATCH 6/6] alloc: pick up allocation start correctly When allocating multiple buffers the first free buffer might not contain a suitably aligned start address. Skip it instead of allocating it with the others. Also don't add alignment to the allocation size, but find a suitably aligned start address instead. Signed-off-by: Guennadi Liakhovetski --- src/lib/alloc.c | 87 +++++++++++++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/src/lib/alloc.c b/src/lib/alloc.c index c8755eeca9e0..6a6b9bd5f4c3 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -254,56 +254,84 @@ static void *alloc_cont_blocks(struct mm_heap *heap, int level, { struct block_map *map = &heap->map[level]; struct block_hdr *hdr; - void *ptr = NULL; - void *unaligned_ptr; - unsigned int start = map->first_free; + void *ptr = NULL, *unaligned_ptr; unsigned int current; - unsigned int count = bytes / map->block_size; - unsigned int remaining = 0; - - if (bytes % map->block_size) - count++; + unsigned int count = 0; /* keep compiler quiet */ + unsigned int start = 0; /* keep compiler quiet */ + uintptr_t blk_start = 0, aligned = 0; /* keep compiler quiet */ + size_t found = 0, total_bytes = bytes; /* check if we have enough consecutive blocks for requested * allocation size. */ - for (current = map->first_free; current < map->count && - remaining < count; current++) { - hdr = &map->block[current]; + for (current = map->first_free, hdr = map->block + current; + current < map->count && found < total_bytes; + current++, hdr++) { + if (hdr->used) { + /* Restart the search */ + found = 0; + count = 0; + total_bytes = bytes; + continue; + } + + if (!found) { + blk_start = map->base + current * map->block_size; + start = current; + + /* Check if we can start a range here */ + if (alignment) { + aligned = ALIGN(blk_start, alignment); + + if (blk_start & (alignment - 1) && + aligned >= blk_start + map->block_size) + /* + * This block doesn't contain an address + * with required alignment, it is useless + * as the beginning of the range + */ + continue; + + /* + * Found a potentially suitable beginning of a range, + * from here we'll check if we get enough blocks + */ + total_bytes += aligned - blk_start; + } else { + aligned = blk_start; + } + } - if (hdr->used) - remaining = 0;/* used, not suitable, reset */ - else if (!remaining++) - start = current;/* new start */ + count++; + found += map->block_size; } - if (count > map->count || remaining < count) { - tr_err(&mem_tr, "%d blocks needed for allocation but only %d blocks are remaining", - count, remaining); + if (found < total_bytes) { + tr_err(&mem_tr, "failed to allocate %u", total_bytes); goto out; } + ptr = (void *)aligned; + /* we found enough space, let's allocate it */ map->free_count -= count; - ptr = (void *)(map->base + start * map->block_size); - unaligned_ptr = ptr; + unaligned_ptr = (void *)blk_start; hdr = &map->block[start]; hdr->size = count; - ptr = align_ptr(heap, alignment, ptr, hdr); - heap->info.used += count * map->block_size; heap->info.free -= count * map->block_size; /* update first_free if needed */ if (map->first_free == start) /* find first available free block */ - for (map->first_free += count; map->first_free < map->count; - map->first_free++) { - hdr = &map->block[map->first_free]; - - if (!hdr->used) + for (current = map->first_free + count, hdr = &map->block[current]; + current < map->count; + current++, hdr++) { + if (!hdr->used) { + map->first_free = current; break; + } } /* update each block */ @@ -858,11 +886,6 @@ static void *alloc_heap_buffer(struct mm_heap *heap, uint32_t flags, /* request spans > 1 block */ if (!ptr) { - /* size of requested buffer is adjusted for alignment purposes - * since we span more blocks we have to assume worst case scenario - */ - bytes += alignment; - /* * Find the best block size for request. We know, that we failed * to find a single large enough block, so, skip those.