From 319872ca8ca52ca911073eaa56d3e1a89d4ba781 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Wed, 18 Jul 2012 14:13:16 -0400 Subject: [PATCH] ck_hp_fifo: Fix race condition on dequeue. I accidentally swapped head/tail load in ck_hp_fifo (not in ck_fifo, however). We must acquire head snapshot before tail snapshot. An example execution history which could cause an incorrect update to occur is below. - tail <- fifo.tail / fifo.head != fifo.tail - dequeue to empty (until final CAS which renders fifo.head = fifo.tail) - head <- fifo.head / (head != tail) - next <- fifo.head->next / next = NULL - As head != tail, update to next pointer (where next is NULL). However, if - head <- fifo.head / (fifo.head != fifo.tail) - dequeue to empty (until final CAS which renders fifo.head = fifo.tail) - tail <- fifo.tail / fifo.head != fifo.tail - next <- fifo.head->next / next = NULL If we caught tail in final transition, the by the time we read next pointer, head would have also changed forcing us to re-read. Thanks to Hendrik Donner for reporting this. --- include/ck_hp_fifo.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/ck_hp_fifo.h b/include/ck_hp_fifo.h index 3a6dd47..45a7944 100644 --- a/include/ck_hp_fifo.h +++ b/include/ck_hp_fifo.h @@ -134,9 +134,8 @@ ck_hp_fifo_dequeue_mpmc(ck_hp_record_t *record, struct ck_hp_fifo_entry *head, *tail, *next; for (;;) { - tail = ck_pr_load_ptr(&fifo->tail); - head = ck_pr_load_ptr(&fifo->head); + tail = ck_pr_load_ptr(&fifo->tail); ck_hp_set(record, 0, head); ck_pr_fence_memory(); if (head != ck_pr_load_ptr(&fifo->head)) @@ -169,8 +168,8 @@ ck_hp_fifo_trydequeue_mpmc(ck_hp_record_t *record, { struct ck_hp_fifo_entry *head, *tail, *next; - tail = ck_pr_load_ptr(&fifo->tail); head = ck_pr_load_ptr(&fifo->head); + tail = ck_pr_load_ptr(&fifo->tail); ck_hp_set(record, 0, head); ck_pr_fence_memory(); if (head != ck_pr_load_ptr(&fifo->head))