diff options
Diffstat (limited to '0212-core-fix-memory-allocation-issues.patch')
-rw-r--r-- | 0212-core-fix-memory-allocation-issues.patch | 169 |
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 + |