[dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location

[Kuusisaari, Juhamatti]

Hello,

> >>> -----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.
> >
> > On Intel, it should not matter due to different memory model, so this
> > is limited to weak memory model systems.
> 
> 
> I agree with Juhamatti. To me, the reading of consumer_head must occur
> before the reading of objects ptrs.
> 
> That was the case before, and this is something I already noticed when I sent
> that mail:
> http://dpdk.org/ml/archives/dev/2014-March/001742.html
> 
> At that time, only Intel CPUs were supported, so it did not make any
> difference.
> 
> Juhamatti, do you have a setup where you can trigger the issue or is it
> something you've seen by code review?

This was found on a code review when we investigated a problem that could have 
caused issues that this kind of bug would introduce. I suppose one would be 
able to see this with very short ring queue lengths and high load, but it depends
on the HW used of course too. 

BR,
--
 Juhamatti

> Thanks,
> Olivier