Re: [PATCH v2] eal: add seqlock

From: Stephen Hemminger <stephen@networkplumber.org>
To: Ola Liljedahl <ola.liljedahl@arm.com>
Cc: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Thomas Monjalon" <thomas@monjalon.net>,
	"David Marchand" <david.marchand@redhat.com>,
	"Onar Olsen" <onar.olsen@ericsson.com>,
	"Honnappa.Nagarahalli@arm.com" <Honnappa.Nagarahalli@arm.com>,
	"nd@arm.com" <nd@arm.com>,
	"konstantin.ananyev@intel.com" <konstantin.ananyev@intel.com>,
	"mb@smartsharesystems.com" <mb@smartsharesystems.com>
Subject: Re: [PATCH v2] eal: add seqlock
Date: Fri, 1 Apr 2022 14:52:17 -1000	[thread overview]
Message-ID: <20220401145217.66506c78@shemminger-XPS-13-9360> (raw)
In-Reply-To: <1094901f-ee27-8c49-d71f-825ec5bfe2ad@arm.com>

On Thu, 31 Mar 2022 16:53:00 +0200
Ola Liljedahl <ola.liljedahl@arm.com> wrote:

> From: Ola Liljedahl <ola.liljedahl@arm.com>
> To: Mattias Rönnblom <mattias.ronnblom@ericsson.com>,  "dev@dpdk.org"
> <dev@dpdk.org> Cc: Thomas Monjalon <thomas@monjalon.net>,  David
> Marchand <david.marchand@redhat.com>,  Onar Olsen
> <onar.olsen@ericsson.com>,  "Honnappa.Nagarahalli@arm.com"
> <Honnappa.Nagarahalli@arm.com>,  "nd@arm.com" <nd@arm.com>,
> "konstantin.ananyev@intel.com" <konstantin.ananyev@intel.com>,
> "mb@smartsharesystems.com" <mb@smartsharesystems.com>,
> "stephen@networkplumber.org" <stephen@networkplumber.org> Subject:
> Re: [PATCH v2] eal: add seqlock Date: Thu, 31 Mar 2022 16:53:00 +0200
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
> Thunderbird/91.7.0
> 
> (Thunderbird suddenly refuses to edit in plain text mode, hope the
> mail gets sent as text anyway)
> 
> On 3/31/22 15:38, Mattias Rönnblom wrote:
> 
> > On 2022-03-31 11:04, Ola Liljedahl wrote:  
> >> On 3/31/22 09:46, Mattias Rönnblom wrote:  
> >>> On 2022-03-30 16:26, Mattias Rönnblom wrote:
> >>>  
> <snip>
> >>> Should the rte_seqlock_read_retry() be called
> >>> rte_seqlock_read_end(), or some third alternative? I wanted to
> >>> make clear it's not just a "release the lock" function. You could
> >>> use the|||__attribute__((warn_unused_result)) annotation to make
> >>> clear the return value cannot be ignored, although I'm not sure
> >>> DPDK ever use that attribute.  
> >> We have to decide how to use the seqlock API from the application
> >> perspective.
> >> Your current proposal:
> >> do {
> >>       sn = rte_seqlock_read_begin(&seqlock)
> >>       //read protected data
> >> } while (rte_seqlock_read_retry(&seqlock, sn));
> >>
> >> or perhaps
> >> sn = rte_seqlock_read_lock(&seqlock);
> >> do {
> >>       //read protected data
> >> } while (!rte_seqlock_read_tryunlock(&seqlock, &sn));
> >>
> >> Tryunlock should signal to the user that the unlock operation
> >> might not succeed and something needs to be repeated.
> >>  
> > I like that your proposal is consistent with rwlock API, although I
> > tend to think about a seqlock more like an arbitrary-size atomic
> > load/store, where begin() is the beginning of the read transaction.
> >  
> 
> I can see the evolution of an application where is starts to use
> plain spin locks, moves to reader/writer locks for better performance
> and eventually moves to seqlocks. The usage is the same, only the 
> characteristics (including performance) differ.

The semantics of seqlock in DPDK must be the same as what Linux kernel
does or you are asking for trouble.  It is not a reader-writer lock in
traditional sense.