]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: pools: always pre-initialize allocated memory outside of the lock
authorWilly Tarreau <w@1wt.eu>
Thu, 4 Jul 2019 09:48:16 +0000 (11:48 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 9 Jul 2019 08:40:33 +0000 (10:40 +0200)
When calling mmap(), in general the system gives us a page but does not
really allocate it until we first dereference it. And it turns out that
this time is much longer than the time to perform the mmap() syscall.
Unfortunately, when running with memory debugging enabled, we mmap/munmap()
each object resulting in lots of such calls and a high contention on the
allocator. And the first accesses to the page being done under the pool
lock is extremely damaging to other threads.

The simple fact of writing a 0 at the beginning of the page after
allocating it and placing the POOL_LINK pointer outside of the lock is
enough to boost the performance by 8x in debug mode and to save the
watchdog from triggering on lock contention. This is what this patch
does.

include/common/memory.h
src/memory.c

index 8fc7cc842aafd23c28cbf6265c9297054ee36e9d..5f96ac079b16fcaa6130a1471cb3f6d0ecc8d2db 100644 (file)
@@ -421,6 +421,10 @@ static inline void *pool_alloc_area(size_t size)
        ret = mmap(NULL, (size + 4095) & -4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
        if (ret == MAP_FAILED)
                return NULL;
+       /* let's dereference the page before returning so that the real
+        * allocation in the system is performed without holding the lock.
+        */
+       *(int *)ret = 0;
        if (pad >= sizeof(void *))
                *(void **)(ret + pad - sizeof(void *)) = ret + pad;
        return ret + pad;
index 8fed2f4e94d0b2d8c31878f4371c3044e524d014..a858a09834aeefa77c1285867551c00409321468 100644 (file)
@@ -337,6 +337,14 @@ void *__pool_refill_alloc(struct pool_head *pool, unsigned int avail)
 
                HA_SPIN_UNLOCK(POOL_LOCK, &pool->lock);
                ptr = pool_alloc_area(pool->size + POOL_EXTRA);
+#ifdef DEBUG_MEMORY_POOLS
+               /* keep track of where the element was allocated from. This
+                * is done out of the lock so that the system really allocates
+                * the data without harming other threads waiting on the lock.
+                */
+               if (ptr)
+                       *POOL_LINK(pool, ptr) = (void *)pool;
+#endif
                HA_SPIN_LOCK(POOL_LOCK, &pool->lock);
                if (!ptr) {
                        pool->failed++;
@@ -355,10 +363,6 @@ void *__pool_refill_alloc(struct pool_head *pool, unsigned int avail)
                pool->free_list = ptr;
        }
        pool->used++;
-#ifdef DEBUG_MEMORY_POOLS
-       /* keep track of where the element was allocated from */
-       *POOL_LINK(pool, ptr) = (void *)pool;
-#endif
        return ptr;
 }
 void *pool_refill_alloc(struct pool_head *pool, unsigned int avail)