18/05/2026 17:14, Robin Jarry:
> Hey Thomas,
>
> Thomas Monjalon, May 18, 2026 at 17:07:
> > When compiling with C++20 standard requirement (default in GCC 16),
> > the increment and decrement of volatile variables are rejected:
> >
> > rte_spinlock.h:241:14: error:
> > '++' expression of 'volatile'-qualified type is deprecated
> > rte_spinlock.h:252:21: error:
> > '--' expression of 'volatile'-qualified type is deprecated
> > rte_spinlock.h:278:14: error:
> > '++' expression of 'volatile'-qualified type is deprecated
> >
> > The count field of rte_spinlock_recursive_t
> > does not need the volatile qualifier
> > because it is only accessed by the thread holding the lock,
> > which already provides the necessary memory ordering.
> >
> > The user field can be accessed outside of the lock,
> > so it must handled as a C11 atomic variable.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: [email protected]
> >
> > Signed-off-by: Thomas Monjalon <[email protected]>
> > ---
> > v1: drop volatile keyword
> > v2: make user an atomic variable
> > ---
> > typedef struct {
> > rte_spinlock_t sl; /**< the actual spinlock */
> > - volatile int user; /**< core id using lock, -1 for unused */
> > - volatile int count; /**< count of time this lock has been called */
> > + RTE_ATOMIC(int) user; /**< core id using lock, -1 for unused */
> > + int count; /**< count of time this lock has been called */
> > } rte_spinlock_recursive_t;
[...]
> > @@ -230,9 +230,9 @@ static inline void
> > rte_spinlock_recursive_lock(rte_spinlock_recursive_t *slr)
> > {
> > int id = rte_gettid();
> >
> > - if (slr->user != id) {
> > + if (rte_atomic_load_explicit(&slr->user, rte_memory_order_relaxed) !=
> > id) {
>
> This needs to be rte_memory_order_acquire
The memory ordering is managed with the sl variable.
The variable user has just to be atomic, so relaxed should be enough,
am I wrong?
[...]
> > @@ -246,10 +246,9 @@ static inline void
> > rte_spinlock_recursive_unlock(rte_spinlock_recursive_t *slr)
> > __rte_no_thread_safety_analysis
> > {
> > if (--(slr->count) == 0) {
>
> This code is completely broken. Any thread can unlock without any check.
Maybe, but I don't intend to fix recursive unlock in this patch.
The subject is "remove volatile qualifier" (and unblock GCC 16).
