> Hi, > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Juhamatti > > > Kuusisaari > > > Sent: Monday, July 11, 2016 11:21 AM > > > To: dev at dpdk.org > > > Subject: [dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct > > > location > > > > > > Fix the location of the rte_ring data dependency read barrier. > > > It needs to be called before accessing indexed data to ensure that the > > > data itself is guaranteed to be correctly updated. > > > > > > See more details at kernel/Documentation/memory-barriers.txt > > > section 'Data dependency barriers'. > > > > > > Any explanation why? > > From my point smp_rmb()s are on the proper places here :) Konstantin > > The problem here is that on a weak memory model system the CPU is > allowed to load the address data out-of-order in advance. > If the read barrier is after the DEQUEUE, you might end up having the old > data there on a race situation when the buffer is continuously full. > Having it before the DEQUEUE guarantees that the load is not done > in advance.
Sorry, still didn't see any race condition in the current code. Can you provide any particular example? >From other side, moving smp_rmb() before dequeueing the objects, could introduce a race condition, on cpus where later writes can be reordered with earlier reads. Konstantin > > On Intel, it should not matter due to different memory model, so this is > limited to weak memory model systems. > > -- > Juhamatti > > > > > > > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari at coriant.com> > > > --- > > > lib/librte_ring/rte_ring.h | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h > > > index eb45e41..a923e49 100644 > > > --- a/lib/librte_ring/rte_ring.h > > > +++ b/lib/librte_ring/rte_ring.h > > > @@ -662,9 +662,10 @@ __rte_ring_mc_do_dequeue(struct rte_ring *r, > > void **obj_table, > > > cons_next); > > > } while (unlikely(success == 0)); > > > > > > + rte_smp_rmb(); > > > + > > > /* copy in table */ > > > DEQUEUE_PTRS(); > > > - rte_smp_rmb(); > > > > > > /* > > > * If there are other dequeues in progress that preceded us, > > > @@ -746,9 +747,10 @@ __rte_ring_sc_do_dequeue(struct rte_ring *r, > > void **obj_table, > > > cons_next = cons_head + n; > > > r->cons.head = cons_next; > > > > > > + rte_smp_rmb(); > > > + > > > /* copy in table */ > > > DEQUEUE_PTRS(); > > > - rte_smp_rmb(); > > > > > > __RING_STAT_ADD(r, deq_success, n); > > > r->cons.tail = cons_next; > > > -- > > > 2.9.0 > > > > > > > > > > > ========================================================== > > == > > > The information contained in this message may be privileged and > > > confidential and protected from disclosure. If the reader of this > > > message is not the intended recipient, or an employee or agent > > > responsible for delivering this message to the intended recipient, you > > > are hereby notified that any reproduction, dissemination or > > > distribution of this communication is strictly prohibited. If you have > > > received this communication in error, please notify us immediately by > > > replying to the message and deleting it from your computer. Thank you. > > > Coriant-Tellabs > > > > > ========================================================== > > ==