Re: [PATCH v2] eal: add seqlock

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2022-04-05 22:16, Stephen Hemminger wrote:
> On Wed, 30 Mar 2022 16:26:02 +0200
> Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:
> 
>> +/**
>> + * A static seqlock initializer.
>> + */
>> +#define RTE_SEQLOCK_INITIALIZER { 0, RTE_SPINLOCK_INITIALIZER }
> 
> Used named field initializers here please.
> 

OK.

>> +/**
>> + * Initialize the seqlock.
>> + *
>> + * This function initializes the seqlock, and leaves the writer-side
>> + * spinlock unlocked.
>> + *
>> + * @param seqlock
>> + *   A pointer to the seqlock.
>> + */
>> +__rte_experimental
>> +void
>> +rte_seqlock_init(rte_seqlock_t *seqlock);
> 
> You need to add the standard experimental prefix
> to the comment so that doxygen marks the API as experimental
> in documentation.
> 

OK.

>> +static inline bool
>> +rte_seqlock_read_retry(const rte_seqlock_t *seqlock, uint32_t begin_sn)
>> +{
>> +	uint32_t end_sn;
>> +
>> +	/* make sure the data loads happens before the sn load */
>> +	rte_atomic_thread_fence(__ATOMIC_ACQUIRE);
>> +
>> +	end_sn = __atomic_load_n(&seqlock->sn, __ATOMIC_RELAXED);
>> +
>> +	return unlikely(begin_sn & 1 || begin_sn != end_sn);
> 
> Please add parenthesis around the and to test if odd.
> It would be good to document why if begin_sn is odd it returns false.
> 
> 

Will do. Thanks.