Re: [dpdk-dev] [PATCH v2] ring: enforce reading the tails before ring operations

["Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Saturday, March 9, 2019 7:48 AM
> To: thomas@monjalon.net; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>
> Cc: Ilya Maximets <i.maximets@samsung.com>; dev@dpdk.org; nd
> <nd@arm.com>; jerinj@marvell.com; hemant.agrawal@nxp.com;
> Nipun.gupta@nxp.com; olivier.matz@6wind.com; Richardson, Bruce
> <bruce.richardson@intel.com>; chaozhu@linux.vnet.ibm.com; nd
> <nd@arm.com>
> Subject: RE: [PATCH v2] ring: enforce reading the tails before ring
> operations
> 
> > 08/03/2019 16:50, Ananyev, Konstantin:
> > > 08/03/2019 16:05, Gavin Hu (Arm Technology China):
> > > > Anyway, on x86, smp_rmb, as a compiler barrier, applies to
> load/store, not
> > only load/load.
> > >
> > > Yes, that's true, but I think that's happened by coincidence, not
> > > intentionally.
> > >
> > > > This is the case also for arm, arm64, ppc32, ppc64.
> > > > I will submit a patch to expand the definition of this API.
> > >
> > > I understand your intention, but that does mean we would also need
> to
> > > change not only rte_smp_rmb() but rte_rmb() too (to keep things
> consistent)?
> > > That sounds worring.
> > > Might be better to keep smp_rmb() definition as it is, and introduce
> > > new function that fits your purposes (smp_rwmb or
> smp_load_store_barrier)?
> Looking at rte_rmb, rte_io_rmb, rte_cio_rmb implementations for Arm,
> they all provide load/store barrier as well. If other architectures also
> provide load/store barrier with rte_xxx_rmb, then we could extend the
> meaning of the existing APIs.
Further looking at rte_rmb, rte_io_rmb, rte_cio_rmb implementations for PPC64 and x86,
They also provide load/store barrier. It is safe to extend the meaning of the existing rte_XXX_rmb API.
> 
> Even if a new API is provided, we need to do provide the same APIs for IO
> and CIO variants.
Since rte_XXX_rmbs API for all architectures already provide the desired load/store ordering,
a new API is redundant and not needed. 

> >
> > How is it managed in other projects?
> In my experience, I usually have been changing the algorithms to use C11
> memory model. So, I have not come across this issue yet. Others can
> comment.
> 
> >