Re: [dpdk-dev] [PATCH 1/2] ring: synchronize the load and store of the tail

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: den 27 oktober 2018 17:13
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Cc: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; dev@dpdk.org;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> stable@dpdk.org; Ola Liljedahl <Ola.Liljedahl@arm.com>;
> olivier.matz@6wind.com; chaozhu@linux.vnet.ibm.com;
> bruce.richardson@intel.com; konstantin.ananyev@intel.com
> Subject: Re: [dpdk-dev] [PATCH 1/2] ring: synchronize the load and store of
> the tail
> 
> 27/10/2018 17:00, Jerin Jacob:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 17/10/2018 08:35, Gavin Hu (Arm Technology China):
> > > > Hi Jerin
> > > >
> > > > As the 1st one of the 3-patch set was not concluded, I submit this 2-
> patch series to unblock the merge.
> > >
> > > The thread is totally messed up because:
> > >         - there is no cover letter
> > >         - some different series (testpmd, i40e and doc) are in the same
> thread
> > >         - v4 replies to a different series
> > >         - this version should be a v5 but has no number
> > >         - this version replies to the v3
> > >         - patchwork still shows v3 and "v5"
> > >         - replies from Ola are not quoting previous discussion
> > >
> > > Because of all of this, it is really difficult to follow.
> > > This is probably the reason of the lack of review outside of Arm.
> > >
> > > One more issue: you must Cc the relevant maintainers.
> > > Here:
> > >         - Olivier for rte_ring
> > >         - Chao for IBM platform
> > >         - Bruce and Konstantin for x86
> > >
> > > Guys, it is really cool to have more Arm developpers in DPDK.
> > > But please consider better formatting your discussions, it is really
> > > important in our contribution workflow.
> > >
> > > I don't know what to do.
> > > I suggest to wait for more feedbacks and integrate it in -rc2.
> >
> > This series has been acked and tested. Sure, if we are looking for
> > some more feedback we can push to -rc2 if not it a good candidate to
> > be selected for -rc1.
> 
> It has been acked and tested only for Arm platforms.
> And Olivier, the ring maintainer, was not Cc.
> 
> I feel it is not enough.
> 

I've just run an out-of-tree test program I have for the DSW scheduler, which verify scheduler atomic semantics. The results are:
Non-C11 mode: pass
C11 mode before this patch set: fail
C11 mode after this patch set: pass

This suggests the current C11 mode is broken even on x86_64. I haven't been following this thread closely, so maybe this is known already.

I've also run an out-of-tree DSW throughput benchmark, and I've found that going from Non-C11 to C11 gives a 4% slowdown. After this patch, the slowdown is only 2,8%.

GCC 7.3.0 and a Skylake x86_64.