mempool: fix race in get_pool()

Fixes a race in get_pool() which could leave the pool with zero refences
but still owned by the last thread using the pool.

Some performance number on Hikey with default configuration:
github/master (edbb89f, before this commit):

4006  real      1m 41.11s
4007  real      1m 14.51s
4008  real      0m 0.13s
4009  real      1m 5.68s

Revert "mempool: optimize reference counting", before this commit:
4006  real      3m 27.78s
4007  real      0m 50.03s
4008  real      0m 0.13s
4009  real      2m 24.07s

With this commit, two runs:
4006  real      1m 37.51s
4007  real      0m 56.67s
4008  real      0m 0.09s
4009  real      1m 3.18s

4006  real      1m 37.61s
4007  real      0m 35.32s
4008  real      0m 0.13s
4009  real      1m 3.15s

Numbers are gathered with this script:
for a in 4006 4007 4008 4009 ; do \
echo -n $a " " >> time.txt ;\
time -o time.txt.tmp xtest -l 15 $a || break ;\
grep real time.txt.tmp >> time.txt
done
cat time.txt

Reviewed-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

1 files changed, 16 insertions, 7 deletions

diff --git a/lib/libutils/ext/mempool.c b/lib/libutils/ext/mempool.c
index 8419e82b..f9776995 100644
--- a/lib/libutils/ext/mempool.c
+++ b/lib/libutils/ext/mempool.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: BSD-2-Clause
 /*
  * Copyright (c) 2014, STMicroelectronics International N.V.
- * Copyright (c) 2018, Linaro Limited
+ * Copyright (c) 2018-2019, Linaro Limited
  */
 
 
@@ -74,10 +74,15 @@ struct mempool {
 static void get_pool(struct mempool *pool __maybe_unused)
 {
 #if defined(__KERNEL__)
-	if (refcount_inc(&pool->refc)) {
-		if (pool->owner == thread_get_id())
-			return;
-		refcount_dec(&pool->refc);
+	/*
+	 * Owner matches our thread it cannot be changed. If it doesn't
+	 * match it can change any at time we're not holding the mutex to
+	 * any value but our thread id.
+	 */
+	if (atomic_load_int(&pool->owner) == thread_get_id()) {
+		if (!refcount_inc(&pool->refc))
+			panic();
+		return;
 	}
 
 	mutex_lock(&pool->mu);
@@ -96,12 +101,16 @@ static void get_pool(struct mempool *pool __maybe_unused)
 static void put_pool(struct mempool *pool __maybe_unused)
 {
 #if defined(__KERNEL__)
-	assert(pool->owner == thread_get_id());
+	assert(atomic_load_int(&pool->owner) == thread_get_id());
 
 	if (refcount_dec(&pool->refc)) {
 		mutex_lock(&pool->mu);
 
-		pool->owner = THREAD_ID_INVALID;
+		/*
+		 * Do an atomic store to match the atomic load in
+		 * get_pool() above.
+		 */
+		atomic_store_int(&pool->owner, THREAD_ID_INVALID);
 		condvar_signal(&pool->cv);
 
 		/* As the refcount is 0 there should be no items left */