From 7ffffbd43e327b0a22ca23f7e8c28236b552ad93 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 2 Mar 2022 17:59:04 +0100 Subject: [PATCH] BUG/MINOR: pool: always align pool_heads to 64 bytes This is the pool equivalent of commit 97ea9c49f ("BUG/MEDIUM: fd: always align fdtab[] to 64 bytes"). After a careful code review, it happens that the pool heads are the other structures allocated with malloc/calloc that claim to be aligned to a size larger than what the allocator can offer. While no issue was reported on them, no memset() is performed and no type is large, this is a problem waiting to happen, so better fix it. In addition, it's relatively easy to do by storing the allocation address inside the pool_head itself and use it at free() time. Finally, threads might benefit from the fact that the caches will really be aligned and that there will be no false sharing. This should be backported to all versions where it applies easily. (cherry picked from commit e81248c0c8c543ab5065b41310e7c9685f61cc87) [wt: context adjustment] Signed-off-by: Willy Tarreau --- include/haproxy/pool-t.h | 3 ++- src/pool.c | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h index 33e6c82..fdbb564 100644 --- a/include/haproxy/pool-t.h +++ b/include/haproxy/pool-t.h @@ -84,9 +84,10 @@ struct pool_head { unsigned int failed; /* failed allocations */ /* 32-bit hole here */ struct list list; /* list of all known pools */ + void *base_addr; /* allocation address, for free() */ char name[12]; /* name of the pool */ #ifdef CONFIG_HAP_POOLS - struct pool_cache_head cache[MAX_THREADS]; /* pool caches */ + struct pool_cache_head cache[MAX_THREADS] THREAD_ALIGNED(64); /* pool caches */ #endif } __attribute__((aligned(64))); diff --git a/src/pool.c b/src/pool.c index 68dd8ea..6756421 100644 --- a/src/pool.c +++ b/src/pool.c @@ -175,11 +175,16 @@ struct pool_head *create_pool(char *name, unsigned int size, unsigned int flags) } if (!pool) { - if (!pool) - pool = calloc(1, sizeof(*pool)); + void *pool_addr; - if (!pool) + pool_addr = calloc(1, sizeof(*pool) + __alignof__(*pool)); + if (!pool_addr) return NULL; + + /* always provide an aligned pool */ + pool = (struct pool_head*)((((size_t)pool_addr) + __alignof__(*pool)) & -(size_t)__alignof__(*pool)); + pool->base_addr = pool_addr; // keep it, it's the address to free later + if (name) strlcpy2(pool->name, name, sizeof(pool->name)); pool->size = size; @@ -518,7 +523,7 @@ void *pool_destroy(struct pool_head *pool) if (!pool->users) { LIST_DELETE(&pool->list); /* note that if used == 0, the cache is empty */ - free(pool); + ha_free(&pool->base_addr); } } return NULL; -- 1.7.10.4