From 799f6143ca2a0a7ff8979ae5850b462363d76437 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 31 Dec 2021 16:00:19 +0100 Subject: [PATCH] CLEANUP: pools: do not use the extra pointer to link shared elements This practice relying on POOL_LINK() dates from the era where there were no pool caches, but given that the structures are a bit more complex now and that pool caches do not make use of this feature, it is totally useless since released elements have already been overwritten, and yet it complicates the architecture and prevents from making simplifications and optimizations. Let's just get rid of this feature. The pointer to the origin pool is preserved though, as it helps detect incorrect frees and serves as a canary for overflows. --- include/haproxy/pool-t.h | 1 - include/haproxy/pool.h | 4 ++-- src/pool.c | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h index 48d06be15..d476f31d4 100644 --- a/include/haproxy/pool-t.h +++ b/include/haproxy/pool-t.h @@ -40,7 +40,6 @@ #define POOL_LINK(pool, item) (void **)(((char *)(item)) + ((pool)->size)) #else #define POOL_EXTRA (0) -#define POOL_LINK(pool, item) ((void **)(item)) #endif /* A special pointer for the pool's free_list that indicates someone is diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h index cb2c8b4de..d8407b19e 100644 --- a/include/haproxy/pool.h +++ b/include/haproxy/pool.h @@ -132,7 +132,7 @@ static inline void *pool_get_from_shared_cache(struct pool_head *pool) } /* this releases the lock */ - _HA_ATOMIC_STORE(&pool->free_list, *POOL_LINK(pool, ret)); + _HA_ATOMIC_STORE(&pool->free_list, *(void **)ret); _HA_ATOMIC_INC(&pool->used); #ifdef DEBUG_MEMORY_POOLS @@ -164,7 +164,7 @@ static inline void pool_put_to_shared_cache(struct pool_head *pool, void *ptr) __ha_cpu_relax(); free_list = _HA_ATOMIC_LOAD(&pool->free_list); } - _HA_ATOMIC_STORE(POOL_LINK(pool, ptr), (void *)free_list); + _HA_ATOMIC_STORE((void **)ptr, (void *)free_list); __ha_barrier_atomic_store(); } while (!_HA_ATOMIC_CAS(&pool->free_list, &free_list, ptr)); __ha_barrier_atomic_store(); diff --git a/src/pool.c b/src/pool.c index e64bfcacf..7d83a953f 100644 --- a/src/pool.c +++ b/src/pool.c @@ -417,7 +417,7 @@ void pool_flush(struct pool_head *pool) while (next) { temp = next; - next = *POOL_LINK(pool, temp); + next = *(void **)temp; pool_put_to_os(pool, temp); } /* here, we should have pool->allocated == pool->used */ @@ -442,7 +442,7 @@ void pool_gc(struct pool_head *pool_ctx) while (entry->free_list && (int)(entry->allocated - entry->used) > (int)entry->minavail) { temp = entry->free_list; - entry->free_list = *POOL_LINK(entry, temp); + entry->free_list = *(void **)temp; pool_put_to_os(entry, temp); } }