Skip to content

Commit 4ea2494

Browse files
authored
[OpenMP] Fix nested parallel with tasking (#87309)
When a nested parallel region ends, the runtime calls __kmp_join_call(). During this call, the primary thread of the nested parallel region will reset its tid (retval of omp_get_thread_num()) to what it was in the outer parallel region. A data race occurs with the current code when another worker thread from the nested inner parallel region tries to steal tasks from the primary thread's task deque. The worker thread reads the tid value directly from the primary thread's data structure and may read the wrong value. This change just uses the calculated victim_tid from execute_tasks() directly in the steal_task() routine rather than reading tid from the data structure. Fixes: #87307
1 parent f119a4f commit 4ea2494

File tree

2 files changed

+52
-6
lines changed

2 files changed

+52
-6
lines changed

openmp/runtime/src/kmp_tasking.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3219,7 +3219,7 @@ static kmp_task_t *__kmp_remove_my_task(kmp_info_t *thread, kmp_int32 gtid,
32193219
// __kmp_steal_task: remove a task from another thread's deque
32203220
// Assume that calling thread has already checked existence of
32213221
// task_team thread_data before calling this routine.
3222-
static kmp_task_t *__kmp_steal_task(kmp_info_t *victim_thr, kmp_int32 gtid,
3222+
static kmp_task_t *__kmp_steal_task(kmp_int32 victim_tid, kmp_int32 gtid,
32233223
kmp_task_team_t *task_team,
32243224
std::atomic<kmp_int32> *unfinished_threads,
32253225
int *thread_finished,
@@ -3229,15 +3229,18 @@ static kmp_task_t *__kmp_steal_task(kmp_info_t *victim_thr, kmp_int32 gtid,
32293229
kmp_taskdata_t *current;
32303230
kmp_thread_data_t *victim_td, *threads_data;
32313231
kmp_int32 target;
3232-
kmp_int32 victim_tid;
3232+
kmp_info_t *victim_thr;
32333233

32343234
KMP_DEBUG_ASSERT(__kmp_tasking_mode != tskm_immediate_exec);
32353235

32363236
threads_data = task_team->tt.tt_threads_data;
32373237
KMP_DEBUG_ASSERT(threads_data != NULL); // Caller should check this condition
3238+
KMP_DEBUG_ASSERT(victim_tid >= 0);
3239+
KMP_DEBUG_ASSERT(victim_tid < task_team->tt.tt_nproc);
32383240

3239-
victim_tid = victim_thr->th.th_info.ds.ds_tid;
32403241
victim_td = &threads_data[victim_tid];
3242+
victim_thr = victim_td->td.td_thr;
3243+
(void)victim_thr; // Use in TRACE messages which aren't always enabled.
32413244

32423245
KA_TRACE(10, ("__kmp_steal_task(enter): T#%d try to steal from T#%d: "
32433246
"task_team=%p ntasks=%d head=%u tail=%u\n",
@@ -3452,9 +3455,9 @@ static inline int __kmp_execute_tasks_template(
34523455

34533456
if (!asleep) {
34543457
// We have a victim to try to steal from
3455-
task = __kmp_steal_task(other_thread, gtid, task_team,
3456-
unfinished_threads, thread_finished,
3457-
is_constrained);
3458+
task =
3459+
__kmp_steal_task(victim_tid, gtid, task_team, unfinished_threads,
3460+
thread_finished, is_constrained);
34583461
}
34593462
if (task != NULL) { // set last stolen to victim
34603463
if (threads_data[tid].td.td_deque_last_stolen != victim_tid) {
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: %libomp-compile-and-run
2+
#include <stdio.h>
3+
#include <stdlib.h>
4+
#include <omp.h>
5+
6+
int a;
7+
8+
void inc_a() {
9+
#pragma omp task
10+
{
11+
#pragma omp atomic
12+
a++;
13+
}
14+
}
15+
16+
int main() {
17+
int n;
18+
int nth_outer;
19+
omp_set_max_active_levels(2);
20+
omp_set_dynamic(0);
21+
22+
for (n = 0; n < 200; ++n) {
23+
a = 0;
24+
#pragma omp parallel num_threads(8)
25+
{
26+
if (omp_get_thread_num() == 0)
27+
nth_outer = omp_get_num_threads();
28+
#pragma omp parallel num_threads(2)
29+
{
30+
int i;
31+
#pragma omp master
32+
for (i = 0; i < 50; ++i)
33+
inc_a();
34+
}
35+
}
36+
if (a != nth_outer * 50) {
37+
fprintf(stderr, "error: a (%d) != %d\n", a, nth_outer * 50);
38+
return EXIT_FAILURE;
39+
}
40+
}
41+
42+
return EXIT_SUCCESS;
43+
}

0 commit comments

Comments
 (0)