Hi lads, > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob at caviumnetworks.com>: > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > >> > > Consumer queue dequeuing must be guaranteed to be done fully > > >> > > before the tail is updated. This is not guaranteed with a read > > >> > > barrier, changed to a write barrier just before tail update which in > practice guarantees correct order of reads and writes. > > >> > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > >> > > <juhamatti.kuusisaari at coriant.com> > > >> > > > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com> > > >> > > >> Applied, thanks > > > > > > There was ongoing discussion on this > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > Sorry Jerin, I forgot this email. > > The problem is that nobody replied to your email and you did not nack > > the v2 of this patch.
It's probably my bad. I acked the patch before Jerin response, and forgot to reply later. > > > > > This change may not be required as it has the performance impact. > > > > We need to clearly understand what is the performance impact (numbers > > and use cases) on one hand, and is there a real bug fixed by this > > patch on the other hand? > > IHMO, there is no real bug here. rte_smb_rmb() provides the LOAD-STORE > barrier to make sure tail pointer WRITE happens only after prior > LOADS. Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > Thoughts? Wonder how big is a performance impact? If there is a real one, I suppose we can revert the patch? Konstantin > > > > > Please guys make things clear and we'll revert if needed.
