[PATCH 0/2] smalloc fixes

[PATCH 0/2] smalloc fixes

[vincentfu]

From: Vincent Fu <vincent.fu@wdc.com>

Jens, please consider these patches that are a follow up to the smalloc
patch from last week.

I broke up last week's patch into two: one that implements your
suggested fix for the synchronization issue and a second that fixes the
SMALLOC_BPB/SMALLOC_BPI issue.

Vincent


Vincent Fu (2):
  smalloc: allocate struct pool array from shared memory
  smalloc: use SMALLOC_BPI instead of SMALLOC_BPB in add_pool()

 smalloc.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/2] smalloc: allocate struct pool array from shared memory

[vincentfu]

From: Vincent Fu <vincent.fu@wdc.com>

If one process is making smalloc calls and another process is making
sfree calls, pool->free_blocks and pool->next_non_full will not be
synchronized because the two processes each have independent, local
copies of the variables.

This patch allocates space for the array of struct pool instances from
shared storage so that separate processes will be modifying quantities
stored at the same locations.

This issue was discovered on the server side running a client/server job
with --status-interval=1. Such a job encountered an OOM error when only
~50 objects were allocated from the smalloc pool.
---
 smalloc.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/smalloc.c b/smalloc.c
index 125e07bf..39bf47a5 100644
--- a/smalloc.c
+++ b/smalloc.c
@@ -54,7 +54,7 @@ struct block_hdr {
  */
 static const bool enable_smalloc_debug = false;
 
-static struct pool mp[MAX_POOLS];
+static struct pool *mp;
 static unsigned int nr_pools;
 static unsigned int last_pool;
 
@@ -208,6 +208,20 @@ void sinit(void)
 	bool ret;
 	int i;
 
+	/*
+	 * sinit() can be called more than once if alloc-size is
+	 * set. But we want to allocate space for the struct pool
+	 * instances only once.
+	 */
+	if (!mp) {
+		mp = (struct pool *) mmap(NULL,
+			MAX_POOLS * sizeof(struct pool),
+			PROT_READ | PROT_WRITE,
+			OS_MAP_ANON | MAP_SHARED, -1, 0);
+
+		assert(mp != MAP_FAILED);
+	}
+
 	for (i = 0; i < INITIAL_POOLS; i++) {
 		ret = add_pool(&mp[nr_pools], smalloc_pool_size);
 		if (!ret)
@@ -239,6 +253,8 @@ void scleanup(void)
 
 	for (i = 0; i < nr_pools; i++)
 		cleanup_pool(&mp[i]);
+
+	munmap(mp, MAX_POOLS * sizeof(struct pool));
 }
 
 #ifdef SMALLOC_REDZONE
-- 
2.17.1

[PATCH 2/2] smalloc: use SMALLOC_BPI instead of SMALLOC_BPB in add_pool()

[vincentfu]

From: Vincent Fu <vincent.fu@wdc.com>

Change the calculation of free_blocks in add_pool() to use SMALLOC_BPI
instead of SMALLOC_BPB. These two constants are coincidentally the same
on Linux and Windows but SMALLOC_BPI is the correct one to use.
free_blocks is the number of available blocks of size SMALLOC_BPB. It is
the product of the number of unsigned integers in the bitmap
(bitmap_blocks) and the number of bits per unsigned integer
(SMALLOC_BPI).
---
 smalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/smalloc.c b/smalloc.c
index 39bf47a5..fa00f0ee 100644
--- a/smalloc.c
+++ b/smalloc.c
@@ -173,7 +173,7 @@ static bool add_pool(struct pool *pool, unsigned int alloc_size)
 	pool->mmap_size = alloc_size;
 
 	pool->nr_blocks = bitmap_blocks;
-	pool->free_blocks = bitmap_blocks * SMALLOC_BPB;
+	pool->free_blocks = bitmap_blocks * SMALLOC_BPI;
 
 	mmap_flags = OS_MAP_ANON;
 #ifdef CONFIG_ESX
-- 
2.17.1

Re: [PATCH 0/2] smalloc fixes

[Jens Axboe]

On 9/3/19 11:44 AM, vincentfu@gmail.com wrote:
> From: Vincent Fu <vincent.fu@wdc.com>
> 
> Jens, please consider these patches that are a follow up to the smalloc
> patch from last week.
> 
> I broke up last week's patch into two: one that implements your
> suggested fix for the synchronization issue and a second that fixes the
> SMALLOC_BPB/SMALLOC_BPI issue.

Thanks Vincent, applied.

-- 
Jens Axboe