core/logging: Fix data race on log_prefix

In ofi_get_core_info, which is supposed to be thread safe ("Multiple threads may call fi_getinfo simultaneously, without any requirement for serialization."), a global variable 'log_prefix' is modified, which may lead to a data race. Changing the variable to a thread local one, fixes that problem. Signed-off-by: Dariusz Sciebura <[email protected]>
ofiwg · Jan 13, 2025 · 116a868 · 116a868
1 parent 2a320f1
commit 116a868
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 2 deletions.
diff --git a/include/ofi.h b/include/ofi.h
@@ -320,7 +320,7 @@ struct ofi_filter {
 
 extern struct ofi_filter prov_log_filter;
 extern struct fi_provider core_prov;
-extern const char *log_prefix;
+extern OFI_THREAD_LOCAL const char *log_prefix;
 
 void ofi_create_filter(struct ofi_filter *filter, const char *env_name);
 void ofi_free_filter(struct ofi_filter *filter);

diff --git a/include/unix/osd.h b/include/unix/osd.h
@@ -83,6 +83,22 @@
 
 #define OFI_MAX_SOCKET_BUF_SIZE	SIZE_MAX
 
+#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L)
+    // C23 and above: use thread_local directly
+    #define OFI_THREAD_LOCAL thread_local
+#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201102L) && defined(_Thread_local)
+    // C11: use _Thread_local
+    #define OFI_THREAD_LOCAL _Thread_local
+#elif defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__SUNPRO_CC) || defined(__IBMCPP__) || defined(__clang__)
+    // GCC/Clang/Intel/SunPro/IBM compilers
+    #define OFI_THREAD_LOCAL __thread
+#else
+    // Unsupported compiler
+    #warning "Thread-local storage is not supported on this platform"
+	#define OFI_THREAD_LOCAL
+#endif
+
+
 struct util_shm
 {
 	int		shared_fd;

diff --git a/include/windows/osd.h b/include/windows/osd.h
@@ -1141,6 +1141,23 @@ ofi_cpuid(unsigned func, unsigned subfunc, unsigned cpuinfo[4])
 
 #endif /* defined(_M_X64) || defined(_M_AMD64) */
 
+#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L)
+    // C23 and above: use thread_local directly
+    #define OFI_THREAD_LOCAL thread_local
+#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201102L) && defined(_Thread_local)
+    // C11: use _Thread_local
+    #define OFI_THREAD_LOCAL _Thread_local
+#elif defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__SUNPRO_CC) || defined(__IBMCPP__) || defined(__clang__)
+    // GCC/Clang/Intel/SunPro/IBM compilers
+    #define OFI_THREAD_LOCAL __thread
+#elif defined(_MSC_VER)
+    // Microsoft Visual C++ compiler
+    #define OFI_THREAD_LOCAL __declspec(thread)
+#else
+    // Unsupported compiler
+    #warning "Thread-local storage is not supported on this platform"
+	#define OFI_THREAD_LOCAL
+#endif
 
 #ifdef __cplusplus
 }

diff --git a/src/common.c b/src/common.c
@@ -2429,4 +2429,4 @@ size_t ofi_vrb_speed(uint8_t speed, uint8_t width)
 }
 
 /* log_prefix is used by fi_log and by prov/util */
-const char *log_prefix = "";
+OFI_THREAD_LOCAL const char *log_prefix = "";