[Barrelfish-users] [New release] free() error
Georgios Varisteas
yorgos at kth.se
Fri Apr 12 10:59:23 CEST 2013
Hi Kornillios,
Yes, the latest patch solves the issue without a visible increase in memory. Here are the prints:
During spanning the domain:
mem_serv.0: ******* FIXME: single_slot_alloc_init_raw buflen=256 != buflen_proper=280call stack: 0x4115df 0x401301
monitor.0: ******* FIXME: single_slot_alloc_init_raw buflen=8192 != buflen_proper=8216call stack: 0x4ad27f 0x44da42
while running and probably allocating memory:
pond20.20: ******* FIXME: single_slot_alloc_init_raw buflen=8192 != buflen_proper=8216call stack: 0x4d5dff 0x4d15aa
For the callstack, I assume you mean when my app crashes, so here it goes (can't do better as I have no debugger set up):
lib/barrelfish/slot_alloc/single_slot_alloc.c - sfree()
lib/barrelfish/slot_alloc/multi_slot_alloc.c - multi_free()
lib/barrelfish/slot_alloc/slot_alloc.c - slot_free()
lib/barrelfish/capabilities.c - cap_destroy()
lib/barrelfish/target/x86_64/pmap_target.c - do_single_unmap()
lib/barrelfish/target/x86_64/pmap_target.c - unmap()
lib/barrelfish/vspace/memobj_anon.c - unfill()
lib/barrelfish/vspace/mmu_aware.c - vspace_mmu_aware_unmap()
lib/barrelfish/morecore.c - morecore_free()
lib/newlib/newlib/libc/sys/barrelfish/oldmalloc.c - free()
cheers,
Georgios
________________________________________
From: Kornilios Kourtis [kornilios.kourtis at inf.ethz.ch]
Sent: Wednesday, April 10, 2013 13:51
To: Georgios Varisteas
Cc: barrelfish-users at lists.inf.ethz.ch
Subject: Re: [Barrelfish-users] [New release] free() error
Hi Georgios,
Again, thanks for reporting this and help us debug it!
On Thu, Apr 04, 2013 at 05:09:50PM +0000, Georgios Varisteas wrote:
> Hi again,
>
> Applying the patch left me unable to boot BF. The assertion added at
> single_slot_alloc.c:162 actually fails after booting core 12 [of 32].
>
> Monitor 0: booting x86_64 core 12 as '/x86_64/sbin/cpu loglevel=4'
> Kernel starting at address 0xffffff802a001000
> Barrelfish CPU driver starting on x86_64 apic_id 12
> assertion "SLAB_STATIC_SIZE(nslots / 2, sizeof(struct cnode_meta)) == buflen" failed: file "../src/lib/barrelfish/slot_alloc/single_slot_alloc.c", line 162, function: single_slot_alloc_init_raw
> Aborted
> kernel 0: monitor terminated; expect badness!
>
> I tested this just on Qemu but I do not think that it makes any difference.
>
> Removing the assertion, BF booted normally (or so it seems). Memory
> consumption is not that bad although visibly increased. Moreover the
> error is indeed fixed and my programs terminate normally.
>
> However, I'm going to revert the patch and choose the simple workaround
> of commenting out one of the calls to free(). In other parts of my
> code, I have other consecutive calls to free on non-consecutive memory
> blocks and get no error; so it takes very special circumstances for
> this behavior to appear which can easily be avoided without hacking BF
> internals.
>
> I will assist anyone who wants to solve this, though.
I was wondering if I could you up on that and have try another patch.
This changes the slab implementation so that it does not need the extra
space for the header, resulting in a very small increase to memory
consumption. It also records callers of single_slot_alloc_init_raw()
that provide the wrong size, but without crashing the app.
Can you please verify that this works for you, and also send me any FIXME
lines that are printed out? If you could also see what are the functions
in the callstack, that would be great.
cheers,
Kornilios.
diff --git a/include/barrelfish/core_state.h b/include/barrelfish/core_state.h
index 6072471..eb6aea5 100644
--- a/include/barrelfish/core_state.h
+++ b/include/barrelfish/core_state.h
@@ -72,12 +72,11 @@ struct slot_alloc_state {
struct slot_allocator_list head;
struct slot_allocator_list reserve;
- struct cnode_meta top_buf[SLOT_ALLOC_CNODE_SLOTS / 2];
- struct cnode_meta head_buf[SLOT_ALLOC_CNODE_SLOTS / 2];
- struct cnode_meta reserve_buf[SLOT_ALLOC_CNODE_SLOTS / 2];
-
+ char top_buf[SLAB_STATIC_SIZE(SLOT_ALLOC_CNODE_SLOTS / 2, sizeof(struct cnode_meta))];
+ char head_buf[SLAB_STATIC_SIZE(SLOT_ALLOC_CNODE_SLOTS / 2, sizeof(struct cnode_meta))];
+ char reserve_buf[SLAB_STATIC_SIZE(SLOT_ALLOC_CNODE_SLOTS / 2, sizeof(struct cnode_meta))];
struct single_slot_allocator rootca;
- struct cnode_meta root_buf[DEFAULT_CNODE_SLOTS / 2];
+ char root_buf[SLAB_STATIC_SIZE(DEFAULT_CNODE_SLOTS / 2, sizeof(struct cnode_meta))];
};
struct terminal_state;
diff --git a/include/barrelfish/slab.h b/include/barrelfish/slab.h
index 2f3ffff..f1aecb8 100644
--- a/include/barrelfish/slab.h
+++ b/include/barrelfish/slab.h
@@ -47,9 +47,15 @@ void slab_free(struct slab_alloc *slabs, void *block);
size_t slab_freecount(struct slab_alloc *slabs);
errval_t slab_default_refill(struct slab_alloc *slabs);
+// size of block header
+#define SLAB_BLOCK_HDRSIZE (sizeof(void *))
+// should be able to fit the header into the block
+#define SLAB_REAL_BLOCKSIZE(blocksize) \
+ (((blocksize) > SLAB_BLOCK_HDRSIZE) ? (blocksize) : SLAB_BLOCK_HDRSIZE)
+
/// Macro to compute the static buffer size required for a given allocation
#define SLAB_STATIC_SIZE(nblocks, blocksize) \
- ((nblocks) * ((blocksize) + sizeof(void *)) + sizeof(struct slab_head))
+ ((nblocks) * SLAB_REAL_BLOCKSIZE(blocksize) + sizeof(struct slab_head))
__END_DECLS
diff --git a/lib/barrelfish/slab.c b/lib/barrelfish/slab.c
index a8a9345..1a39385 100644
--- a/lib/barrelfish/slab.c
+++ b/lib/barrelfish/slab.c
@@ -17,12 +17,14 @@
#include <barrelfish/barrelfish.h>
#include <barrelfish/slab.h>
+#include <barrelfish/static_assert.h>
struct block_head {
struct block_head *next;///< Pointer to next block in free list
- char buf[0]; ///< Start of actual block data
};
+STATIC_ASSERT_SIZEOF(struct block_head, SLAB_BLOCK_HDRSIZE);
+
/**
* \brief Initialise a new slab allocator
*
@@ -34,7 +36,7 @@ void slab_init(struct slab_alloc *slabs, size_t blocksize,
slab_refill_func_t refill_func)
{
slabs->slabs = NULL;
- slabs->blocksize = blocksize;
+ slabs->blocksize = SLAB_REAL_BLOCKSIZE(blocksize);
slabs->refill_func = refill_func;
}
@@ -55,15 +57,15 @@ void slab_grow(struct slab_alloc *slabs, void *buf, size_t buflen)
buf = (char *)buf + sizeof(struct slab_head);
/* calculate number of blocks in buffer */
- size_t headed_blocksize = slabs->blocksize + sizeof(struct block_head);
- assert(buflen / headed_blocksize <= UINT32_MAX);
- head->free = head->total = buflen / headed_blocksize;
+ size_t blocksize = slabs->blocksize;
+ assert(buflen / blocksize <= UINT32_MAX);
+ head->free = head->total = buflen / blocksize;
assert(head->total > 0);
/* enqueue blocks in freelist */
struct block_head *bh = head->blocks = buf;
for (uint32_t i = head->total; i > 1; i--) {
- buf = (char *)buf + headed_blocksize;
+ buf = (char *)buf + blocksize;
bh->next = buf;
bh = buf;
}
@@ -111,7 +113,7 @@ void *slab_alloc(struct slab_alloc *slabs)
sh->blocks = bh->next;
sh->free--;
- return &bh->buf[0];
+ return bh;
}
/**
@@ -126,15 +128,15 @@ void slab_free(struct slab_alloc *slabs, void *block)
return;
}
- struct block_head *bh = (struct block_head *)block - 1;
+ struct block_head *bh = (struct block_head *)block;
/* find matching slab */
struct slab_head *sh;
- size_t headed_blocksize = slabs->blocksize + sizeof(struct block_head);
+ size_t blocksize = slabs->blocksize;
for (sh = slabs->slabs; sh != NULL; sh = sh->next) {
/* check if block falls inside this slab */
uintptr_t slab_limit = (uintptr_t)sh + sizeof(struct slab_head)
- + headed_blocksize * sh->total;
+ + blocksize * sh->total;
if ((uintptr_t)bh > (uintptr_t)sh && (uintptr_t)bh < slab_limit) {
break;
}
diff --git a/lib/barrelfish/slot_alloc/multi_slot_alloc.c b/lib/barrelfish/slot_alloc/multi_slot_alloc.c
index a12ab48..0cfecca 100644
--- a/lib/barrelfish/slot_alloc/multi_slot_alloc.c
+++ b/lib/barrelfish/slot_alloc/multi_slot_alloc.c
@@ -253,7 +253,7 @@ errval_t multi_slot_alloc_init(struct multi_slot_allocator *ret,
{
errval_t err;
nslots = ROUND_UP(nslots, DEFAULT_CNODE_SLOTS);
- size_t bufsize = sizeof(struct cnode_meta) * nslots / 2; // XXX: ?
+ size_t bufsize = SLAB_STATIC_SIZE(nslots/2, sizeof(struct cnode_meta)); // XXX?
ret->top = malloc(sizeof(struct single_slot_allocator));
if (!ret->top) {
diff --git a/lib/barrelfish/slot_alloc/single_slot_alloc.c b/lib/barrelfish/slot_alloc/single_slot_alloc.c
index 56e4d70..315d616 100644
--- a/lib/barrelfish/slot_alloc/single_slot_alloc.c
+++ b/lib/barrelfish/slot_alloc/single_slot_alloc.c
@@ -92,6 +92,15 @@ static errval_t sfree(struct slot_allocator *ca, struct capref cap)
// Freeing at the edge of walk
if (cap.slot == walk->slot + walk->space) {
walk->space++;
+
+ // check if we can merge walk to next
+ struct cnode_meta *next = walk->next;
+ if (next && next->slot == walk->slot + walk->space) {
+ walk->space += next->space;
+ walk->next = next->next;
+ slab_free(&sca->slab, next);
+ }
+
goto finish;
}
else if (cap.slot < walk->slot + walk->space) {
@@ -99,6 +108,13 @@ static errval_t sfree(struct slot_allocator *ca, struct capref cap)
goto unlock;
}
+ // Freing just before walk->next
+ if (walk->next && cap.slot + 1 == walk->next->slot) {
+ walk->next->slot = cap.slot;
+ walk->next->space++;
+ goto finish;
+ }
+
// Freeing after walk and before walk->next
if (walk->next && cap.slot < walk->next->slot) {
struct cnode_meta *new = walk->next;
@@ -143,6 +159,18 @@ errval_t single_slot_alloc_init_raw(struct single_slot_allocator *ret,
slab_init(&ret->slab, sizeof(struct cnode_meta), NULL);
if (buflen > 0) {
+ // check for callers that do not provide enough buffer space
+ #if !defined(NDEBUG)
+ size_t buflen_proper = SLAB_STATIC_SIZE(nslots / 2,
+ sizeof(struct cnode_meta));
+ if (buflen != buflen_proper) {
+ debug_printf("******* FIXME: %s buflen=%lu != buflen_proper=%lu"
+ "call stack: %p %p\n",
+ __FUNCTION__, buflen, buflen_proper,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
+ }
+ #endif
slab_grow(&ret->slab, buf, buflen);
}
@@ -166,7 +194,7 @@ errval_t single_slot_alloc_init(struct single_slot_allocator *ret,
if (err_is_fail(err)) {
return err_push(err, LIB_ERR_CNODE_CREATE);
}
- size_t buflen = sizeof(struct cnode_meta) * nslots / 2; // worst case
+ size_t buflen = SLAB_STATIC_SIZE(nslots / 2, sizeof(struct cnode_meta));
void *buf = malloc(buflen);
if (!buf) {
return LIB_ERR_MALLOC_FAIL;
diff --git a/lib/spawndomain/spawn.c b/lib/spawndomain/spawn.c
index 2e9ea56..920c086 100644
--- a/lib/spawndomain/spawn.c
+++ b/lib/spawndomain/spawn.c
@@ -167,7 +167,7 @@ static errval_t spawn_setup_vspace(struct spawninfo *si)
/* Init pagecn's slot allocator */
// XXX: satisfy a peculiarity of the single_slot_alloc_init_raw API
- size_t bufsize = sizeof(struct cnode_meta) * PAGE_CNODE_SLOTS / 2;
+ size_t bufsize = SLAB_STATIC_SIZE(PAGE_CNODE_SLOTS/2, sizeof(struct cnode_meta));
void *buf = malloc(bufsize);
assert(buf != NULL);
--
Kornilios Kourtis
More information about the Barrelfish-users
mailing list