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.

diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h
index 47fa34a5ddcc608b75b134e8d7ca2cffe621e3bd..460173c6fb6743726405e086757f466e9b457313 100644 (file)
--- a/include/haproxy/pool-t.h
+++ b/include/haproxy/pool-t.h
@@ -118,8 +118,9 @@ struct pool_head {
        unsigned int failed;    /* failed allocations */
        unsigned int alloc_sz;  /* allocated size (includes hidden fields) */
        struct list list;       /* list of all known pools */
+       void *base_addr;        /* allocation address, for free() */
        char name[12];          /* name of the pool */
-       struct pool_cache_head cache[MAX_THREADS]; /* pool caches */
+       struct pool_cache_head cache[MAX_THREADS] THREAD_ALIGNED(64); /* pool caches */
 } __attribute__((aligned(64)));
 
 #endif /* _HAPROXY_POOL_T_H */

diff --git a/src/pool.c b/src/pool.c
index 45c05fa454dd2fcf2bb8c4124665441428dc1f86..441133e7c7c7ebcd5e01270919de2c8018de0c43 100644 (file)
--- a/src/pool.c
+++ b/src/pool.c
@@ -275,11 +275,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->alloc_sz = size + extra;
@@ -816,7 +821,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;