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

- make core_log_threshold_mt a dedicated test
- keep Core_log_threshold available only via API
- add _core_log_get_threshold_internal() calls

Ref: #6035

Signed-off-by: Jan Michalski <jan.michalski@intel.com>

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
 };
@@ -163,6 +163,7 @@ core_log_set_threshold(enum core_log_threshold threshold,
  return EINVAL;
 
  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],
@@ -192,6 +193,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 +232,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)

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,

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__); \
  } \

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\

src/test/core_log_threshold_mt/.gitignore

@@ -0,0 +1 @@
+core_log_threshold_mt

src/test/core_log_mt/Makefile → src/test/core_log_threshold_mt/Makefile

@@ -1,8 +1,8 @@
 # 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

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')

src/test/core_log_threshold_mt/core_log_threshold_mt.c

@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/* Copyright 2024, Intel Corporation */
+
+/*
+ * core_log_threshold_mt.c -- unit test for core_log_set/get_threshold() and
+ * CORE_LOG_X() since all of them may write/read thresholds in parallel.
+ */
+
+#include "unittest.h"
+#include "log_internal.h"
+
+#define NO_ARGS_CONSUMED 0
+
+#define THREADS_IN_GROUP 10
+#define THREADS_SET_MIN 0
+#define THREADS_SET_MAX (THREADS_SET_MIN + THREADS_IN_GROUP)
+#define THREADS_GET_MIN THREADS_SET_MAX
+#define THREADS_GET_MAX (THREADS_GET_MIN + THREADS_IN_GROUP)
+#define TOTAL_THREADS THREADS_GET_MAX
+
+#define OP_REDO 4096
+
+struct helper_ctx {
+ enum core_log_threshold threshold;
+ enum core_log_level level;
+} helper_ctx_ [TOTAL_THREADS];
+
+os_mutex_t mutex;
+os_cond_t cond;
+unsigned threads_waiting;
+
+static void *
+helper_set(void *arg)
+{
+ struct helper_ctx *ctx = (struct helper_ctx *)arg;
+ os_mutex_lock(&mutex);
+ ++threads_waiting;
+ os_cond_wait(&cond, &mutex);
+ os_mutex_unlock(&mutex);
+ for (int i = 0; i < OP_REDO; ++i) {
+ int ret = core_log_set_threshold(ctx->threshold, ctx->level);
+ UT_ASSERT(ret == 0 || ret == EAGAIN);
+ }
+ return NULL;
+}
+
+static void *
+helper_get(void *arg)
+{
+ struct helper_ctx *ctx = (struct helper_ctx *)arg;
+ os_mutex_lock(&mutex);
+ ++threads_waiting;
+ os_cond_wait(&cond, &mutex);
+ os_mutex_unlock(&mutex);
+ for (int i = 0; i < OP_REDO; ++i) {
+ int ret = core_log_get_threshold(ctx->threshold, &ctx->level);
+ UT_ASSERTeq(ret, 0);
+ ctx->level = _core_log_get_threshold_internal();
+ }
+ return NULL;
+}
+
+static void
+helper(enum core_log_threshold threshold)
+{
+ os_thread_t threads[TOTAL_THREADS];
+
+ os_mutex_init(&mutex);
+ os_cond_init(&cond);
+ threads_waiting = 0;
+
+ /* core_log_set_threshold() threads */
+ for (int idx = THREADS_SET_MIN; idx < THREADS_SET_MAX; idx++) {
+ helper_ctx_[idx].threshold = threshold;
+ helper_ctx_[idx].level =
+ (enum core_log_level)(idx % CORE_LOG_LEVEL_MAX);
+ THREAD_CREATE(&threads[idx], 0, helper_set,
+ (void *)&helper_ctx_[idx]);
+ }
+
+ /* core_log_get_threshold/_core_log_get_threshold_internal() threads */
+ for (int idx = THREADS_GET_MIN; idx < THREADS_GET_MAX; idx++) {
+ helper_ctx_[idx].threshold = threshold;
+ THREAD_CREATE(&threads[idx], 0, helper_get,
+ (void *)&helper_ctx_[idx]);
+ }
+
+ do {
+ os_mutex_lock(&mutex);
+ if (threads_waiting == TOTAL_THREADS) {
+ os_cond_broadcast(&cond);
+ os_mutex_unlock(&mutex);
+ break;
+ }
+ os_mutex_unlock(&mutex);
+ } while (1);
+
+ for (int idx = 0; idx < TOTAL_THREADS; idx++) {
+ void *retval;
+ THREAD_JOIN(&threads[idx], &retval);
+ }
+
+ os_cond_destroy(&cond);
+ os_mutex_destroy(&mutex);
+}
+
+/* tests */
+
+/*
+ * Run core_log_set/get_threshold(CORE_LOG_THRESHOLD, ...) and CORE_LOG_X()
+ * in parallel.
+ */
+static int
+test_threshold_set_get(const struct test_case *tc, int argc, char *argv[])
+{
+ helper(CORE_LOG_THRESHOLD);
+ return NO_ARGS_CONSUMED;
+}
+
+/*
+ * Run core_log_set/get_threshold(CORE_LOG_THRESHOLD_AUX, ...) and CORE_LOG_X()
+ * in parallel.
+ */
+static int
+test_threshold_aux_set_get(const struct test_case *tc, int argc, char *argv[])
+{
+ helper(CORE_LOG_THRESHOLD_AUX);
+ return NO_ARGS_CONSUMED;
+}
+
+/*
+ * A Valgrind tool external to the test binary is assumed to monitor
+ * the execution and assess synchronisation correctness.
+ */
+static struct test_case test_cases[] = {
+ TEST_CASE(test_threshold_set_get),
+ TEST_CASE(test_threshold_aux_set_get),
+};
+
+#define NTESTS ARRAY_SIZE(test_cases)
+
+int
+main(int argc, char *argv[])
+{
+ START(argc, argv, "core_log_threshold_mt");
+ TEST_CASE_PROCESS(argc, argv, test_cases, NTESTS);
+ DONE(NULL);
+}