From 21072b94806501c26a6e286ed4e46467afd557fe Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Fri, 29 May 2020 17:23:05 +0200
Subject: [PATCH] CLEANUP: pools: use the regular lock for the flush operation
 on lockless pools

Commit 04f5fe87d3d introduced an rwlock in the pools to deal with the risk
that pool_flush() dereferences an area being freed, and commit 899fb8abdcd
turned it into a spinlock. The pools already contain a spinlock in case of
locked pools, so let's use the same and simplify the code by removing ifdefs.

At this point I'm really suspecting that if pool_flush() would instead
rely on __pool_get_first() to pick entries from the pool, the concurrency
problem could never happen since only one user would get a given entry at
once, thus it could not be freed by another user. It's not certain this
would be faster however because of the number of atomic ops to retrieve
one entry compared to a locked batch.
---
 include/common/memory.h | 7 ++++---
 src/memory.c            | 8 ++------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/include/common/memory.h b/include/common/memory.h
index 84ec18e14..d3950ca5d 100644
--- a/include/common/memory.h
+++ b/include/common/memory.h
@@ -81,14 +81,15 @@ struct pool_free_list {
 };
 #endif
 
+/* Note below, in case of lockless pools, we still need the lock only for
+ * the flush() operation.
+ */
 struct pool_head {
 	void **free_list;
 #ifdef CONFIG_HAP_LOCKLESS_POOLS
 	uintptr_t seq;
-	HA_SPINLOCK_T flush_lock;
-#else
-	__decl_hathreads(HA_SPINLOCK_T lock); /* the spin lock */
 #endif
+	__decl_hathreads(HA_SPINLOCK_T lock); /* the spin lock */
 	unsigned int used;	/* how many chunks are currently in use */
 	unsigned int needed_avg;/* floating indicator between used and allocated */
 	unsigned int allocated;	/* how many chunks have been allocated */
diff --git a/src/memory.c b/src/memory.c
index 4552e4559..b442f0a10 100644
--- a/src/memory.c
+++ b/src/memory.c
@@ -139,11 +139,7 @@ struct pool_head *create_pool(char *name, unsigned int size, unsigned int flags)
 			for (thr = 0; thr < MAX_THREADS; thr++)
 				pool_cache[thr][idx].size = size;
 		}
-#ifndef CONFIG_HAP_LOCKLESS_POOLS
 		HA_SPIN_INIT(&pool->lock);
-#else
-		HA_SPIN_INIT(&pool->flush_lock);
-#endif
 	}
 	pool->users++;
 	return pool;
@@ -227,7 +223,7 @@ void pool_flush(struct pool_head *pool)
 
 	if (!pool)
 		return;
-	HA_SPIN_LOCK(POOL_LOCK, &pool->flush_lock);
+	HA_SPIN_LOCK(POOL_LOCK, &pool->lock);
 	do {
 		cmp.free_list = pool->free_list;
 		cmp.seq = pool->seq;
@@ -235,7 +231,7 @@ void pool_flush(struct pool_head *pool)
 		new.seq = cmp.seq + 1;
 	} while (!_HA_ATOMIC_DWCAS(&pool->free_list, &cmp, &new));
 	__ha_barrier_atomic_store();
-	HA_SPIN_UNLOCK(POOL_LOCK, &pool->flush_lock);
+	HA_SPIN_UNLOCK(POOL_LOCK, &pool->lock);
 	next = cmp.free_list;
 	while (next) {
 		temp = next;