Attention is currently required from: Furquan Shaikh, Aaron Durbin.
Julius Werner would like Furquan Shaikh and Aaron Durbin to review this change.
mem_pool: Track the last two allocations (not just one)
This patch changes the mem_pool implementation to track the last two
allocations (instead of just the last) and allow them both to be freed
if the mem_pool_free() calls come in in reverse order. This is intended
as a specific optimization for the CBFS cache case when a compressed
file is mapped on a platform that doesn't natively support
memory-mapping flash. In this case, cbfs_map() (chaining through to
_cbfs_alloc() with allocator == NULL) will call
mem_pool_alloc(&cbfs_cache) to allocate space for the uncompressed file
data. It will then call cbfs_load_and_decompress() to fill that
allocation, which will notice the compression and in turn call
rdev_mmap_full() to map the compressed data (which on platforms without
memory-mapped flash usually results in a second call to
mem_pool_alloc(&cbfs_cache)). It then runs the decompression algorithm
and calls rdev_munmap() on the compressed data buffer (the latter one in
the allocation sequence), leading to a mem_pool_free(). The remaining
buffer with the uncompressed data is returned out of cbfs_map() to the
caller, which should eventually call cbfs_unmap() to mem_pool_free()
that as well. This patch allows this simple case to succeed without
leaking any permanent allocations on the cache. (More complicated cases
where the caller maps other files before cbfs_unmap()ing the first one
may still lead to leaks, but those are very rare in practice.)
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ic5c4c56a8482752ed65e10cf35565f9b2d3e4b17
---
M src/commonlib/include/commonlib/mem_pool.h
M src/commonlib/mem_pool.c
2 files changed, 20 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/52087/1
diff --git a/src/commonlib/include/commonlib/mem_pool.h b/src/commonlib/include/commonlib/mem_pool.h
index bde9e41..6c85397 100644
--- a/src/commonlib/include/commonlib/mem_pool.h
+++ b/src/commonlib/include/commonlib/mem_pool.h
@@ -7,11 +7,14 @@
#include <stdint.h>
/*
- * The memory pool allows one to allocate memory from a fixed size buffer
- * that also allows freeing semantics for reuse. However, the current
- * limitation is that the most recent allocation is the only one that
- * can be freed. If one tries to free any allocation that isn't the
- * most recently allocated it will result in a leak within the memory pool.
+ * The memory pool allows one to allocate memory from a fixed size buffer that
+ * also allows freeing semantics for reuse. However, the current limitation is
+ * that only the two most recent allocations can be freed (in exact reverse
+ * order). If one tries to free any allocation that isn't at the top of the
+ * allocation stack, or one allocates more than two buffers in a row without
+ * freeing, it will result in a leak within the memory pool. (Two allocations
+ * were chosen to optimize for the CBFS cache case which may need two buffers
+ * to map a single compressed file, and will free them in reverse order.)
*
* The memory returned by allocations are at least 8 byte aligned. Note
* that this requires the backing buffer to start on at least an 8 byte
@@ -22,20 +25,23 @@
uint8_t *buf;
size_t size;
uint8_t *last_alloc;
+ uint8_t *second_to_last_alloc;
size_t free_offset;
};
-#define MEM_POOL_INIT(buf_, size_) \
- { \
- .buf = (buf_), \
- .size = (size_), \
- .last_alloc = NULL, \
- .free_offset = 0, \
+#define MEM_POOL_INIT(buf_, size_) \
+ { \
+ .buf = (buf_), \
+ .size = (size_), \
+ .last_alloc = NULL, \
+ .second_to_last_alloc = NULL, \
+ .free_offset = 0, \
}
static inline void mem_pool_reset(struct mem_pool *mp)
{
mp->last_alloc = NULL;
+ mp->second_to_last_alloc = NULL;
mp->free_offset = 0;
}
diff --git a/src/commonlib/mem_pool.c b/src/commonlib/mem_pool.c
index 2ad1099..c300c65 100644
--- a/src/commonlib/mem_pool.c
+++ b/src/commonlib/mem_pool.c
@@ -17,6 +17,7 @@
p = &mp->buf[mp->free_offset];
mp->free_offset += sz;
+ mp->second_to_last_alloc = mp->last_alloc;
mp->last_alloc = p;
return p;
@@ -29,6 +30,7 @@
return;
mp->free_offset = mp->last_alloc - mp->buf;
+ mp->last_alloc = mp->second_to_last_alloc;
/* No way to track allocation before this one. */
- mp->last_alloc = NULL;
+ mp->second_to_last_alloc = NULL;
}
To view, visit change 52087. To unsubscribe, or for help writing mail filters, visit settings.