Re: [PATCH v2] eal: add seqlock

[Ola Liljedahl <ola.liljedahl@arm.com>]

On 4/2/22 02:50, Stephen Hemminger wrote:
> On Wed, 30 Mar 2022 16:26:02 +0200
> Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:
>
>> +	__atomic_store_n(&seqlock->sn, sn, __ATOMIC_RELAXED);
>> +
>> +	/* __ATOMIC_RELEASE to prevent stores after (in program
>> order)
>> +	 * from happening before the sn store.
>> +	 */
>> +	rte_atomic_thread_fence(__ATOMIC_RELEASE);
> Couldn't atomic store with __ATOMIC_RELEASE do same thing?

No, store-release wouldn't prevent later stores from moving up. It only 
ensures that earlier loads and stores have completed before 
store-release completes. If later stores could move before a supposed 
store-release(seqlock->sn), readers could see inconsistent (torn) data 
with a valid sequence number.


>> +static inline void
>> +rte_seqlock_write_end(rte_seqlock_t *seqlock)
>> +{
>> +	uint32_t sn;
>> +
>> +	sn = seqlock->sn + 1;
>> +
>> +	/* synchronizes-with the load acquire in rte_seqlock_begin()
>> */
>> +	__atomic_store_n(&seqlock->sn, sn, __ATOMIC_RELEASE);
>> +
>> +	rte_spinlock_unlock(&seqlock->lock);
> Atomic store is not necessary here, the atomic operation in
> spinlock_unlock wil assure theat the seqeuence number update is
> ordered correctly.
Load-acquire(seqlock->sn) in rte_seqlock_begin() must be paired with 
store-release(seqlock->sn) in rte_seqlock_write_end() or there wouldn't 
exist any synchronize-with relationship. Readers don't access the spin 
lock so any writer-side updates to the spin lock don't mean anything to 
readers.