From 3e157c8b960cbf92db7047bb5814c243fda4ef6b Mon Sep 17 00:00:00 2001
From: Piotr Balcer <piotr.balcer@intel.com>
Date: Thu, 25 Jul 2019 17:25:24 +0200
Subject: [PATCH] obj: fix recycler not locating unused chunks

In scenarios where free() is done very rarely the memory in the recycler
would not be looked at because the estimates would indicate that no memory
was freed and there's no reason to search the tree. Normally that's OK,
because eventually the heap would trigger a forced search of the entire
recycler for the allocation class, locating all the memory that might have
been missed. But, the forced recalculation erronously looked at the
estimates and required that at least some memory were freed since the
last recycler run - which might have not been the case.
This patch makes the forced recycling run without looking at the estimates.

Reported-by: Denny Zhao <denny.zhao@memverge.com>
---
 src/libpmemobj/recycler.c      |  4 +--
 src/test/obj_zones/TEST1       | 54 ++++++++++++++++++++++++++++++++++
 src/test/obj_zones/obj_zones.c | 44 ++++++++++++++++++++++++++-
 3 files changed, 99 insertions(+), 3 deletions(-)
 create mode 100755 src/test/obj_zones/TEST1

diff --git a/src/libpmemobj/recycler.c b/src/libpmemobj/recycler.c
index 73fa1f72c..73e2e37bc 100644
--- a/src/libpmemobj/recycler.c
+++ b/src/libpmemobj/recycler.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2018, Intel Corporation
+ * Copyright 2016-2019, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -285,7 +285,7 @@ recycler_recalc(struct recycler *r, int force)
 
 	uint64_t units = r->unaccounted_total;
 
-	if (units == 0 || (!force && units < (r->recalc_threshold)))
+	if (!force && units < r->recalc_threshold)
 		return runs;
 
 	if (util_mutex_trylock(&r->lock) != 0)
diff --git a/src/test/obj_zones/TEST1 b/src/test/obj_zones/TEST1
new file mode 100755
index 000000000..0d7b76414
--- /dev/null
+++ b/src/test/obj_zones/TEST1
@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+#
+# Copyright 2019, Intel Corporation
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#
+#     * Neither the name of the copyright holder nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+
+
+. ../unittest/unittest.sh
+
+# too large
+configure_valgrind force-disable
+
+require_test_type medium
+
+# runs too long on debug builds
+require_build_type nondebug
+
+setup
+
+create_holey_file 64G $DIR/testfile1
+
+PMEM_IS_PMEM_FORCE=1 PMEM_NO_FLUSH=1 expect_normal_exit\
+	./obj_zones$EXESUFFIX $DIR/testfile1 f
+
+check
+
+pass
diff --git a/src/test/obj_zones/obj_zones.c b/src/test/obj_zones/obj_zones.c
index 715b48740..c1d84268a 100644
--- a/src/test/obj_zones/obj_zones.c
+++ b/src/test/obj_zones/obj_zones.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2018, Intel Corporation
+ * Copyright 2015-2019, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -92,6 +92,46 @@ test_open(const char *path)
 	pmemobj_close(pop);
 }
 
+/*
+ * test_malloc_free -- test if alloc until OOM/free/alloc until OOM sequence
+ *	produces the same number of allocations for the second alloc loop.
+ */
+static void
+test_malloc_free(const char *path)
+{
+	PMEMobjpool *pop = NULL;
+	if ((pop = pmemobj_create(path, LAYOUT_NAME,
+			0, S_IWUSR | S_IRUSR)) == NULL)
+		UT_FATAL("!pmemobj_create: %s", path);
+
+	size_t alloc_size = 128 * 1024;
+	size_t max_allocs = 1000000;
+	PMEMoid *oid = MALLOC(sizeof(PMEMoid) * max_allocs);
+	size_t n = 0;
+	while (1) {
+		if (pmemobj_alloc(pop, &oid[n], alloc_size, 0, NULL, NULL) != 0)
+			break;
+		n++;
+		UT_ASSERTne(n, max_allocs);
+	}
+	size_t first_run_allocated = n;
+
+	for (size_t i = 0; i < n; ++i) {
+		pmemobj_free(&oid[i]);
+	}
+
+	n = 0;
+	while (1) {
+		if (pmemobj_alloc(pop, &oid[n], alloc_size, 0, NULL, NULL) != 0)
+			break;
+		n++;
+	}
+	UT_ASSERTeq(first_run_allocated, n);
+
+	pmemobj_close(pop);
+	FREE(oid);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -106,6 +146,8 @@ main(int argc, char *argv[])
 		test_create(path);
 	else if (op == 'o')
 		test_open(path);
+	else if (op == 'f')
+		test_malloc_free(path);
 	else
 		UT_FATAL("invalid operation");
 
-- 
GitLab