[PATCH v2] eal: add seqlock

[Mattias Rönnblom]

On 2022-04-02 02:52, Stephen Hemminger wrote:
> On Thu, 31 Mar 2022 16:53:00 +0200
> Ola Liljedahl <ola.liljedahl at arm.com> wrote:
> 
>> From: Ola Liljedahl <ola.liljedahl at arm.com>
>> To: Mattias Rönnblom <mattias.ronnblom at ericsson.com>,  "dev at dpdk.org"
>> <dev at dpdk.org> Cc: Thomas Monjalon <thomas at monjalon.net>,  David
>> Marchand <david.marchand at redhat.com>,  Onar Olsen
>> <onar.olsen at ericsson.com>,  "Honnappa.Nagarahalli at arm.com"
>> <Honnappa.Nagarahalli at arm.com>,  "nd at arm.com" <nd at arm.com>,
>> "konstantin.ananyev at intel.com" <konstantin.ananyev at intel.com>,
>> "mb at smartsharesystems.com" <mb at smartsharesystems.com>,
>> "stephen at networkplumber.org" <stephen at 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.

Does "semantics" here including the naming of the functions? The overall 
semantics will be the same, except the kernel has a bunch of variants 
with different kind of write-side synchronization, if I recall correctly.

I'll try to summarize the options as I see them:

Option A: (PATCH v3):
rte_seqlock_read_lock()
rte_seqlock_read_tryunlock() /* with built-in "read restart" */ 
rte_seqlock_write_lock()
rte_seqlock_write_unlock()


Option B: (Linux kernel-style naming):
rte_seqlock_read_begin()
rte_seqlock_read_end()
rte_seqlock_write_begin()
rte_seqlock_write_end()

A combination, acknowledging there's a lock on the writer side, but not 
on the read side.

Option C:

rte_seqlock_read_begin()
rte_seqlock_read_retry()
rte_seqlock_write_lock()
rte_seqlock_write_unlock()