[Differential] D8637: buf_ring.h: fix memory order issues.

Tue Dec 6 00:54:15 UTC 2016

oleg added a comment.

  In https://reviews.freebsd.org/D8637#180625, @alc wrote:

  > Have you looked at https://reviews.freebsd.org/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 https://reviews.freebsd.org/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.

INLINE COMMENTS

> alc wrote in buf_ring.h:98
> 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.

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).

REVISION DETAIL
  https://reviews.freebsd.org/D8637

EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: oleg, kmacy, kib, alc
Cc: emaste, freebsd-net-list