Skip to content

Commit

Permalink
kernel/mempool: Handle transient failure condition
Browse files Browse the repository at this point in the history
The sys_mem_pool implementation has a subtle error case where it
detected a simultaneous allocation after having released the lock, in
which case exactly one of the racing allocators will return with
-EAGAIN (the other one suceeds of course).

I documented this condition at the lower level, but forgot to actually
handle it at the k_mem_pool level where we want to retry once before
going to sleep, as it doesn't generally represent an empty heap.  It
got caught by code auditing in:

zephyrproject-rtos#6757

(Full disclosure: I tested this by whiteboxing the first failure.  I
wasn't able to put together a rig to reliably exercise the actual
race.)

This patch also fixes a noop thinko in the return logic in the same
function, which contained:

   (ret == -EAGAIN) || (ret && ret != -ENOMEM)

The first term is needless and implied by the second.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
  • Loading branch information
Andy Ross committed May 24, 2018
1 parent 45221a9 commit 453b889
Showing 1 changed file with 21 additions and 3 deletions.
24 changes: 21 additions & 3 deletions kernel/mempool.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,32 @@ int k_mem_pool_alloc(struct k_mem_pool *p, struct k_mem_block *block,
while (1) {
u32_t level_num, block_num;

ret = _sys_mem_pool_block_alloc(&p->base, size, &level_num,
&block_num, &block->data);
/* There is a "managed race" in alloc that can fail
* (albeit in a well-defined way, see comments there)
* with -EAGAIN when simultaneous allocations happen.
* Retry exactly once before sleeping to resolve it.
* If we're so contended that it fails twice, then we
* clearly want to block.
*/
for (int i = 0; i < 2; i++) {
ret = _sys_mem_pool_block_alloc(&p->base, size,
&level_num, &block_num,
&block->data);
if (ret != -EAGAIN) {
break;
}
}

if (ret == -EAGAIN) {
ret = -ENOMEM;
}

block->id.pool = pool_id(p);
block->id.level = level_num;
block->id.block = block_num;

if (ret == 0 || timeout == K_NO_WAIT ||
ret == -EAGAIN || (ret && ret != -ENOMEM)) {
(ret && ret != -ENOMEM)) {
return ret;
}

Expand Down

0 comments on commit 453b889

Please sign in to comment.