From f9dc048d2f50902ae3cbe41b50c73635c3200ec0 Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Tue, 19 Nov 2024 13:41:07 -0600 Subject: [PATCH] Use H5_ATOMIC macros in H5VL --- src/H5VLdyn_ops.c | 47 ++++++++++++++++++++++++++++++++++++++++------ src/H5VLint.c | 6 +----- src/H5VLpassthru.c | 14 +++++--------- src/H5VLpkg.h | 5 +++++ src/H5VLprivate.h | 12 ++---------- 5 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/H5VLdyn_ops.c b/src/H5VLdyn_ops.c index 0887fad9045..f7e24be1e1b 100644 --- a/src/H5VLdyn_ops.c +++ b/src/H5VLdyn_ops.c @@ -193,8 +193,11 @@ H5VL__term_opt_operation(void) herr_t H5VL__register_opt_operation(H5VL_subclass_t subcls, const char *op_name, int *op_val) { - H5VL_dyn_op_t *new_op; /* Info about new operation */ - herr_t ret_value = SUCCEED; /* Return value */ + H5VL_dyn_op_t *new_op = NULL; /* Info about new operation */ + herr_t ret_value = SUCCEED; /* Return value */ + H5SL_t *new_op_list = NULL; /* Newly created skip list, if any */ + H5SL_t *op_list = NULL; /* Existing skip list */ + bool new_list_inserted = false; /* Whether a new skip list was inserted into global array */ FUNC_ENTER_PACKAGE H5_API_LOCK @@ -204,14 +207,41 @@ H5VL__register_opt_operation(H5VL_subclass_t subcls, const char *op_name, int *o assert(op_name && *op_name); /* Check for duplicate operation */ - if (H5VL_opt_ops_g[subcls]) { +#ifdef H5_HAVE_MULTITHREAD + if ((op_list = atomic_load(&H5VL_opt_ops_g[subcls])) != NULL) { + if (NULL != H5SL_search(op_list, op_name)) + HGOTO_ERROR(H5E_VOL, H5E_EXISTS, FAIL, "operation name already exists"); + } /* end if */ +#else + if ((op_list = H5VL_opt_ops_g[subcls]) != NULL) { if (NULL != H5SL_search(H5VL_opt_ops_g[subcls], op_name)) HGOTO_ERROR(H5E_VOL, H5E_EXISTS, FAIL, "operation name already exists"); } /* end if */ +#endif + + /* List does not exist: Create skip list for operations of this subclass */ else { - /* Create skip list for operation of this subclass */ - if (NULL == (H5VL_opt_ops_g[subcls] = H5SL_create(H5SL_TYPE_STR, NULL))) + if ((new_op_list = H5SL_create(H5SL_TYPE_STR, NULL)) == NULL) HGOTO_ERROR(H5E_VOL, H5E_CANTCREATE, FAIL, "can't create skip list for operations"); + +#ifdef H5_HAVE_MULTITHREAD + H5SL_t *expected = NULL; + + /* If another thread has concurrently set up this op list, abort and use that instead */ + if ((new_list_inserted = + atomic_compare_exchange_strong(&H5VL_opt_ops_g[subcls], &expected, new_op_list))) { + op_list = new_op_list; + } + else { + /* Another thread initialized the list */ + if ((op_list = atomic_load(&H5VL_opt_ops_g[subcls])) == NULL) + HGOTO_ERROR(H5E_VOL, H5E_CANTGET, FAIL, "can't get skip list for operations"); + } +#else + H5VL_opt_ops_g[subcls] = new_op_list; + op_list = new_op_list; + new_list_inserted = true; +#endif } /* end else */ /* Register new operation */ @@ -219,10 +249,15 @@ H5VL__register_opt_operation(H5VL_subclass_t subcls, const char *op_name, int *o HGOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL, "can't allocate memory for dynamic operation info"); if (NULL == (new_op->op_name = H5MM_strdup(op_name))) HGOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL, "can't allocate name for dynamic operation info"); + +#ifdef H5_HAVE_MULTITHREAD + new_op->op_val = atomic_fetch_add(&H5VL_opt_vals_g[subcls], 1); +#else new_op->op_val = H5VL_opt_vals_g[subcls]++; +#endif /* Insert into subclass's skip list */ - if (H5SL_insert(H5VL_opt_ops_g[subcls], new_op, new_op->op_name) < 0) + if (H5SL_insert(op_list, new_op, new_op->op_name) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTINSERT, FAIL, "can't insert operation info into skip list"); /* Return the next operation value to the caller */ diff --git a/src/H5VLint.c b/src/H5VLint.c index 4cd1fdbe8cd..4ef2dc84423 100644 --- a/src/H5VLint.c +++ b/src/H5VLint.c @@ -57,11 +57,7 @@ /* Object wrapping context info */ typedef struct H5VL_wrap_ctx_t { -#ifdef H5_HAVE_MULTITHREAD - _Atomic unsigned rc; -#else - unsigned rc; /* Ref. count for the # of times the context was set / reset */ -#endif + H5_ATOMIC(unsigned) rc; /* Ref. count for the # of times the context was set / reset */ H5VL_t *connector; /* VOL connector for "outermost" class to start wrap */ void *obj_wrap_ctx; /* "wrap context" for outermost connector */ } H5VL_wrap_ctx_t; diff --git a/src/H5VLpassthru.c b/src/H5VLpassthru.c index 0a032a647aa..e2a92c88104 100644 --- a/src/H5VLpassthru.c +++ b/src/H5VLpassthru.c @@ -35,12 +35,13 @@ #include #include -/* TODO: H5_HAVE_MULTITHREAD seems not to be exposed to this module, always import */ -#include - /* Public HDF5 file */ #include "hdf5.h" +#ifdef H5_HAVE_MULTITHREAD +#include +#endif + /* This connector's header */ #include "H5VLpassthru.h" @@ -368,12 +369,7 @@ static const H5VL_class_t H5VL_pass_through_g = { }; /* The connector identification number, initialized at runtime */ -#ifdef H5_HAVE_MULTITHREAD -static _Atomic(hid_t) H5VL_PASSTHRU_ID_g = ATOMIC_VAR_INIT(H5I_INVALID_HID); -#else -static hid_t H5VL_PASSTHRU_ID_g = H5I_INVALID_HID; -#endif - +static H5_ATOMIC_SPECIFIER(hid_t) H5VL_PASSTHRU_ID_g = H5_ATOMIC_VAR_INIT(H5I_INVALID_HID); /*------------------------------------------------------------------------- * Function: H5VL__pass_through_new_obj * diff --git a/src/H5VLpkg.h b/src/H5VLpkg.h index 5a4ccc1482c..d1cb1323c28 100644 --- a/src/H5VLpkg.h +++ b/src/H5VLpkg.h @@ -26,6 +26,10 @@ /* Get package's private header */ #include "H5VLprivate.h" /* Generic Functions */ +#ifdef H5_HAVE_MULTITHREAD +#include +#endif + /* Other private headers needed by this file */ /**************************/ @@ -63,6 +67,7 @@ H5_DLL size_t H5VL__num_opt_operation(void); H5_DLL herr_t H5VL__find_opt_operation(H5VL_subclass_t subcls, const char *op_name, int *op_val); H5_DLL herr_t H5VL__unregister_opt_operation(H5VL_subclass_t subcls, const char *op_name); H5_DLL herr_t H5VL__term_opt_operation(void); +H5_DLL void H5VL__init_opt_operation_table(void); /* Testing functions */ #ifdef H5VL_TESTING diff --git a/src/H5VLprivate.h b/src/H5VLprivate.h index e5ab7aacfd7..56cee886409 100644 --- a/src/H5VLprivate.h +++ b/src/H5VLprivate.h @@ -34,11 +34,7 @@ /* Internal struct to track VOL connector information for objects */ typedef struct H5VL_t { const H5VL_class_t *cls; /* Pointer to connector class struct */ -#ifdef H5_HAVE_MULTITHREAD - _Atomic int64_t nrefs; /* Number of references by objects using this struct */ -#else - int64_t nrefs; /* Number of references by objects using this struct */ -#endif + H5_ATOMIC(int64_t) nrefs; /* Number of references by objects using this struct */ hid_t id; /* Identifier for the VOL connector */ } H5VL_t; @@ -46,11 +42,7 @@ typedef struct H5VL_t { typedef struct H5VL_object_t { void *data; /* Pointer to connector-managed data for this object */ H5VL_t *connector; /* Pointer to VOL connector struct */ -#ifdef H5_HAVE_MULTITHREAD - _Atomic size_t rc; /* Reference count */ -#else - size_t rc; /* Reference count */ -#endif + H5_ATOMIC(size_t) rc; /* Reference count */ } H5VL_object_t;