Date: Mon, 5 Dec 2016 17:50:24 +0000 From: "alc (Alan Cox)" <phabric-noreply@FreeBSD.org> To: freebsd-net@freebsd.org Subject: [Differential] D8637: buf_ring.h: fix memory order issues. Message-ID: <b1d8337448f23239e3644d85081c2aca@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-7phoe4ltzb7cywaexwdr-req@FreeBSD.org> References: <differential-rev-PHID-DREV-7phoe4ltzb7cywaexwdr-req@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
alc added a comment. 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. INLINE COMMENTS > buf_ring.h:78 > do { > + cons_tail = atomic_load_acq_32(&br->br_cons_tail); > prod_head = br->br_prod_head; Was there a reason for moving this load? > buf_ring.h:98 > */ > while (br->br_prod_tail != prod_head) > cpu_spinwait(); 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. > buf_ring.h:117 > do { > + prod_tail = atomic_load_acq_32(&br->br_prod_tail); > cons_head = br->br_cons_head; Was there a reason for moving this load? > buf_ring.h:159 > prod_tail = atomic_load_acq_32(&br->br_prod_tail); > - > - cons_next = (cons_head + 1) & br->br_cons_mask; > -#ifdef PREFETCH_DEFINED > - cons_next_next = (cons_head + 2) & br->br_cons_mask; > -#endif > + cons_head = br->br_cons_head; > Was there a reason for swapping the order of the preceding loads? > buf_ring.h:174 > buf = br->br_ring[cons_head]; > + br->br_cons_head = cons_next; > Was there a reason for swapping the order of the preceding loads? 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-listhelp
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b1d8337448f23239e3644d85081c2aca>