patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 8 Packages Projects Releases Wiki Activity

kernel/mempool: Handle transient failure condition

Browse source 
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:

https://github.com/zephyrproject-rtos/zephyr/issues/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>
This commit is contained in:
Andy Ross 2018-05-24 13:27:35 -07:00 committed by Anas Nashif
parent 6eabea3a7e
commit 75398d2c38
1 changed files with 21 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
kernel/mempool.c
View file

@ -62,14 +62,32 @@ int k_mem_pool_alloc(struct k_mem_pool *p, struct k_mem_block *block,
while (1) { while (1) {
u32_t level_num, block_num; u32_t level_num, block_num;
ret = _sys_mem_pool_block_alloc(&p->base, size, &level_num, /* There is a "managed race" in alloc that can fail
&block_num, &block->data); * (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.pool = pool_id(p);
block->id.level = level_num; block->id.level = level_num;
block->id.block = block_num; block->id.block = block_num;
if (ret == 0 || timeout == K_NO_WAIT || if (ret == 0 || timeout == K_NO_WAIT ||
ret == -EAGAIN || (ret && ret != -ENOMEM)) { (ret && ret != -ENOMEM)) {
return ret; return ret;
} }