Date: Thu, 19 Feb 2015 18:15:22 +0000 From: "zbb (Zbigniew Bodek)" <phabric-noreply@FreeBSD.org> To: freebsd-arm@freebsd.org Subject: [Differential] [Commented On] D1833: Add memory barriers to buf_ring Message-ID: <1a584d41a402fc7b4b3b6a1c33ee36cb@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-ntnotl4wj5wcj2eoukly-req@FreeBSD.org> References: <differential-rev-PHID-DREV-ntnotl4wj5wcj2eoukly-req@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
zbb added a comment. >From W. Macek: It was observed on ARM Cortex-A15, that sometimes, the buf_ring_dequeue_sc finction returned the old pointer, which left there from the previous buffer ring pass. With this fix, the issue disappeared. Inside buf_ring_dequeue_sc, after the cons_head is read, the CPU memory access predictors are capable of preloading the values from the memory for the future use - like a br->br_ring[cons_head], for example. However, this value might not be valid. There is a small "hole" in the logic, between line 165 and 166. If the enqueue function puts the new entry to the ring just after the core prefetches the buf = br->br_ring[cons_head] and before the br->br_prod_tail is read, it will return the wrong, old, value. To prevent this, full memmory barrier must be added before reading br->br_ring[cons_head]. However, that change would negatively impact x86, so it's better to use atomic_load_acq_32 on br->br_prod_tail variable: on Intel it will not change anything (except adding compiler barrier), but on ARM it puts a full dmb() after the br->br_prod_tail is read. As Andrew suggested, the atomic operation was also added to br->br_cons_head variable. REVISION DETAIL https://reviews.freebsd.org/D1833 To: zbb, imp, kmacy, rpaulo Cc: andrew, ian, adrian, freebsd-arm
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1a584d41a402fc7b4b3b6a1c33ee36cb>