Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64

[Ruifeng Wang <Ruifeng.Wang@arm.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, April 29, 2021 11:06 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> david.marchand@redhat.com; thomas@monjalon.net; jerinj@marvell.com;
> nd <nd@arm.com>; Gavin Hu <Gavin.Hu@arm.com>; Steve Capper
> <Steve.Capper@arm.com>; Ola Liljedahl <Ola.Liljedahl@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail
> update on aarch64
> 
> On Thu, 29 Apr 2021 14:35:35 +0000
> Ruifeng Wang <Ruifeng.Wang@arm.com> wrote:
> 
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Thursday, April 29, 2021 1:17 AM
> > > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > > Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> > > Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> > > david.marchand@redhat.com; thomas@monjalon.net;
> jerinj@marvell.com;
> > > nd <nd@arm.com>; Gavin Hu <Gavin.Hu@arm.com>; Steve Capper
> > > <Steve.Capper@arm.com>; Ola Liljedahl <Ola.Liljedahl@arm.com>
> > > Subject: Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for
> > > ring tail update on aarch64
> > >
> > > On Sun, 25 Apr 2021 05:56:53 +0000
> > > Ruifeng Wang <ruifeng.wang@arm.com> wrote:
> > >
> > > > Instead of polling for tail to be updated, use wfe instruction.
> > > >
> > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > >
> > > Looks ok to me, but it does raise an interesting question.
> > > Shouldn't the original code have been using atomic load to look at ht->tail.
> > >
> > > This another place where "volatile considered harmful" applies.
> >
> > Do you mean 'volatile' should be removed from rte_wait_until_equal_xxx
> parameters?
> >
> 
> I meant that all access to tail should be via C11 atomic builtin. At that point,
> the volatile on the data structure elements does not matter.

Agreed. If synchronization is ensured by using C11 atomic builtin, 'volatile' on elements can be removed.