[PATCH] spinlock: remove volatile qualifier

[PATCH] spinlock: remove volatile qualifier

[Thomas Monjalon]

The user and count fields of rte_spinlock_recursive_t
do not need the volatile qualifier
because they are only accessed by the thread holding the lock,
which already provides the necessary memory ordering.

Removing volatile aligns with a C++20 deprecation
for increment and decrement of volatile variables.

This issue was seen with GCC 16 which changes the default C++ version
from -std=gnu++17 to -std=gnu++20.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/eal/include/generic/rte_spinlock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
index c907d4e45c..19c0e34f0a 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 */
+	int user; /**< core id using lock, -1 for unused */
+	int count; /**< count of time this lock has been called */
 } rte_spinlock_recursive_t;
 
 /**
-- 
2.54.0

Re: [PATCH] spinlock: remove volatile qualifier

[Thomas Monjalon]

04/05/2026 10:37, Thomas Monjalon:
> The user and count fields of rte_spinlock_recursive_t
> do not need the volatile qualifier
> because they are only accessed by the thread holding the lock,
> which already provides the necessary memory ordering.
> 
> Removing volatile aligns with a C++20 deprecation
> for increment and decrement of volatile variables.
> 
> This issue was seen with GCC 16 which changes the default C++ version
> from -std=gnu++17 to -std=gnu++20.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

I've just found this has been discussed 4 years ago:
https://inbox.dpdk.org/dev/20221221083717.135c3f81@hermes.local

Stephen had found some comments issues that I will fix in another patch.

[PATCH] spinlock: fix API comments

[Thomas Monjalon]

This is not a read-write lock.

The user field stores the thread ID, not the core ID.

The implementation is architecture-specific in some cases only.

Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/eal/include/generic/rte_spinlock.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
index 19c0e34f0a..d9f255f8c4 100644
--- a/lib/eal/include/generic/rte_spinlock.h
+++ b/lib/eal/include/generic/rte_spinlock.h
@@ -7,12 +7,16 @@
 
 /**
  * @file
+ * DPDK spinlocks
  *
- * RTE Spinlocks
+ * This is an API for spinlocks.
+ * This kind of lock simply waits in a loop
+ * repeatedly checking until the lock becomes available.
  *
- * This file defines an API for read-write locks, which are implemented
- * in an architecture-specific way. This kind of lock simply waits in
- * a loop repeatedly checking until the lock becomes available.
+ * Some functions may have an architecture-specific implementation
+ * if RTE_FORCE_INTRINSICS is disabled.
+ * The hardware transactional memory (lock elision) functions have _tm suffix
+ * and are implemented in architecture-specific files.
  *
  * All locks must be initialised before use, and only initialised once.
  */
@@ -197,7 +201,7 @@ rte_spinlock_trylock_tm(rte_spinlock_t *sl)
  */
 typedef struct {
 	rte_spinlock_t sl; /**< the actual spinlock */
-	int user; /**< core id using lock, -1 for unused */
+	int user; /**< thread id using lock, -1 for unused */
 	int count; /**< count of time this lock has been called */
 } rte_spinlock_recursive_t;
 
-- 
2.54.0

Re: [PATCH] spinlock: fix API comments

[Thomas Monjalon]

04/05/2026 11:12, Thomas Monjalon:
> This is not a read-write lock.
> 
> The user field stores the thread ID, not the core ID.
> 
> The implementation is architecture-specific in some cases only.
> 
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Fixes: ca2e2dab079a ("spinlock: support non-EAL thread")
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Re: [PATCH] spinlock: remove volatile qualifier

[Stephen Hemminger]

On Mon,  4 May 2026 10:37:14 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
> index c907d4e45c..19c0e34f0a 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 */
> +	int user; /**< core id using lock, -1 for unused */
> +	int count; /**< count of time this lock has been called */

It might make sense to use unsigned for count, and sized types.
I.e do you really need 32 bit values?

Only in tree use of recursive spinlock in vdev code.

PS: I wonder if we really need to keep the transactional memory versions of stuff.
I 

Re: [PATCH] spinlock: remove volatile qualifier

[Thomas Monjalon]

04/05/2026 19:02, Stephen Hemminger:
> On Mon,  4 May 2026 10:37:14 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
> > index c907d4e45c..19c0e34f0a 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 */
> > +	int user; /**< core id using lock, -1 for unused */
> > +	int count; /**< count of time this lock has been called */
> 
> It might make sense to use unsigned for count, and sized types.
> I.e do you really need 32 bit values?

Such a change would need to be evaluated in a performance benchmark.

> Only in tree use of recursive spinlock in vdev code.
> 
> PS: I wonder if we really need to keep the transactional memory versions of stuff.

I don't know, it's part of the API and has no maintenance cost.

Re: [PATCH] spinlock: remove volatile qualifier

[Stephen Hemminger]

On Tue, 05 May 2026 09:01:43 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> > PS: I wonder if we really need to keep the transactional memory versions of stuff.  
> 
> I don't know, it's part of the API and has no maintenance cost.


It is not tested, only worked on x86 and most of the CPU's and tooling
dropped support for transactional memory years ago. It was a failed idea.

Re: [PATCH] spinlock: remove volatile qualifier

[Thomas Monjalon]

04/05/2026 11:06, Thomas Monjalon:
> 04/05/2026 10:37, Thomas Monjalon:
> > The user and count fields of rte_spinlock_recursive_t
> > do not need the volatile qualifier
> > because they are only accessed by the thread holding the lock,
> > which already provides the necessary memory ordering.
> > 
> > Removing volatile aligns with a C++20 deprecation
> > for increment and decrement of volatile variables.
> > 
> > This issue was seen with GCC 16 which changes the default C++ version
> > from -std=gnu++17 to -std=gnu++20.
> > 
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> I've just found this has been discussed 4 years ago:
> https://inbox.dpdk.org/dev/20221221083717.135c3f81@hermes.local
> 
> Stephen had found some comments issues that I will fix in another patch.

Adding the compiler messages:

rte_spinlock.h:241:14: error: '++' expression of 'volatile'-qualified type is deprecated [-Werror=volatile]
rte_spinlock.h:252:21: error: '--' expression of 'volatile'-qualified type is deprecated [-Werror=volatile]
rte_spinlock.h:278:14: error: '++' expression of 'volatile'-qualified type is deprecated [-Werror=volatile]

[PATCH v2 1/2] spinlock: remove volatile qualifier

[Thomas Monjalon]

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: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
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) {
 		rte_spinlock_lock(&slr->sl);
-		slr->user = id;
+		rte_atomic_store_explicit(&slr->user, id, rte_memory_order_relaxed);
 	}
 	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) {
-		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) {
 		if (rte_spinlock_trylock(&slr->sl) == 0)
 			return 0;
-		slr->user = id;
+		rte_atomic_store_explicit(&slr->user, id, rte_memory_order_relaxed);
 	}
 	slr->count++;
 	return 1;
-- 
2.54.0

[PATCH v2 2/2] spinlock: fix API comments

[Thomas Monjalon]

This is not a read-write lock.

The user field stores the thread ID, not the core ID
since a change in DPDK 2.0.0.

The implementation is architecture-specific in some cases only.

Fixes: ca2e2dab079a ("spinlock: support non-EAL thread")
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/eal/include/generic/rte_spinlock.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
index 5d810b682a..5c46a7a90f 100644
--- a/lib/eal/include/generic/rte_spinlock.h
+++ b/lib/eal/include/generic/rte_spinlock.h
@@ -7,12 +7,16 @@
 
 /**
  * @file
+ * DPDK spinlocks
  *
- * RTE Spinlocks
+ * This is an API for spinlocks.
+ * This kind of lock simply waits in a loop
+ * repeatedly checking until the lock becomes available.
  *
- * This file defines an API for read-write locks, which are implemented
- * in an architecture-specific way. This kind of lock simply waits in
- * a loop repeatedly checking until the lock becomes available.
+ * Some functions may have an architecture-specific implementation
+ * if RTE_FORCE_INTRINSICS is disabled.
+ * The hardware transactional memory (lock elision) functions have _tm suffix
+ * and are implemented in architecture-specific files.
  *
  * All locks must be initialised before use, and only initialised once.
  */
@@ -197,7 +201,7 @@ rte_spinlock_trylock_tm(rte_spinlock_t *sl)
  */
 typedef struct {
 	rte_spinlock_t sl; /**< the actual spinlock */
-	RTE_ATOMIC(int) user; /**< core id using lock, -1 for unused */
+	RTE_ATOMIC(int) user; /**< thread id using lock, -1 for unused */
 	int count; /**< count of time this lock has been called */
 } rte_spinlock_recursive_t;
 
-- 
2.54.0

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[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: stable@dpdk.org
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> 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.

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[Thomas Monjalon]

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: stable@dpdk.org
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > 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).

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[Bruce Richardson]

On Mon, May 18, 2026 at 05:25:43PM +0200, Thomas Monjalon wrote:
> 18/05/2026 17:14, Robin Jarry:
> > Hey Thomas,
> > 
> > Thomas Monjalon, May 18, 2026 at 17:07:

<snip>

> 
> [...]
> > > @@ -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).
> 
As with regular mutexes, threads should not go around unlocking when not
holding the lock. Therefore, I think this is fine. [With regular spinlocks
we can't check since we don't track threads, therefore I think we just need
to assume a well-behaved app.]

/Bruce

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[Stephen Hemminger]

On Mon, 18 May 2026 17:14:54 +0200
"Robin Jarry" <rjarry@redhat.com> wrote:

> 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: stable@dpdk.org
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > 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

No it does not. There is no dependency here.
The acquire is inside the spinlock.

> 
> >  		rte_spinlock_lock(&slr->sl);
> > -		slr->user = id;
> > +		rte_atomic_store_explicit(&slr->user, id, rte_memory_order_relaxed);  
> 
> And rte_memory_order_release

Ditto.

> 
> >  	}
> >  	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 would make an RTE_ASSERT() that id matched current thread id.
Since caller holds lock, no atomic required for that.

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[Stephen Hemminger]

On Mon, 18 May 2026 17:07:35 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 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: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

I would also suggest renaming 'user' to 'owner' to follow common
practice in other places. It also provides an intentional API
break if somebody is misguided enough to do direct access to the
structure field.

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[Robin Jarry]

Bruce Richardson, May 18, 2026 at 17:28:
> On Mon, May 18, 2026 at 05:25:43PM +0200, Thomas Monjalon wrote:
>> 18/05/2026 17:14, Robin Jarry:
>> > Hey Thomas,
>> > 
>> > Thomas Monjalon, May 18, 2026 at 17:07:
>
> <snip>
>
>> 
>> [...]
>> > > @@ -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).
>> 
> As with regular mutexes, threads should not go around unlocking when not
> holding the lock. Therefore, I think this is fine. [With regular spinlocks
> we can't check since we don't track threads, therefore I think we just need
> to assume a well-behaved app.]

Ok, fair enough :)

As Stephen suggested, maybe we could add some assertions?

 static inline void rte_spinlock_recursive_unlock(rte_spinlock_recursive_t *slr)
        __rte_no_thread_safety_analysis
 {
+       RTE_ASSERT(rte_atomic_load_explicit(&slr->user, rte_memory_order_relaxed) == rte_gettid());
+       RTE_ASSERT(slr->count > 0);
+

[PATCH v3 1/3] spinlock: remove volatile qualifier

[Thomas Monjalon]

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.
The name is also changed from user to owner.
It will break if an application is accessing this field directly,
which should never happen.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v1: drop volatile keyword
v2: make user an atomic variable
v3: rename user as owner
---
 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..ffdcb8fa3d 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) owner; /**< 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->owner, -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->owner, rte_memory_order_relaxed) != id) {
 		rte_spinlock_lock(&slr->sl);
-		slr->user = id;
+		rte_atomic_store_explicit(&slr->owner, id, rte_memory_order_relaxed);
 	}
 	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) {
-		slr->user = -1;
+		rte_atomic_store_explicit(&slr->owner, -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->owner, rte_memory_order_relaxed) != id) {
 		if (rte_spinlock_trylock(&slr->sl) == 0)
 			return 0;
-		slr->user = id;
+		rte_atomic_store_explicit(&slr->owner, id, rte_memory_order_relaxed);
 	}
 	slr->count++;
 	return 1;
-- 
2.54.0

[PATCH v3 2/3] spinlock: add debug checks in recursive unlock

[Thomas Monjalon]

The recursive unlock assumes that the caller owns the lock.
This behavior will be checked when RTE_ENABLE_ASSERT is on.
There is an additional check for the count which should be positive
if no corruption happened.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v3: new patch in the series
---
 lib/eal/include/generic/rte_spinlock.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
index ffdcb8fa3d..e7cd18a8e2 100644
--- a/lib/eal/include/generic/rte_spinlock.h
+++ b/lib/eal/include/generic/rte_spinlock.h
@@ -21,6 +21,7 @@
 #ifdef RTE_FORCE_INTRINSICS
 #include <rte_common.h>
 #endif
+#include <rte_debug.h>
 #include <rte_lock_annotations.h>
 #include <rte_pause.h>
 #include <rte_stdatomic.h>
@@ -245,6 +246,8 @@ 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
 {
+	RTE_ASSERT(rte_atomic_load_explicit(&slr->owner, rte_memory_order_relaxed) == rte_gettid());
+	RTE_ASSERT(slr->count > 0);
 	if (--(slr->count) == 0) {
 		rte_atomic_store_explicit(&slr->owner, -1, rte_memory_order_relaxed);
 		rte_spinlock_unlock(&slr->sl);
-- 
2.54.0

[PATCH v3 3/3] spinlock: fix API comments

[Thomas Monjalon]

This is not a read-write lock.

The user field stores the thread ID, not the core ID
since a change in DPDK 2.0.0.

The implementation is architecture-specific in some cases only.

Fixes: ca2e2dab079a ("spinlock: support non-EAL thread")
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/eal/include/generic/rte_spinlock.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
index e7cd18a8e2..dd3d2d046c 100644
--- a/lib/eal/include/generic/rte_spinlock.h
+++ b/lib/eal/include/generic/rte_spinlock.h
@@ -7,12 +7,16 @@
 
 /**
  * @file
+ * DPDK spinlocks
  *
- * RTE Spinlocks
+ * This is an API for spinlocks.
+ * This kind of lock simply waits in a loop
+ * repeatedly checking until the lock becomes available.
  *
- * This file defines an API for read-write locks, which are implemented
- * in an architecture-specific way. This kind of lock simply waits in
- * a loop repeatedly checking until the lock becomes available.
+ * Some functions may have an architecture-specific implementation
+ * if RTE_FORCE_INTRINSICS is disabled.
+ * The hardware transactional memory (lock elision) functions have _tm suffix
+ * and are implemented in architecture-specific files.
  *
  * All locks must be initialised before use, and only initialised once.
  */
@@ -198,7 +202,7 @@ rte_spinlock_trylock_tm(rte_spinlock_t *sl)
  */
 typedef struct {
 	rte_spinlock_t sl; /**< the actual spinlock */
-	RTE_ATOMIC(int) owner; /**< core id using lock, -1 for unused */
+	RTE_ATOMIC(int) owner; /**< thread id owning lock, -1 for unused */
 	int count; /**< count of time this lock has been called */
 } rte_spinlock_recursive_t;
 
-- 
2.54.0

Re: [PATCH v2 1/2] spinlock: remove volatile qualifier

[Thomas Monjalon]

19/05/2026 11:17, Robin Jarry:
> Bruce Richardson, May 18, 2026 at 17:28:
> > On Mon, May 18, 2026 at 05:25:43PM +0200, Thomas Monjalon wrote:
> >> 18/05/2026 17:14, Robin Jarry:
> >> > Thomas Monjalon, May 18, 2026 at 17:07:
> >> > > @@ -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).
> >> 
> > As with regular mutexes, threads should not go around unlocking when not
> > holding the lock. Therefore, I think this is fine. [With regular spinlocks
> > we can't check since we don't track threads, therefore I think we just need
> > to assume a well-behaved app.]
> 
> Ok, fair enough :)
> 
> As Stephen suggested, maybe we could add some assertions?
> 
>  static inline void rte_spinlock_recursive_unlock(rte_spinlock_recursive_t *slr)
>         __rte_no_thread_safety_analysis
>  {
> +       RTE_ASSERT(rte_atomic_load_explicit(&slr->user, rte_memory_order_relaxed) == rte_gettid());
> +       RTE_ASSERT(slr->count > 0);
> +

OK I add it as a separate patch in v3.

Re: [PATCH v3 1/3] spinlock: remove volatile qualifier

[Bruce Richardson]

On Tue, May 19, 2026 at 12:34:14PM +0200, Thomas Monjalon wrote:
> 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.
> The name is also changed from user to owner.
> It will break if an application is accessing this field directly,
> which should never happen.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v1: drop volatile keyword
> v2: make user an atomic variable
> v3: rename user as owner
> ---
>  lib/eal/include/generic/rte_spinlock.h | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v3 2/3] spinlock: add debug checks in recursive unlock

[Thomas Monjalon]

19/05/2026 12:34, Thomas Monjalon:
> The recursive unlock assumes that the caller owns the lock.
> This behavior will be checked when RTE_ENABLE_ASSERT is on.
> There is an additional check for the count which should be positive
> if no corruption happened.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v3: new patch in the series

Forgot:

Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Suggested-by: Robin Jarry <rjarry@redhat.com>

Re: [PATCH v3 2/3] spinlock: add debug checks in recursive unlock

[Robin Jarry]

Thomas Monjalon, May 19, 2026 at 12:34:
> The recursive unlock assumes that the caller owns the lock.
> This behavior will be checked when RTE_ENABLE_ASSERT is on.
> There is an additional check for the count which should be positive
> if no corruption happened.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v3: new patch in the series
> ---
>  lib/eal/include/generic/rte_spinlock.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
> index ffdcb8fa3d..e7cd18a8e2 100644
> --- a/lib/eal/include/generic/rte_spinlock.h
> +++ b/lib/eal/include/generic/rte_spinlock.h
> @@ -21,6 +21,7 @@
>  #ifdef RTE_FORCE_INTRINSICS
>  #include <rte_common.h>
>  #endif
> +#include <rte_debug.h>
>  #include <rte_lock_annotations.h>
>  #include <rte_pause.h>
>  #include <rte_stdatomic.h>
> @@ -245,6 +246,8 @@ 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
>  {
> +	RTE_ASSERT(rte_atomic_load_explicit(&slr->owner, rte_memory_order_relaxed) == rte_gettid());
> +	RTE_ASSERT(slr->count > 0);
>  	if (--(slr->count) == 0) {
>  		rte_atomic_store_explicit(&slr->owner, -1, rte_memory_order_relaxed);
>  		rte_spinlock_unlock(&slr->sl);

Acked-by: Robin Jarry <rjarry@redhat.com>

Re: [PATCH v3 1/3] spinlock: remove volatile qualifier

[Stephen Hemminger]

On Tue, 19 May 2026 12:34:14 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 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.
> The name is also changed from user to owner.
> It will break if an application is accessing this field directly,
> which should never happen.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed

Series-acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH v3 1/3] spinlock: remove volatile qualifier

[David Marchand]

On Tue, 19 May 2026 at 12:37, Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 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.
> The name is also changed from user to owner.
> It will break if an application is accessing this field directly,
> which should never happen.
>
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Series applied, thanks.


-- 
David Marchand