common: core_log_set/get_threshold MT unit tests (part 2)

- make core_log_set_threshold() robust to EAGAIN - make core_log_threshold_mt a dedicated test - add threads attempting to call CORE_LOG_X() in parallel Ref: #6035 Signed-off-by: Jan Michalski <jan.michalski@intel.com>
pmem · Mar 7, 2024 · d95effd · d95effd
1 parent dde2f7e
commit d95effd
Show file tree

Hide file tree

Showing 11 changed files with 203 additions and 126 deletions.
diff --git a/src/core/log.c b/src/core/log.c
@@ -57,7 +57,7 @@ _Atomic
 void *Core_log_function_context;
 
 /* threshold levels */
-enum core_log_level Core_log_threshold[] = {
+static enum core_log_level Core_log_threshold[] = {
 		CORE_LOG_THRESHOLD_DEFAULT,
 		CORE_LOG_THRESHOLD_AUX_DEFAULT
 };
@@ -148,6 +148,22 @@ core_log_set_function(core_log_function *log_function, void *context)
 #endif /* ATOMIC_OPERATIONS_SUPPORTED */
 }
 
+static inline int
+core_log_set_threshold_attempt(enum core_log_threshold threshold,
+	enum core_log_level level)
+{
+	enum core_log_level level_old;
+	/* fed with already validated arguments it can't fail */
+	(void) core_log_get_threshold(threshold, &level_old);
+
+	if (!__sync_bool_compare_and_swap(&Core_log_threshold[threshold],
+			level_old, level)) {
+		return EAGAIN;
+	}
+
+	return 0;
+}
+
 /*
  * core_log_set_threshold -- set the log level threshold
  */
@@ -162,13 +178,8 @@ core_log_set_threshold(enum core_log_threshold threshold,
 	if (level < CORE_LOG_LEVEL_HARK || level > CORE_LOG_LEVEL_DEBUG)
 		return EINVAL;
 
-	enum core_log_level level_old;
-	(void) core_log_get_threshold(threshold, &level_old);
-
-	if (!__sync_bool_compare_and_swap(&Core_log_threshold[threshold],
-			level_old, level)) {
-		return EAGAIN;
-	}
+	while (EAGAIN == core_log_set_threshold_attempt(threshold, level))
+		;
 
 	return 0;
 }
@@ -192,6 +203,17 @@ core_log_get_threshold(enum core_log_threshold threshold,
 	return 0;
 }
 
+/*
+ * _core_log_get_threshold_internal -- a core_log_get_threshold variant
+ * optimized for performance and not affecting the stack size of all
+ * the functions using the CORE_LOG_* macros.
+ */
+volatile enum core_log_level
+_core_log_get_threshold_internal()
+{
+	return Core_log_threshold[CORE_LOG_THRESHOLD];
+}
+
 static void inline
 core_log_va(char *buf, size_t buf_len, enum core_log_level level,
 	int errnum, const char *file_name, int line_no,
@@ -220,7 +242,7 @@ core_log_va(char *buf, size_t buf_len, enum core_log_level level,
 	 * the CORE_LOG() macro it has to be done here again since it is not
 	 * performed in the case of the CORE_LOG_TO_LAST macro. Sorry.
 	 */
-	if (level > Core_log_threshold[CORE_LOG_THRESHOLD])
+	if (level > _core_log_get_threshold_internal())
 		goto end;
 
 	if (0 == Core_log_function)

diff --git a/src/core/log_default.c b/src/core/log_default.c
@@ -107,6 +107,7 @@ core_log_default_function(void *context, enum core_log_level level,
 	char file_info_buffer[256] = "";
 	const char *file_info = file_info_buffer;
 	const char file_info_error[] = "[file info error]: ";
+	enum core_log_level threshold_aux;
 
 	if (file_name) {
 		/* extract base_file_name */
@@ -139,7 +140,8 @@ core_log_default_function(void *context, enum core_log_level level,
 	}
 
 	/* secondary logging destination (CORE_LOG_THRESHOLD_AUX) */
-	if (level <= Core_log_threshold[CORE_LOG_THRESHOLD_AUX]) {
+	(void) core_log_get_threshold(CORE_LOG_THRESHOLD_AUX, &threshold_aux);
+	if (level <= threshold_aux) {
 		char times_tamp[45] = "";
 		get_timestamp_prefix(times_tamp, sizeof(times_tamp));
 		(void) fprintf(stderr, "%s[%ld] %s%s%s\n", times_tamp,

diff --git a/src/core/log_internal.h b/src/core/log_internal.h
@@ -62,6 +62,8 @@ int core_log_set_threshold(enum core_log_threshold threshold,
 int core_log_get_threshold(enum core_log_threshold threshold,
 	enum core_log_level *level);
 
+volatile enum core_log_level _core_log_get_threshold_internal(void);
+
 /*
  * the type used for defining logging functions
  */
@@ -83,12 +85,12 @@ typedef void core_log_function(
 
 int core_log_set_function(core_log_function *log_function, void *context);
 
-/* threshold levels */
+/* pointer to the logging function */
 extern
 #ifdef ATOMIC_OPERATIONS_SUPPORTED
 _Atomic
 #endif /* ATOMIC_OPERATIONS_SUPPORTED */
-enum core_log_level Core_log_threshold[CORE_LOG_THRESHOLD_MAX];
+uintptr_t Core_log_function;
 
 void core_log_init(void);
 
@@ -121,7 +123,8 @@ void core_log(enum core_log_level level, int errnum, const char *file_name,
 
 #define _CORE_LOG(level, errnum, format, ...) \
 	do { \
-		if (level <= Core_log_threshold[CORE_LOG_THRESHOLD]) { \
+		if (level <= \
+				_core_log_get_threshold_internal()) { \
 			core_log(level, errnum, __FILE__, __LINE__, \
 			__func__, format, ##__VA_ARGS__); \
 		} \

diff --git a/src/include/libpmemobj/log.h b/src/include/libpmemobj/log.h
@@ -114,7 +114,6 @@ enum pmemobj_log_threshold {
  * - EINVAL - threshold is not PMEMOBJ_LOG_THRESHOLD nor
  *   PMEMOBJ_LOG_THRESHOLD_AUX
  * - EINVAL - level is not a value defined by enum log_level type
- * - EAGAIN - a temporary error occurred, the retry may fix the problem
  *
  * SEE ALSO
  * pmemobj_log_get_threshold(3), pmemobj_log_set_function(3).

diff --git a/src/test/Makefile b/src/test/Makefile
@@ -111,8 +111,8 @@ OTHER_TESTS = \
 	core_log_default_function\
 	core_log_internal\
 	core_log_max\
-	core_log_mt\
 	core_log_no_func\
+	core_log_threshold_mt\
 	checksum\
 	compat_incompat_features\
 	ctl_prefault\

diff --git a/src/test/core_log_mt/.gitignore b/src/test/core_log_mt/.gitignore
diff --git a/src/test/core_log_mt/core_log_mt.c b/src/test/core_log_mt/core_log_mt.c
diff --git a/src/test/core_log_threshold_mt/.gitignore b/src/test/core_log_threshold_mt/.gitignore
@@ -0,0 +1 @@
+core_log_threshold_mt
diff --git a/src/test/core_log_mt/Makefile → src/test/core_log_threshold_mt/Makefile b/src/test/core_log_mt/Makefile → src/test/core_log_threshold_mt/Makefile
@@ -1,12 +1,13 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2024, Intel Corporation
 
-TARGET = core_log_mt
-OBJS = core_log_mt.o
+TARGET = core_log_threshold_mt
+OBJS = core_log_threshold_mt.o
 
 BUILD_STATIC_DEBUG=n
 BUILD_STATIC_NONDEBUG=n
 
 LIBPMEMCORE=nondebug
 
 include ../Makefile.inc
+LDFLAGS += $(call extract_funcs, core_log_threshold_mt.c)
diff --git a/src/test/core_log_mt/TESTS.py → src/test/core_log_threshold_mt/TESTS.py b/src/test/core_log_mt/TESTS.py → src/test/core_log_threshold_mt/TESTS.py
@@ -14,15 +14,15 @@ class CORE_LOG_MT(t.BaseTest):
     test_type = t.Short
 
     def run(self, ctx):
-        ctx.exec('core_log_mt', self.test_case)
+        ctx.exec('core_log_threshold_mt', self.test_case)
 
 
 class THRESHOLD(CORE_LOG_MT):
-    test_case = 'test_threshold'
+    test_case = 'test_threshold_set_get'
 
 
 class THRESHOLD_AUX(CORE_LOG_MT):
-    test_case = 'test_threshold_aux'
+    test_case = 'test_threshold_aux_set_get'
 
 
 @t.require_valgrind_enabled('helgrind')