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
> ---
> lib/eal/include/generic/rte_spinlock.h | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/lib/eal/include/generic/rte_spinlock.h
> b/lib/eal/include/generic/rte_spinlock.h
> index c907d4e45c..5d810b682a 100644
> --- a/lib/eal/include/generic/rte_spinlock.h
> +++ b/lib/eal/include/generic/rte_spinlock.h
> @@ -197,8 +197,8 @@ rte_spinlock_trylock_tm(rte_spinlock_t *sl)
> */
> 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;
>
> /**
> @@ -215,7 +215,7 @@ typedef struct {
> static inline void rte_spinlock_recursive_init(rte_spinlock_recursive_t *slr)
> {
> rte_spinlock_init(&slr->sl);
> - slr->user = -1;
> + rte_atomic_store_explicit(&slr->user, -1, rte_memory_order_relaxed);
> slr->count = 0;
> }
>
> @@ -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
> rte_spinlock_lock(&slr->sl);
> - slr->user = id;
> + rte_atomic_store_explicit(&slr->user, id,
> rte_memory_order_relaxed);
And rte_memory_order_release
> }
> slr->count++;
> }
> @@ -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.
I think it should be:
diff --git a/lib/eal/include/generic/rte_spinlock.h
b/lib/eal/include/generic/rte_spinlock.h
index c907d4e45c39..b1058e4f8b4f 100644
--- a/lib/eal/include/generic/rte_spinlock.h
+++ b/lib/eal/include/generic/rte_spinlock.h
@@ -245,11 +247,17 @@ static inline void
rte_spinlock_recursive_lock(rte_spinlock_recursive_t *slr)
static inline void rte_spinlock_recursive_unlock(rte_spinlock_recursive_t *slr)
__rte_no_thread_safety_analysis
{
- if (--(slr->count) == 0) {
- slr->user = -1;
- rte_spinlock_unlock(&slr->sl);
- }
+ int id = rte_gettid();
+ if (rte_atomic_load_explicit(&slr->user, rte_memory_order_acquire) ==
id) {
+ RTE_ASSERT(slr->count > 0);
+ if (--(slr->count) == 0) {
+ rte_atomic_store_explicit(&slr->user, -1,
rte_memory_order_release);
+ rte_spinlock_unlock(&slr->sl);
+ }
+ } else {
+ RTE_ASSERT(id != -1 && "unlocked from another thread");
+ }
}
(not tested)
> - slr->user = -1;
> + rte_atomic_store_explicit(&slr->user, -1,
> rte_memory_order_relaxed);
> rte_spinlock_unlock(&slr->sl);
> }
> -
> }
>
> /**
> @@ -266,10 +265,10 @@ static inline int
> rte_spinlock_recursive_trylock(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) {
rte_memory_order_acquire
> if (rte_spinlock_trylock(&slr->sl) == 0)
> return 0;
> - slr->user = id;
> + rte_atomic_store_explicit(&slr->user, id,
> rte_memory_order_relaxed);
rte_memory_order_release
> }
> slr->count++;
> return 1;
--
Robin
> Monitored by the American Human Association.
