You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
169 lines
6.7 KiB
169 lines
6.7 KiB
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 |
|
|
|
|