Re: [dpdk-dev] [PATCH 5/5] lib/stack: remove pop cas release ordering

[David Marchand <david.marchand@redhat.com>]

Hello Steven,

On Mon, Sep 21, 2020 at 7:17 PM Eads, Gage <gage.eads@intel.com> wrote:
> > -----Original Message-----
> > From: Steven Lariau <steven.lariau@arm.com>
> > Sent: Friday, September 11, 2020 10:30 AM
> > To: Eads, Gage <gage.eads@intel.com>; Olivier Matz <olivier.matz@6wind.com>
> > Cc: dev@dpdk.org; nd@arm.com; dharmik.thakkar@arm.com; Steven Lariau
> > <steven.lariau@arm.com>
> > Subject: [PATCH 5/5] lib/stack: remove pop cas release ordering
> >
> > Replace the store-release by relaxed for the CAS success at the end of
> > pop. Release isn't needed, because there is not write to data that need
> > to be synchronized.
> > The only preceding write is when the length is decreased, but the length
> > CAS loop already ensures the right synchronization.
> > The situation to avoid is when a thread sees the old length but the new
> > list, that doesn't have enough items for pop to success.
> > But the CAS success on length before the pop loop ensures any core reads
> > and updates the latest length, preventing this situation.
> >
> > The store-release is also used to make sure that the items are read
> > before the head is updated, in order to prevent a core in pop to read an
> > incorrect value because another core rewrites it with push.
> > But this isn't needed, because items are read only when removed from the
> > used list. Right after this, they are pushed to the free list, and the
> > store-release in push makes sure the items are read before they are
> > visible in the free list.
>
> Please add a comment to this effect above the compare-exchange call. Depending
> on this caller behavior for correctness is a little risky, but since this header
> is private to the library I think it's ok as long as it's well-documented.
>

Can you prepare a v2 for this comment?
Thanks.


-- 
David Marchand