[PATCH v2] eal: add seqlock

[Ola Liljedahl]

(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.

>
> What I don't like so much with "tryunlock" is that it's not obvious what
> return type and values it should have. I seem not to be the only one
> which suffers from a lack of intuition here, since the DPDK spinlock
> trylock() function returns '1' in case lock is taken (using an int, but
> treating it like a bool), while the rwlock equivalent returns '0' (also
> int, but treating it as an error code).
Then you have two different ways of doing it! Or invent a third since 
there seems to be no consistent pattern.
>
> "lock" also suggests you prevent something from occurring, which is not
> the case on the reader side.

That's why my implementations in Progress64 use the terms acquire and 
release. Locks are acquired and released (with acquire and release 
semantics!). Hazard pointers are acquired and released (with acquire and 
release semantics!). Slots in reorder buffers are acquired and released. 
Etc.

https://github.com/ARM-software/progress64

>   A calling application also need not call
> the reader unlock (or retry) function for all seqlocks it has locked,
> although I don't see a point why it wouldn't. (I don't see why a
> read-side critical section should contain much logic at all, since you
> can't act on the just-read data.)

Lock without unlock/retry is meaningless and not something we need to 
consider IMO.


- Ola