summaryrefslogtreecommitdiff
path: root/0212-core-fix-memory-allocation-issues.patch
diff options
context:
space:
mode:
Diffstat (limited to '0212-core-fix-memory-allocation-issues.patch')
-rw-r--r--0212-core-fix-memory-allocation-issues.patch169
1 files changed, 169 insertions, 0 deletions
diff --git a/0212-core-fix-memory-allocation-issues.patch b/0212-core-fix-memory-allocation-issues.patch
new file mode 100644
index 0000000..18da11d
--- /dev/null
+++ b/0212-core-fix-memory-allocation-issues.patch
@@ -0,0 +1,169 @@
+From 0bf728030e0ad7a49e6e1737ea06ae74da9279d3 Mon Sep 17 00:00:00 2001
+From: Xavi Hernandez <xhernandez@redhat.com>
+Date: Fri, 21 Jun 2019 11:28:08 +0200
+Subject: [PATCH 212/221] core: fix memory allocation issues
+
+Two problems have been identified that caused that gluster's memory
+usage were twice higher than required.
+
+1. An off by 1 error caused that all objects allocated from the memory
+ pools were taken from a pool bigger than required. Since each pool
+ corresponds to a size equal to a power of two, this was wasting half
+ of the available memory.
+
+2. The header information used for accounting on each memory object was
+ not taken into consideration when searching for a suitable memory
+ pool. It was added later when each individual block was allocated.
+ This made this space "invisible" to memory accounting.
+
+Credits: Thanks to Nithya Balachandran for identifying this problem and
+ testing this patch.
+
+Upstream patch:
+> BUG: 1722802
+> Upstream patch link: https://review.gluster.org/c/glusterfs/+/22921
+> Change-Id: I90e27ad795fe51ca11c13080f62207451f6c138c
+> Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
+
+Fixes: bz#1722801
+Change-Id: I90e27ad795fe51ca11c13080f62207451f6c138c
+Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
+Reviewed-on: https://code.engineering.redhat.com/gerrit/174714
+Tested-by: RHGS Build Bot <nigelb@redhat.com>
+Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
+---
+ libglusterfs/src/glusterfs/mem-pool.h | 5 ++-
+ libglusterfs/src/mem-pool.c | 57 +++++++++++++++++++----------------
+ 2 files changed, 35 insertions(+), 27 deletions(-)
+
+diff --git a/libglusterfs/src/glusterfs/mem-pool.h b/libglusterfs/src/glusterfs/mem-pool.h
+index c5a486b..be0a26d 100644
+--- a/libglusterfs/src/glusterfs/mem-pool.h
++++ b/libglusterfs/src/glusterfs/mem-pool.h
+@@ -231,7 +231,10 @@ typedef struct pooled_obj_hdr {
+ struct mem_pool *pool;
+ } pooled_obj_hdr_t;
+
+-#define AVAILABLE_SIZE(p2) (1 << (p2))
++/* Each memory block inside a pool has a fixed size that is a power of two.
++ * However each object will have a header that will reduce the available
++ * space. */
++#define AVAILABLE_SIZE(p2) ((1UL << (p2)) - sizeof(pooled_obj_hdr_t))
+
+ typedef struct per_thread_pool {
+ /* the pool that was used to request this allocation */
+diff --git a/libglusterfs/src/mem-pool.c b/libglusterfs/src/mem-pool.c
+index df167b6..d88041d 100644
+--- a/libglusterfs/src/mem-pool.c
++++ b/libglusterfs/src/mem-pool.c
+@@ -627,6 +627,7 @@ struct mem_pool *
+ mem_pool_new_fn(glusterfs_ctx_t *ctx, unsigned long sizeof_type,
+ unsigned long count, char *name)
+ {
++ unsigned long extra_size, size;
+ unsigned int power;
+ struct mem_pool *new = NULL;
+ struct mem_pool_shared *pool = NULL;
+@@ -637,10 +638,25 @@ mem_pool_new_fn(glusterfs_ctx_t *ctx, unsigned long sizeof_type,
+ return NULL;
+ }
+
+- /* We ensure sizeof_type > 1 and the next power of two will be, at least,
+- * 2^POOL_SMALLEST */
+- sizeof_type |= (1 << POOL_SMALLEST) - 1;
+- power = sizeof(sizeof_type) * 8 - __builtin_clzl(sizeof_type - 1) + 1;
++ /* This is the overhead we'll have because of memory accounting for each
++ * memory block. */
++ extra_size = sizeof(pooled_obj_hdr_t);
++
++ /* We need to compute the total space needed to hold the data type and
++ * the header. Given that the smallest block size we have in the pools
++ * is 2^POOL_SMALLEST, we need to take the MAX(size, 2^POOL_SMALLEST).
++ * However, since this value is only needed to compute its rounded
++ * logarithm in base 2, and this only depends on the highest bit set,
++ * we can simply do a bitwise or with the minimum size. We need to
++ * subtract 1 for correct handling of sizes that are exactly a power
++ * of 2. */
++ size = (sizeof_type + extra_size - 1UL) | ((1UL << POOL_SMALLEST) - 1UL);
++
++ /* We compute the logarithm in base 2 rounded up of the resulting size.
++ * This value will identify which pool we need to use from the pools of
++ * powers of 2. This is equivalent to finding the position of the highest
++ * bit set. */
++ power = sizeof(size) * 8 - __builtin_clzl(size);
+ if (power > POOL_LARGEST) {
+ gf_msg_callingfn("mem-pool", GF_LOG_ERROR, EINVAL, LG_MSG_INVALID_ARG,
+ "invalid argument");
+@@ -732,8 +748,8 @@ mem_get_pool_list(void)
+ return pool_list;
+ }
+
+-pooled_obj_hdr_t *
+-mem_get_from_pool(struct mem_pool *mem_pool, struct mem_pool_shared *pool)
++static pooled_obj_hdr_t *
++mem_get_from_pool(struct mem_pool *mem_pool)
+ {
+ per_thread_pool_list_t *pool_list;
+ per_thread_pool_t *pt_pool;
+@@ -747,12 +763,7 @@ mem_get_from_pool(struct mem_pool *mem_pool, struct mem_pool_shared *pool)
+ return NULL;
+ }
+
+- if (mem_pool) {
+- pt_pool = &pool_list
+- ->pools[mem_pool->pool->power_of_two - POOL_SMALLEST];
+- } else {
+- pt_pool = &pool_list->pools[pool->power_of_two - POOL_SMALLEST];
+- }
++ pt_pool = &pool_list->pools[mem_pool->pool->power_of_two - POOL_SMALLEST];
+
+ (void)pthread_spin_lock(&pool_list->lock);
+
+@@ -770,8 +781,7 @@ mem_get_from_pool(struct mem_pool *mem_pool, struct mem_pool_shared *pool)
+ } else {
+ (void)pthread_spin_unlock(&pool_list->lock);
+ GF_ATOMIC_INC(pt_pool->parent->allocs_stdc);
+- retval = malloc((1 << pt_pool->parent->power_of_two) +
+- sizeof(pooled_obj_hdr_t));
++ retval = malloc(1 << pt_pool->parent->power_of_two);
+ #ifdef DEBUG
+ hit = _gf_false;
+ #endif
+@@ -779,19 +789,14 @@ mem_get_from_pool(struct mem_pool *mem_pool, struct mem_pool_shared *pool)
+ }
+
+ if (retval != NULL) {
+- if (mem_pool) {
+- retval->pool = mem_pool;
+- retval->power_of_two = mem_pool->pool->power_of_two;
++ retval->pool = mem_pool;
++ retval->power_of_two = mem_pool->pool->power_of_two;
+ #ifdef DEBUG
+- if (hit == _gf_true)
+- GF_ATOMIC_INC(mem_pool->hit);
+- else
+- GF_ATOMIC_INC(mem_pool->miss);
++ if (hit == _gf_true)
++ GF_ATOMIC_INC(mem_pool->hit);
++ else
++ GF_ATOMIC_INC(mem_pool->miss);
+ #endif
+- } else {
+- retval->power_of_two = pool->power_of_two;
+- retval->pool = NULL;
+- }
+ retval->magic = GF_MEM_HEADER_MAGIC;
+ retval->pool_list = pool_list;
+ }
+@@ -811,7 +816,7 @@ mem_get(struct mem_pool *mem_pool)
+ #if defined(GF_DISABLE_MEMPOOL)
+ return GF_MALLOC(mem_pool->sizeof_type, gf_common_mt_mem_pool);
+ #else
+- pooled_obj_hdr_t *retval = mem_get_from_pool(mem_pool, NULL);
++ pooled_obj_hdr_t *retval = mem_get_from_pool(mem_pool);
+ if (!retval) {
+ return NULL;
+ }
+--
+1.8.3.1
+