Skip to content

Make data race in tracking provider far less probable #1275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Make data race in tracking provider far less probable #1275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/critnib/critnib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ void *critnib_remove(struct critnib *c, word key) {

del_leaf:
value = k->value;
utils_atomic_store_release_ptr(&k->value, NULL);
c->pending_del_leaves[del] = k;

not_found:
Expand Down Expand Up @@ -635,7 +636,10 @@ void *critnib_find_le(struct critnib *c, word key) {
struct critnib_node *n; /* avoid a subtle TOCTOU */
utils_atomic_load_acquire_ptr((void **)&c->root, (void **)&n);
struct critnib_leaf *k = n ? find_le(n, key) : NULL;
res = k ? k->value : NULL;
res = NULL;
if (k) {
utils_atomic_load_acquire_ptr(&k->value, &res);
}
utils_atomic_load_acquire_u64(&c->remove_count, &wrs2);
} while (wrs1 + DELETED_LIFE <= wrs2);

Expand Down
52 changes: 49 additions & 3 deletions src/provider/provider_tracking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,22 @@ static tracker_alloc_info_t *get_most_nested_alloc_segment(
found =
critnib_find(hTracker->alloc_segments_map[level], (uintptr_t)ptr,
FIND_LE, (void *)&rkey, (void **)&rvalue);
if (!found || !rvalue) {
if (!found) {
break;
}

// rvalue == NULL means that the entry was removed
if (rvalue == NULL) {
// restart the search
parent_value = NULL;
parent_key = 0;
rkey = 0;
rsize = 0;
level = 0;
found = 0;
continue;
}

utils_atomic_load_acquire_u64((uint64_t *)&rvalue->size, &rsize);

if (found && (uintptr_t)ptr < rkey + rsize) {
Expand Down Expand Up @@ -195,10 +207,22 @@ static umf_result_t umfMemoryTrackerAdd(umf_memory_tracker_handle_t hTracker,
found =
critnib_find(hTracker->alloc_segments_map[level], (uintptr_t)ptr,
FIND_LE, (void *)&rkey, (void **)&rvalue);
if (!found || !rvalue) {
if (!found) {
break;
}

// rvalue == NULL means that the entry was removed
if (rvalue == NULL) {
// restart the search
parent_value = NULL;
parent_key = 0;
rkey = 0;
rsize = 0;
level = 0;
found = 0;
continue;
}

utils_atomic_load_acquire_u64((uint64_t *)&rvalue->size, &rsize);

if ((uintptr_t)ptr < rkey + rsize) {
Expand Down Expand Up @@ -383,6 +407,17 @@ umf_result_t umfMemoryTrackerGetAllocInfo(const void *ptr,
assert(level < MAX_LEVELS_OF_ALLOC_SEGMENT_MAP);
found = critnib_find(TRACKER->alloc_segments_map[level], (uintptr_t)ptr,
FIND_LE, (void *)&rkey, (void **)&rvalue);
// rvalue == NULL means that the entry was removed
if (found && (rvalue == NULL)) {
// restart the search
top_most_value = NULL;
top_most_key = 0;
rkey = 0;
level = 0;
found = 0;
continue;
}

if (found && (uintptr_t)ptr < rkey + rvalue->size) {
top_most_key = rkey;
top_most_value = rvalue;
Expand Down Expand Up @@ -428,8 +463,14 @@ umf_result_t umfMemoryTrackerGetIpcInfo(const void *ptr,

uintptr_t rkey;
tracker_ipc_info_t *rvalue = NULL;
int found = critnib_find(TRACKER->ipc_segments_map, (uintptr_t)ptr, FIND_LE,
int found = 0;

do {
found = critnib_find(TRACKER->ipc_segments_map, (uintptr_t)ptr, FIND_LE,
(void *)&rkey, (void **)&rvalue);
// rvalue == NULL means that the entry was removed
} while (found && (rvalue == NULL));

if (!found || (uintptr_t)ptr >= rkey + rvalue->size) {
LOG_DEBUG("pointer %p not found in the tracker, TRACKER=%p", ptr,
(void *)TRACKER);
Expand Down Expand Up @@ -768,6 +809,11 @@ static void check_if_tracker_is_empty(umf_memory_tracker_handle_t hTracker,

while (1 == critnib_find(hTracker->alloc_segments_map[i], last_key,
FIND_G, &rkey, (void **)&rvalue)) {
// rvalue == NULL means that the entry was removed
if (rvalue == NULL) {
continue;
}

if (rvalue->pool == pool || pool == NULL) {
n_items++;
}
Expand Down
Loading