aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRickard Green <[email protected]>2013-02-18 00:29:51 +0100
committerRickard Green <[email protected]>2013-02-21 01:08:09 +0100
commitdf40b0b99810121d9630730d6f58cf96bfea70e8 (patch)
tree33b715adbfe2522b45a98c92e939d9b1b879ea5f
parent4f6a7255afbd5b296139e073d66564976d80cc06 (diff)
downloadotp-df40b0b99810121d9630730d6f58cf96bfea70e8.tar.gz
otp-df40b0b99810121d9630730d6f58cf96bfea70e8.tar.bz2
otp-df40b0b99810121d9630730d6f58cf96bfea70e8.zip
Use dirty read instead of union which can be unsafe on some platforms
-rw-r--r--erts/emulator/beam/erl_thr_queue.c100
-rw-r--r--erts/emulator/beam/erl_thr_queue.h13
2 files changed, 61 insertions, 52 deletions
diff --git a/erts/emulator/beam/erl_thr_queue.c b/erts/emulator/beam/erl_thr_queue.c
index f07964a265..acadc5d4cd 100644
--- a/erts/emulator/beam/erl_thr_queue.c
+++ b/erts/emulator/beam/erl_thr_queue.c
@@ -113,6 +113,11 @@ sl_element_free(ErtsThrQElement_t *p)
#endif
+#define ErtsThrQDirtyReadEl(A) \
+ ((ErtsThrQElement_t *) erts_atomic_read_dirty((A)))
+#define ErtsThrQDirtySetEl(A, V) \
+ erts_atomic_set_dirty((A), (erts_aint_t) (V))
+
typedef union {
ErtsThrQ_t q;
char align__[ERTS_ALC_CACHE_LINE_ALIGN_SIZE(sizeof(ErtsThrQ_t))];
@@ -137,7 +142,7 @@ erts_thr_q_initialize(ErtsThrQ_t *q, ErtsThrQInit_t *qi)
q->last = NULL;
q->q.blk = NULL;
#else
- erts_atomic_init_nob(&q->tail.data.marker.next.atmc, ERTS_AINT_NULL);
+ erts_atomic_init_nob(&q->tail.data.marker.next, ERTS_AINT_NULL);
q->tail.data.marker.data.ptr = NULL;
erts_atomic_init_nob(&q->tail.data.last,
(erts_aint_t) &q->tail.data.marker);
@@ -150,7 +155,7 @@ erts_thr_q_initialize(ErtsThrQ_t *q, ErtsThrQInit_t *qi)
if (!q->tail.data.notify)
q->tail.data.notify = noop_callback;
- q->head.head.ptr = &q->tail.data.marker;
+ erts_atomic_init_nob(&q->head.head, (erts_aint_t) &q->tail.data.marker);
q->head.live = qi->live.objects;
q->head.first = &q->tail.data.marker;
q->head.unref_end = &q->tail.data.marker;
@@ -300,13 +305,13 @@ enqueue_managed(ErtsThrQ_t *q, ErtsThrQElement_t *this, int want_last)
{
erts_aint_t ilast, itmp;
- erts_atomic_init_nob(&this->next.atmc, ERTS_AINT_NULL);
+ erts_atomic_init_nob(&this->next, ERTS_AINT_NULL);
/* Enqueue at end of list... */
ilast = erts_atomic_read_nob(&q->tail.data.last);
while (1) {
ErtsThrQElement_t *last = (ErtsThrQElement_t *) ilast;
- itmp = erts_atomic_cmpxchg_mb(&last->next.atmc,
+ itmp = erts_atomic_cmpxchg_mb(&last->next,
(erts_aint_t) this,
ERTS_AINT_NULL);
if (itmp == ERTS_AINT_NULL)
@@ -317,14 +322,14 @@ enqueue_managed(ErtsThrQ_t *q, ErtsThrQElement_t *this, int want_last)
/* Move last pointer forward... */
while (1) {
if (want_last) {
- if (erts_atomic_read_rb(&this->next.atmc) != ERTS_AINT_NULL) {
+ if (erts_atomic_read_rb(&this->next) != ERTS_AINT_NULL) {
/* Someone else will move it forward */
ilast = erts_atomic_read_rb(&q->tail.data.last);
return (ErtsThrQElement_t *) ilast;
}
}
else {
- if (erts_atomic_read_nob(&this->next.atmc) != ERTS_AINT_NULL) {
+ if (erts_atomic_read_nob(&this->next) != ERTS_AINT_NULL) {
/* Someone else will move it forward */
return NULL;
}
@@ -341,6 +346,7 @@ enqueue_managed(ErtsThrQ_t *q, ErtsThrQElement_t *this, int want_last)
static ErtsThrQCleanState_t
clean(ErtsThrQ_t *q, int max_ops, int do_notify)
{
+ ErtsThrQElement_t *head;
erts_aint_t ilast;
int um_refc_ix;
int ops;
@@ -349,7 +355,8 @@ clean(ErtsThrQ_t *q, int max_ops, int do_notify)
ErtsThrQElement_t *tmp;
restart:
ASSERT(q->head.first);
- if (q->head.first == q->head.head.ptr) {
+ head = ErtsThrQDirtyReadEl(&q->head.head);
+ if (q->head.first == head) {
q->head.clean_reached_head_count++;
if (q->head.clean_reached_head_count
>= ERTS_THR_Q_MAX_CLEAN_REACHED_HEAD_COUNT) {
@@ -362,19 +369,20 @@ clean(ErtsThrQ_t *q, int max_ops, int do_notify)
break;
if (q->head.first == &q->tail.data.marker) {
q->head.used_marker = 0;
- q->head.first = q->head.first->next.ptr;
+ q->head.first = ErtsThrQDirtyReadEl(&q->head.first->next);
goto restart;
}
tmp = q->head.first;
- q->head.first = q->head.first->next.ptr;
+ q->head.first = ErtsThrQDirtyReadEl(&q->head.first->next);
if (q->head.deq_fini.automatic)
element_free(q, tmp);
else {
tmp->data.ptr = (void *) (UWord) q->head.live;
if (!q->head.deq_fini.start)
q->head.deq_fini.start = tmp;
- else if (q->head.deq_fini.end->next.ptr == &q->tail.data.marker)
- q->head.deq_fini.end->next.ptr = tmp;
+ else if (ErtsThrQDirtyReadEl(&q->head.deq_fini.end->next)
+ == &q->tail.data.marker)
+ ErtsThrQDirtySetEl(&q->head.deq_fini.end->next, tmp);
q->head.deq_fini.end = tmp;
}
}
@@ -406,14 +414,13 @@ clean(ErtsThrQ_t *q, int max_ops, int do_notify)
ilast = (erts_aint_t) enqueue_managed(q,
&q->tail.data.marker,
1);
- if (q->head.head.ptr == q->head.unref_end) {
+ head = ErtsThrQDirtyReadEl(&q->head.head);
+ if (head == q->head.unref_end) {
ErtsThrQElement_t *next;
next = ((ErtsThrQElement_t *)
- erts_atomic_read_acqb(&q->head.head.ptr->next.atmc));
- if (next == &q->tail.data.marker) {
- q->head.head.ptr->next.ptr = &q->tail.data.marker;
- q->head.head.ptr = &q->tail.data.marker;
- }
+ erts_atomic_read_acqb(&head->next));
+ if (next == &q->tail.data.marker)
+ ErtsThrQDirtySetEl(&q->head.head, &q->tail.data.marker);
}
}
@@ -436,18 +443,18 @@ clean(ErtsThrQ_t *q, int max_ops, int do_notify)
}
#endif
- if (q->head.first == q->head.head.ptr) {
+ head = ErtsThrQDirtyReadEl(&q->head.head);
+ if (q->head.first == head) {
inspect_head:
if (!q->head.used_marker) {
erts_aint_t inext;
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ inext = erts_atomic_read_acqb(&head->next);
if (inext == ERTS_AINT_NULL) {
q->head.used_marker = 1;
(void) enqueue_managed(q, &q->tail.data.marker, 0);
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ inext = erts_atomic_read_acqb(&head->next);
if (inext == (erts_aint_t) &q->tail.data.marker) {
- q->head.head.ptr->next.ptr = &q->tail.data.marker;
- q->head.head.ptr = &q->tail.data.marker;
+ ErtsThrQDirtySetEl(&q->head.head, &q->tail.data.marker);
goto check_thr_progress;
}
}
@@ -506,26 +513,27 @@ erts_thr_q_inspect(ErtsThrQ_t *q, int ensure_empty)
#ifndef USE_THREADS
return ERTS_THR_Q_CLEAN;
#else
+ ErtsThrQElement_t *head = ErtsThrQDirtyReadEl(&q->head.head);
if (ensure_empty) {
erts_aint_t inext;
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ inext = erts_atomic_read_acqb(&head->next);
if (inext != ERTS_AINT_NULL) {
if (&q->tail.data.marker != (ErtsThrQElement_t *) inext)
return ERTS_THR_Q_DIRTY;
else {
- q->head.head.ptr->next.ptr = (ErtsThrQElement_t *) inext;
- q->head.head.ptr = (ErtsThrQElement_t *) inext;
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ head = (ErtsThrQElement_t *) inext;
+ ErtsThrQDirtySetEl(&q->head.head, head);
+ inext = erts_atomic_read_acqb(&head->next);
if (inext != ERTS_AINT_NULL)
return ERTS_THR_Q_DIRTY;
}
}
}
- if (q->head.first == q->head.head.ptr) {
+ if (q->head.first == head) {
if (!q->head.used_marker) {
erts_aint_t inext;
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ inext = erts_atomic_read_acqb(&head->next);
if (inext == ERTS_AINT_NULL)
return ERTS_THR_Q_DIRTY;
}
@@ -553,11 +561,11 @@ enqueue(ErtsThrQ_t *q, void *data, ErtsThrQElement_t *this)
#ifndef USE_THREADS
ASSERT(data);
- this->next.ptr = NULL;
+ this->next = NULL;
this->data.ptr = data;
if (q->last)
- q->last->next.ptr = this;
+ q->last->next = this;
else {
q->first = q->last = this;
q->init.notify(q->init.arg);
@@ -638,17 +646,17 @@ erts_thr_q_get_finalize_dequeue_data(ErtsThrQ_t *q, ErtsThrQFinDeQ_t *fdp)
ErtsThrQElement_t *e = q->head.deq_fini.start;
ErtsThrQElement_t *end = q->head.deq_fini.end;
while (e != end) {
- ASSERT(q->head.head.ptr != e);
+ ASSERT(ErtsThrQDirtyReadEl(&q->head.head) != e);
ASSERT(q->head.first != e);
ASSERT(q->head.unref_end != e);
- e = e->next.ptr;
+ e = ErtsThrQDirtyReadEl(&e->next);
}
}
#endif
fdp->start = q->head.deq_fini.start;
fdp->end = q->head.deq_fini.end;
if (fdp->end)
- fdp->end->next.ptr = NULL;
+ ErtsThrQDirtySetEl(&fdp->end->next, NULL);
q->head.deq_fini.start = NULL;
q->head.deq_fini.end = NULL;
return fdp->start != NULL;
@@ -662,7 +670,7 @@ erts_thr_q_append_finalize_dequeue_data(ErtsThrQFinDeQ_t *fdp0,
#ifdef USE_THREADS
if (fdp1->start) {
if (fdp0->end)
- fdp0->end->next.ptr = fdp1->start;
+ ErtsThrQDirtySetEl(&fdp0->end->next, fdp1->start);
else
fdp0->start = fdp1->start;
fdp0->end = fdp1->end;
@@ -683,7 +691,7 @@ int erts_thr_q_finalize_dequeue(ErtsThrQFinDeQ_t *state)
if (!start)
break;
tmp = start;
- start = start->next.ptr;
+ start = ErtsThrQDirtyReadEl(&start->next);
live = (ErtsThrQLive_t) (UWord) tmp->data.ptr;
element_live_free(live, tmp);
}
@@ -724,7 +732,7 @@ erts_thr_q_dequeue(ErtsThrQ_t *q)
return NULL;
tmp = q->first;
res = tmp->data.ptr;
- q->first = tmp->next.ptr;
+ q->first = tmp->next;
if (!q->first)
q->last = NULL;
@@ -732,24 +740,26 @@ erts_thr_q_dequeue(ErtsThrQ_t *q)
return res;
#else
+ ErtsThrQElement_t *head;
erts_aint_t inext;
void *res;
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ head = ErtsThrQDirtyReadEl(&q->head.head);
+ inext = erts_atomic_read_acqb(&head->next);
if (inext == ERTS_AINT_NULL)
return NULL;
- q->head.head.ptr->next.ptr = (ErtsThrQElement_t *) inext;
- q->head.head.ptr = (ErtsThrQElement_t *) inext;
- if (q->head.head.ptr == &q->tail.data.marker) {
- inext = erts_atomic_read_acqb(&q->head.head.ptr->next.atmc);
+ head = (ErtsThrQElement_t *) inext;
+ ErtsThrQDirtySetEl(&q->head.head, head);
+ if (head == &q->tail.data.marker) {
+ inext = erts_atomic_read_acqb(&head->next);
if (inext == ERTS_AINT_NULL)
return NULL;
- q->head.head.ptr->next.ptr = (ErtsThrQElement_t *) inext;
- q->head.head.ptr = (ErtsThrQElement_t *) inext;
+ head = (ErtsThrQElement_t *) inext;
+ ErtsThrQDirtySetEl(&q->head.head, head);
}
- res = q->head.head.ptr->data.ptr;
+ res = head->data.ptr;
#if ERTS_THR_Q_DBG_CHK_DATA
- q->head.head.ptr->data.ptr = NULL;
+ head->data.ptr = NULL;
if (!res)
erl_exit(ERTS_ABORT_EXIT, "Missing data in dequeue\n");
#endif
diff --git a/erts/emulator/beam/erl_thr_queue.h b/erts/emulator/beam/erl_thr_queue.h
index edcf2c3823..ae8c7fb19a 100644
--- a/erts/emulator/beam/erl_thr_queue.h
+++ b/erts/emulator/beam/erl_thr_queue.h
@@ -76,13 +76,12 @@ typedef struct {
typedef struct ErtsThrQElement_t_ ErtsThrQElement_t;
typedef struct ErtsThrQElement_t ErtsThrQPrepEnQ_t;
-typedef union {
- erts_atomic_t atmc;
- ErtsThrQElement_t *ptr;
-} ErtsThrQPtr_t;
-
struct ErtsThrQElement_t_ {
- ErtsThrQPtr_t next;
+#ifdef USE_THREADS
+ erts_atomic_t next;
+#else
+ ErtsThrQElement_t *next;
+#endif
union {
erts_atomic_t atmc;
void *ptr;
@@ -130,7 +129,7 @@ struct ErtsThrQ_t_ {
* thread dequeuing.
*/
struct {
- ErtsThrQPtr_t head;
+ erts_atomic_t head;
ErtsThrQLive_t live;
ErtsThrQElement_t *first;
ErtsThrQElement_t *unref_end;