Skip to content

Commit 928cb23

Browse files
committed
fix atomic shared limits update in D. P.
1 parent d658fae commit 928cb23

File tree

4 files changed

+32
-12
lines changed

4 files changed

+32
-12
lines changed

cmake/helpers.cmake

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,10 @@ function(add_umf_target_compile_options name)
270270
/W4
271271
/Gy
272272
/GS
273-
# disable warning 6326: Potential comparison of a constant
273+
# disable 4324 warning: structure was padded due to
274+
# alignment specifier
275+
/wd4324
276+
# disable 6326 warning: Potential comparison of a constant
274277
# with another constant
275278
/wd6326
276279
# disable 4200 warning: nonstandard extension used:

src/pool/pool_disjoint.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -433,18 +433,19 @@ static bool bucket_can_pool(bucket_t *bucket) {
433433

434434
// we keep at most params.capacity slabs in the pool
435435
if (bucket_max_pooled_slabs(bucket) >= new_free_slabs_in_bucket) {
436-
size_t pool_size = 0;
436+
CACHE_ALIGNED size_t shared_size = 0;
437437
utils_atomic_load_acquire(&bucket->shared_limits->total_size,
438-
&pool_size);
438+
&shared_size);
439439
while (true) {
440-
size_t new_pool_size = pool_size + bucket_slab_alloc_size(bucket);
441-
442-
if (bucket->shared_limits->max_size < new_pool_size) {
440+
CACHE_ALIGNED size_t new_shared_size = 0;
441+
new_shared_size = shared_size + bucket_slab_alloc_size(bucket);
442+
if (bucket->shared_limits->max_size < new_shared_size) {
443443
break;
444444
}
445445

446+
// update the total shared size if needed
446447
if (utils_compare_exchange(&bucket->shared_limits->total_size,
447-
&pool_size, &new_pool_size)) {
448+
&shared_size, &new_shared_size)) {
448449
++bucket->chunked_slabs_in_pool;
449450

450451
bucket_update_stats(bucket, -1, 1);
@@ -895,7 +896,8 @@ umf_memory_pool_ops_t *umfDisjointPoolOps(void) {
895896

896897
umf_disjoint_pool_shared_limits_t *
897898
umfDisjointPoolSharedLimitsCreate(size_t max_size) {
898-
umf_disjoint_pool_shared_limits_t *ptr = umf_ba_global_alloc(sizeof(*ptr));
899+
umf_disjoint_pool_shared_limits_t *ptr =
900+
umf_ba_global_aligned_alloc(sizeof(*ptr), 64);
899901
if (ptr == NULL) {
900902
LOG_ERR("cannot allocate memory for disjoint pool shared limits");
901903
return NULL;

src/pool/pool_disjoint_internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ typedef struct slab_t {
101101
} slab_t;
102102

103103
typedef struct umf_disjoint_pool_shared_limits_t {
104+
CACHE_ALIGNED size_t total_size; // requires atomic access
104105
size_t max_size;
105-
size_t total_size; // requires atomic access
106106
} umf_disjoint_pool_shared_limits_t;
107107

108108
typedef struct umf_disjoint_pool_params_t {

src/utils/utils_concurrency.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
#endif /* _WIN32 */
3333

34+
#include "utils_common.h"
3435
#include "utils_sanitizers.h"
3536

3637
#ifdef __cplusplus
@@ -45,6 +46,12 @@ typedef struct utils_mutex_t {
4546
#endif
4647
} utils_mutex_t;
4748

49+
#ifdef _WIN32
50+
#define CACHE_ALIGNED __declspec(align(64))
51+
#else
52+
#define CACHE_ALIGNED __attribute__((aligned(64)))
53+
#endif
54+
4855
size_t utils_mutex_get_size(void);
4956
utils_mutex_t *utils_mutex_init(utils_mutex_t *ptr);
5057
void utils_mutex_destroy_not_free(utils_mutex_t *m);
@@ -110,9 +117,17 @@ static __inline unsigned char utils_mssb_index(long long value) {
110117
#define utils_fetch_and_add64(ptr, value) \
111118
InterlockedExchangeAdd64((LONG64 *)(ptr), value)
112119

113-
// NOTE: windows version have different order of args
114-
#define utils_compare_exchange(object, desired, expected) \
115-
InterlockedCompareExchange64((LONG64 volatile *)object, *expected, *desired)
120+
// NOTE: windows InterlockedCompareExchange64 have different order of args than
121+
// __atomic_compare_exchange. Also, the windows version returns the initial
122+
// value of "object", so to match the linux version we return true if the
123+
// exchange was successful or not needed
124+
#define utils_compare_exchange(object, expected, desired) \
125+
assert(IS_ALIGNED((uintptr_t)object, 64)), \
126+
assert(IS_ALIGNED((uintptr_t)expected, 64)), \
127+
assert(IS_ALIGNED((uintptr_t)desired, 64)), \
128+
InterlockedCompareExchange64((LONG64 volatile *)object, *desired, \
129+
*expected), \
130+
*(LONG64 volatile *)object == *(LONG64 volatile *)desired
116131

117132
#else // !defined(_WIN32)
118133

0 commit comments

Comments
 (0)