smol-rs / concurrent-queue

Concurrent multi-producer multi-consumer queue
Apache License 2.0
254 stars 22 forks source link

Revert "Fix fence on non-x86 arch and miri (#16)" #18

Closed taiki-e closed 2 years ago

taiki-e commented 2 years ago

Based on the discussion in #17:

The fence for the unbounded queue needs to be fixed again in some way, but for now, I'm going to revert all of them.

@sbarral Any thoughts?

sbarral commented 2 years ago

@sbarral Any thoughts?

Nope, looks good to me :-)

RalfJung commented 2 years ago

This is still doing the fence before the load though, how does that make sense?

sbarral commented 2 years ago

This is still doing the fence before the load though, how does that make sense?

I do not know the full context of your reply (I understand this has been discussed someplace else) so sorry if my comment is irrelevant, but in general there certainly are legit use cases for SeqCst fences before a load. For instance:

// Thread A:
x.store(true, Ordering::relaxed);
fence(Ordering::SeqCst);
let y_val = y.load(Ordering::relaxed);

// Thread B:
y.store(true, Ordering::relaxed);
fence(Ordering::SeqCst);
let x_val = x.load(Ordering::relaxed);

will ensure that either the x store will be visible in thread B or the y store will be visible in thread A, or both.