From 319872ca8ca52ca911073eaa56d3e1a89d4ba781 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Wed, 18 Jul 2012 14:13:16 -0400 Subject: [PATCH 01/10] 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)) From 7d6626131d34a6ad18cf58ae58131c51acd2495c Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Wed, 18 Jul 2012 15:32:37 -0400 Subject: [PATCH 02/10] ck_hp_fifo: Forgot load fence in last commit. --- include/ck_hp_fifo.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/ck_hp_fifo.h b/include/ck_hp_fifo.h index 45a7944..73b77bf 100644 --- a/include/ck_hp_fifo.h +++ b/include/ck_hp_fifo.h @@ -135,6 +135,7 @@ ck_hp_fifo_dequeue_mpmc(ck_hp_record_t *record, for (;;) { head = ck_pr_load_ptr(&fifo->head); + ck_pr_fence_load(); tail = ck_pr_load_ptr(&fifo->tail); ck_hp_set(record, 0, head); ck_pr_fence_memory(); @@ -169,6 +170,7 @@ ck_hp_fifo_trydequeue_mpmc(ck_hp_record_t *record, struct ck_hp_fifo_entry *head, *tail, *next; head = ck_pr_load_ptr(&fifo->head); + ck_pr_fence_load(); tail = ck_pr_load_ptr(&fifo->tail); ck_hp_set(record, 0, head); ck_pr_fence_memory(); From ccb1fd6d869c4dbcffff032c9a4bd4f2d04a9508 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Wed, 18 Jul 2012 15:49:26 -0400 Subject: [PATCH 03/10] ck_fifo: Add some load fences for SPARC/PPC. --- include/ck_fifo.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/include/ck_fifo.h b/include/ck_fifo.h index ea40ab2..f3ac243 100644 --- a/include/ck_fifo.h +++ b/include/ck_fifo.h @@ -249,8 +249,9 @@ ck_fifo_mpmc_enqueue(struct ck_fifo_mpmc *fifo, for (;;) { tail.generation = ck_pr_load_ptr(&fifo->tail.generation); + ck_pr_fence_load(); tail.pointer = ck_pr_load_ptr(&fifo->tail.pointer); - + ck_pr_fence_load_depends(); next.generation = ck_pr_load_ptr(&tail.pointer->next.generation); next.pointer = ck_pr_load_ptr(&tail.pointer->next.pointer); @@ -299,8 +300,9 @@ ck_fifo_mpmc_tryenqueue(struct ck_fifo_mpmc *fifo, ck_pr_fence_store(); tail.generation = ck_pr_load_ptr(&fifo->tail.generation); + ck_pr_fence_load(); tail.pointer = ck_pr_load_ptr(&fifo->tail.pointer); - + ck_pr_fence_load_depends(); next.generation = ck_pr_load_ptr(&tail.pointer->next.generation); next.pointer = ck_pr_load_ptr(&tail.pointer->next.pointer); @@ -344,9 +346,10 @@ ck_fifo_mpmc_dequeue(struct ck_fifo_mpmc *fifo, for (;;) { head.generation = ck_pr_load_ptr(&fifo->head.generation); + ck_pr_fence_load(); head.pointer = ck_pr_load_ptr(&fifo->head.pointer); - tail.generation = ck_pr_load_ptr(&fifo->tail.generation); + ck_pr_fence_load(); tail.pointer = ck_pr_load_ptr(&fifo->tail.pointer); next.generation = ck_pr_load_ptr(&head.pointer->next.generation); @@ -388,9 +391,11 @@ ck_fifo_mpmc_trydequeue(struct ck_fifo_mpmc *fifo, struct ck_fifo_mpmc_pointer head, tail, next, update; head.generation = ck_pr_load_ptr(&fifo->head.generation); + ck_pr_fence_load(); head.pointer = ck_pr_load_ptr(&fifo->head.pointer); tail.generation = ck_pr_load_ptr(&fifo->tail.generation); + ck_pr_fence_load(); tail.pointer = ck_pr_load_ptr(&fifo->tail.pointer); next.generation = ck_pr_load_ptr(&head.pointer->next.generation); From 59158c824ba76441e499dbf44c64c6afb2c86c89 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Wed, 18 Jul 2012 19:00:00 -0400 Subject: [PATCH 04/10] ck_ht: Do not re-hash on growth for non-PP case. We already store the hash value in the hash table entry if pointer packing is not enabled. --- include/ck_ht.h | 7 +++++++ regressions/ck_ht/benchmark/parallel_direct.c | 4 ++-- regressions/ck_ht/validate/serial.c | 4 ++-- src/ck_ht.c | 8 ++++++++ 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/include/ck_ht.h b/include/ck_ht.h index ddd8f78..33be38c 100644 --- a/include/ck_ht.h +++ b/include/ck_ht.h @@ -188,12 +188,19 @@ ck_ht_entry_set(struct ck_ht_entry *entry, CK_CC_INLINE static void ck_ht_entry_set_direct(struct ck_ht_entry *entry, + ck_ht_hash_t h, uintptr_t key, uintptr_t value) { entry->key = key; entry->value = value; + +#ifndef CK_HT_PP + entry->hash = h.value; +#else + (void)h; +#endif return; } diff --git a/regressions/ck_ht/benchmark/parallel_direct.c b/regressions/ck_ht/benchmark/parallel_direct.c index 0f1b3e9..94e834c 100644 --- a/regressions/ck_ht/benchmark/parallel_direct.c +++ b/regressions/ck_ht/benchmark/parallel_direct.c @@ -162,7 +162,7 @@ table_replace(uintptr_t value) ck_ht_hash_t h; ck_ht_hash_direct(&h, &ht, value); - ck_ht_entry_set_direct(&entry, value, 6605241); + ck_ht_entry_set_direct(&entry, h, value, 6605241); return ck_ht_set_spmc(&ht, h, &entry); } @@ -187,7 +187,7 @@ table_insert(uintptr_t value) ck_ht_hash_t h; ck_ht_hash_direct(&h, &ht, value); - ck_ht_entry_set_direct(&entry, value, value); + ck_ht_entry_set_direct(&entry, h, value, value); return ck_ht_put_spmc(&ht, h, &entry); } diff --git a/regressions/ck_ht/validate/serial.c b/regressions/ck_ht/validate/serial.c index 200dbc3..9278927 100644 --- a/regressions/ck_ht/validate/serial.c +++ b/regressions/ck_ht/validate/serial.c @@ -254,7 +254,7 @@ main(void) l = 0; for (i = 0; i < sizeof(direct) / sizeof(*direct); i++) { ck_ht_hash_direct(&h, &ht, direct[i]); - ck_ht_entry_set_direct(&entry, direct[i], (uintptr_t)test[i]); + ck_ht_entry_set_direct(&entry, h, direct[i], (uintptr_t)test[i]); l += ck_ht_put_spmc(&ht, h, &entry) == false; } @@ -265,7 +265,7 @@ main(void) for (i = 0; i < sizeof(direct) / sizeof(*direct); i++) { ck_ht_hash_direct(&h, &ht, direct[i]); - ck_ht_entry_set_direct(&entry, direct[i], (uintptr_t)"REPLACED"); + ck_ht_entry_set_direct(&entry, h, direct[i], (uintptr_t)"REPLACED"); l += ck_ht_set_spmc(&ht, h, &entry) == false; } diff --git a/src/ck_ht.c b/src/ck_ht.c index 8f71f42..414c9dd 100644 --- a/src/ck_ht.c +++ b/src/ck_ht.c @@ -404,9 +404,17 @@ restart: key = ck_ht_entry_key(previous); key_length = ck_ht_entry_key_length(previous); +#ifndef CK_HT_PP + h.value = previous->hash; +#else table->h(&h, key, key_length, table->seed); +#endif } else { +#ifndef CK_HT_PP + h.value = previous->hash; +#else table->h(&h, &previous->key, sizeof(previous->key), table->seed); +#endif } offset = h.value & update->mask; From 7dd705c86f5d85b872792d638c2b75143f8732cc Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Thu, 19 Jul 2012 11:55:54 -0400 Subject: [PATCH 05/10] ck_ht: Remove ck_ht_allocator_set declaration. --- include/ck_ht.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/ck_ht.h b/include/ck_ht.h index 33be38c..a0218e7 100644 --- a/include/ck_ht.h +++ b/include/ck_ht.h @@ -226,7 +226,6 @@ bool ck_ht_next(ck_ht_t *, ck_ht_iterator_t *, ck_ht_entry_t **entry); void ck_ht_hash(ck_ht_hash_t *, ck_ht_t *, const void *, uint16_t); void ck_ht_hash_direct(ck_ht_hash_t *, ck_ht_t *, uintptr_t); -bool ck_ht_allocator_set(struct ck_malloc *); bool ck_ht_init(ck_ht_t *, enum ck_ht_mode, ck_ht_hash_cb_t *, struct ck_malloc *, uint64_t, uint64_t); void ck_ht_destroy(ck_ht_t *); bool ck_ht_set_spmc(ck_ht_t *, ck_ht_hash_t, ck_ht_entry_t *); From 65cf506af99ec93fc4f0672bee6cbf35da1818af Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Mon, 23 Jul 2012 11:50:52 -0400 Subject: [PATCH 06/10] ck_cc: Add CK_CC_ALIASED attribute. Submitted by John Wittrock . --- include/ck_cc.h | 4 ++++ include/gcc/ck_cc.h | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/include/ck_cc.h b/include/ck_cc.h index b1549bb..c5f17a4 100644 --- a/include/ck_cc.h +++ b/include/ck_cc.h @@ -45,4 +45,8 @@ #define CK_CC_PAD(x) union { char pad[x]; } +#ifndef CK_CC_ALIASED +#define CK_CC_ALIASED +#endif + #endif /* _CK_CC_H */ diff --git a/include/gcc/ck_cc.h b/include/gcc/ck_cc.h index 15743e7..f7b13a7 100644 --- a/include/gcc/ck_cc.h +++ b/include/gcc/ck_cc.h @@ -75,4 +75,11 @@ #pragma GCC poison malloc free #endif +/* + * Some compilers are overly strict regarding aliasing semantics. + * Unfortunately, in many cases it makes more sense to pay aliasing + * cost rather than overly expensive register spillage. + */ +#define CK_CC_ALIASED __attribute__((__may_alias__)) + #endif /* _CK_GCC_CC_H */ From 031d950cc0b5531461473a70c6b2cfe880e65271 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Mon, 23 Jul 2012 11:51:22 -0400 Subject: [PATCH 07/10] ck_stack: Add aliased attribute to ck_stack for GCC 4.4. --- include/ck_stack.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ck_stack.h b/include/ck_stack.h index 56a39c3..f71e515 100644 --- a/include/ck_stack.h +++ b/include/ck_stack.h @@ -40,7 +40,7 @@ typedef struct ck_stack_entry ck_stack_entry_t; struct ck_stack { struct ck_stack_entry *head; char *generation CK_CC_PACKED; -}; +} CK_CC_ALIASED; typedef struct ck_stack ck_stack_t; #define CK_STACK_INITIALIZER { NULL, NULL } From 976f017450f6bd6b5c42457c91bd4ab0ad1d9a99 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Thu, 26 Jul 2012 23:21:56 -0400 Subject: [PATCH 08/10] build: Move symbolic link to -devel package, keep .spec file in distributions. Patch submitted by Andrew Schorr . --- Makefile.in | 1 - build/ck.spec.in | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile.in b/Makefile.in index c3ab190..806b53a 100644 --- a/Makefile.in +++ b/Makefile.in @@ -66,7 +66,6 @@ distribution: clean rm -f include/ck_md.h rm -f build/regressions.build rm -f build/ck.build - rm -f build/ck.spec rm -f build/ck.pc rm -f Makefile rm -f doc/Makefile diff --git a/build/ck.spec.in b/build/ck.spec.in index e4caace..7b71faf 100644 --- a/build/ck.spec.in +++ b/build/ck.spec.in @@ -53,12 +53,12 @@ rm -rf $RPM_BUILD_ROOT %files %defattr(-,root,root,-) -%{_libdir}/libck.so %{_libdir}/libck.so.@VERSION@ %{_libdir}/libck.so.@VERSION_MAJOR@ %files devel %defattr(-,root,root) +%{_libdir}/libck.so %{_includedir}/%{name}/*.h %{_includedir}/%{name}/*/*.h %{_includedir}/%{name}/*/*/*.h From 37d8a5e98d4ff8c8d864b48e7775ce5b049b77e8 Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Mon, 30 Jul 2012 14:37:27 -0400 Subject: [PATCH 09/10] build: Bump version for next release. --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 0874213..f62bb4a 100755 --- a/configure +++ b/configure @@ -32,7 +32,7 @@ EXIT_SUCCESS=0 EXIT_FAILURE=1 MAINTAINER='sbahra@repnop.org' -VERSION='0.2.5' +VERSION='0.2.6' VERSION_MAJOR='0' BUILD="$PWD/build/ck.build" PREFIX=${PREFIX:-"/usr/local"} From e9bd7448778a030024de41c3a95a40447f3e274c Mon Sep 17 00:00:00 2001 From: Samy Al Bahra Date: Mon, 30 Jul 2012 15:48:20 -0400 Subject: [PATCH 10/10] doc/ck_ht_entry_set_direct: Update manual page to include latest API change. --- doc/ck_ht_entry_set_direct | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/ck_ht_entry_set_direct b/doc/ck_ht_entry_set_direct index 8042d33..69e2ed7 100644 --- a/doc/ck_ht_entry_set_direct +++ b/doc/ck_ht_entry_set_direct @@ -34,15 +34,17 @@ Concurrency Kit (libck, \-lck) .Sh SYNOPSIS .In ck_ht.h .Ft void -.Fn ck_ht_entry_set "ck_ht_entry_t *entry" "uintptr_t key" "uintptr_t value" +.Fn ck_ht_entry_set_direct "ck_ht_entry_t *entry" "ck_ht_hash_t h" "uintptr_t key" "uintptr_t value" .Sh DESCRIPTION The .Fn ck_ht_entry_set function will initialize the object pointed to by .Fa entry -with the key value specified in the +with the hash value specified by the +.Fa h +argument, the key value specified in the .Fa key -argument and a value specified by the +argument and the value specified by the .Fa value argument. .Pp