concurrencykit / ck

Concurrency primitives, safe memory reclamation mechanisms and non-blocking (including lock-free) data structures designed to aid in the research, design and implementation of high performance concurrent systems developed in C99+.
http://concurrencykit.org/
Other
2.34k stars 312 forks source link

Question about ck_rhs_flush and memory ordering. #229

Closed michael-grunder closed 4 months ago

michael-grunder commented 4 months ago

Hi :wave:

I have what I hope is a simple question about the ck_rhs_flush operation and its memory ordering guarantees.

https://github.com/concurrencykit/ck/blob/07e90d02251aa038bcd715cd0c5ca591a548575b/src/ck_rhs.c#L405-L412

Ignoring error handling the logic is very straightforward.

  1. Take a copy of the existing map.
  2. Allocate a new map
  3. atomically store the new map replacing the old in the set.
  4. destroy the previous map.

As with all write operations against ck_rhs there must never be more than one write operation at any one time.

That said, it's not clear to me whether a subsequent reset operation is at risk of reading stale data from hs->map given that there isn't a store-load fence. In other words is it possible on relaxed memory models like arm that something like this happens:

Flush #1 (CPU1)
previous = hs->map;
updated = ck_rhs_map_create();
ck_pr_store_ptr(&hs->map, updated);

Flush #2 (CPU2)
// Is it possible this reads `hs->map` from *before* the above atomic store?
previous = hs->map;

I've done a lot of concurrent testing for this on an ARM machine and can't make it happen but was wondering if it is technically possible. Basically should my wrapping logic emit a ck_pr_fence_memory() after the flush?

cognet commented 4 months ago

Hmm I guess it is theorically possible if m->free() is a no-op, or just doesn't contain any barrier. At least I don't see what would prevent it. That is true for ck_hs too.

michael-grunder commented 4 months ago

Awesome, thanks for answering!

I'll emit a barrier in my m->free wrapper for completeness.