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

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

> >
> > 27/10/2018 17:00, Jerin Jacob:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 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%.
This is interesting. The general understanding seems to be that C11 atomics should not add any additional instructions on x86. But, we still see some drop in performance. Is this attributed to compiler not being allowed to re-order?

> 
> GCC 7.3.0 and a Skylake x86_64.