buf_ring.h: fix memory order issues.

[oleg]

Summary

Properly use acqure/release atomics.

[alc]

Have you looked at D1945, in particular, the most recent postings by sbahra_repnop.org? It's not clear to me that these changes will address the problem described in sbahra_repnop.org's postings. That said, your proposed changes do correct the most obvious remaining issues with the use of acquires and releases in this code.

sys/sys/buf_ring.h

78

Was there a reason for moving this load?

138

You may need to use a load acquire on br_prod_tail here to establish an unbroken synchronizes-with chain between the thread that enqueues an item X and the thread that later dequeues it if there are other concurrent enqueues.

157

Was there a reason for moving this load?

230

Was there a reason for swapping the order of the preceding loads?

250

Was there a reason for swapping the order of the preceding loads?

[oleg]

In D8637#180625, @alc wrote:

Have you looked at D1945, in particular, the most recent postings by sbahra_repnop.org? It's not clear to me that these changes will address the problem described in sbahra_repnop.org's postings. That said, your proposed changes do correct the most obvious remaining issues with the use of acquires and releases in this code.

Yes, i've looked at D1945 but somehow i've missed sbahra_repnop.org's comments. Since i was trying to fix different issue (https://lists.freebsd.org/pipermail/freebsd-net/2013-December/037265.html) and remove my own rmb() hack the problem described by sbahra_repnop.org is still here. I'll post updated diff tomorrow.

sys/sys/buf_ring.h

138

AFAIK 'synchronizes-with chains' are about RMW operations in between load_acq/store_rel sequence. Can you explain why load_acq is necessary here? I think producer should not care about visibility of br->br_ring[prod_head] stores of other producers.
Correct order of our own stores guaranteed by store_rel(&br->br_prod_tail) (and load_acq() in consumer).

[oleg]

1. fix issues reported by sbahra_repnop.org here: https://reviews.freebsd.org/D1945
2. fix synchronization chain (producer -> producer -> consumer) noted by alc.
3. massive comments.