Re: [PATCH v3 6/8] stack: add C11 atomic implementation

["Eads, Gage" <gage.eads@intel.com>]

> -----Original Message-----
> From: Eads, Gage
> Sent: Friday, March 29, 2019 2:25 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> dev@dpdk.org
> Cc: olivier.matz@6wind.com; arybchenko@solarflare.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; nd <nd@arm.com>; thomas@monjalon.net; nd
> <nd@arm.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: RE: [PATCH v3 6/8] stack: add C11 atomic implementation
> 
> [snip]
> 
> > > +static __rte_always_inline void
> > > +__rte_stack_lf_push(struct rte_stack_lf_list *list,
> > > +		    struct rte_stack_lf_elem *first,
> > > +		    struct rte_stack_lf_elem *last,
> > > +		    unsigned int num)
> > > +{
> > > +#ifndef RTE_ARCH_X86_64
> > > +	RTE_SET_USED(first);
> > > +	RTE_SET_USED(last);
> > > +	RTE_SET_USED(list);
> > > +	RTE_SET_USED(num);
> > > +#else
> > > +	struct rte_stack_lf_head old_head;
> > > +	int success;
> > > +
> > > +	old_head = list->head;
> > This can be a torn read (same as you have mentioned in
> > __rte_stack_lf_pop). I suggest we use acquire thread fence here as
> > well (please see the comments in __rte_stack_lf_pop).
> 
> Agreed. I'll add the acquire fence.
> 

On second thought, an acquire fence isn't necessary. The acquire fence in __rte_stack_lf_pop() ensures the list->head is ordered before the list element reads. That isn't necessary here; we need to ensure that the last->next write occurs (and is observed) before the list->head write, which the CAS's RELEASE success memorder accomplishes.

If a torn read occurs, the CAS will fail and will atomically re-load &old_head.