[PATCH v3 0/4] Switch __DECLARE_TRACE() to notrace variant of SRCU-fast

[PATCH v3 0/4] Switch __DECLARE_TRACE() to notrace variant of SRCU-fast

[Paul E. McKenney]

Hello!

This is version 3 of a patch series creating a new notrace variant of
SRCU-fast and introducing it to the __DECLARE_TRACE() in place of the
current preemption disabling.  This change enable preemption of BPF
programs attached to tracepoints, as is required for runtime use of BPF
in real-time systems.

1.	Move rcu_is_watching() checks to srcu_read_{,un}lock_fast().

2.	Add srcu_read_lock_fast_notrace() and
	srcu_read_unlock_fast_notrace().

3.	Add guards for notrace variants of SRCU-fast readers.

4.	Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast.

Changes since v2:

o	Posting standalone as opposed to a reply.

	https://lore.kernel.org/all/3cecf6c9-b2ee-4f34-9d1b-ca4cfb8e56a7@paulmck-laptop/

Changes since RFC version:

o	RFC patch 6/4 has been pulled into the shared RCU tree:
	e88c632a8698 ("srcu: Add guards for SRCU-fast readers")

o	RFC patch 5/4 (which removed the now-unnecessary special boot-time
	avoidance of SRCU) has been folded into patch 4/4 shown above,
	as suggested by Steven Rostedt.

	https://lore.kernel.org/all/bb20a575-235b-499e-aa1d-70fe9e2c7617@paulmck-laptop/

						Thanx, Paul

------------------------------------------------------------------------

 b/include/linux/srcu.h       |    4 ++++
 b/include/linux/srcutree.h   |    2 --
 b/include/linux/tracepoint.h |    6 ++++--
 b/kernel/tracepoint.c        |   21 ++++++++++++++++++++-
 include/linux/srcu.h         |   30 ++++++++++++++++++++++++++++++
 5 files changed, 58 insertions(+), 5 deletions(-)

[PATCH v3 1/4] srcu: Move rcu_is_watching() checks to srcu_read_{,un}lock_fast()

[Paul E. McKenney]

The rcu_is_watching() warnings are currently in the SRCU-tree
implementations of __srcu_read_lock_fast() and __srcu_read_unlock_fast().
However, this makes it difficult to create _notrace variants of
srcu_read_lock_fast() and srcu_read_unlock_fast().  This commit therefore
moves these checks to srcu_read_lock_fast(), srcu_read_unlock_fast(),
srcu_down_read_fast(), and srcu_up_read_fast().

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/srcu.h     | 4 ++++
 include/linux/srcutree.h | 2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index f179700fecafb..478c73d067f7d 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -275,6 +275,7 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
 {
 	struct srcu_ctr __percpu *retval;
 
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_lock_fast().");
 	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
 	retval = __srcu_read_lock_fast(ssp);
 	rcu_try_lock_acquire(&ssp->dep_map);
@@ -295,6 +296,7 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
 static inline struct srcu_ctr __percpu *srcu_down_read_fast(struct srcu_struct *ssp) __acquires(ssp)
 {
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && in_nmi());
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_down_read_fast().");
 	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
 	return __srcu_read_lock_fast(ssp);
 }
@@ -389,6 +391,7 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
 	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
 	srcu_lock_release(&ssp->dep_map);
 	__srcu_read_unlock_fast(ssp, scp);
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
 }
 
 /**
@@ -405,6 +408,7 @@ static inline void srcu_up_read_fast(struct srcu_struct *ssp, struct srcu_ctr __
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && in_nmi());
 	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
 	__srcu_read_unlock_fast(ssp, scp);
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_up_read_fast().");
 }
 
 /**
diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index bf44d8d1e69ea..043b5a67ef71e 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -244,7 +244,6 @@ static inline struct srcu_ctr __percpu *__srcu_read_lock_fast(struct srcu_struct
 {
 	struct srcu_ctr __percpu *scp = READ_ONCE(ssp->srcu_ctrp);
 
-	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_lock_fast().");
 	if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
 		this_cpu_inc(scp->srcu_locks.counter); /* Y */
 	else
@@ -275,7 +274,6 @@ static inline void __srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_
 		this_cpu_inc(scp->srcu_unlocks.counter);  /* Z */
 	else
 		atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  /* Z */
-	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
 }
 
 void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
-- 
2.40.1

Re: [PATCH v3 1/4] srcu: Move rcu_is_watching() checks to srcu_read_{,un}lock_fast()

[Joel Fernandes]

On Mon, Jul 21, 2025 at 09:24:30AM -0700, Paul E. McKenney wrote:
> The rcu_is_watching() warnings are currently in the SRCU-tree
> implementations of __srcu_read_lock_fast() and __srcu_read_unlock_fast().
> However, this makes it difficult to create _notrace variants of
> srcu_read_lock_fast() and srcu_read_unlock_fast().  This commit therefore
> moves these checks to srcu_read_lock_fast(), srcu_read_unlock_fast(),
> srcu_down_read_fast(), and srcu_up_read_fast().
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

thanks,

 - Joel



> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/srcu.h     | 4 ++++
>  include/linux/srcutree.h | 2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index f179700fecafb..478c73d067f7d 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -275,6 +275,7 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
>  {
>  	struct srcu_ctr __percpu *retval;
>  
> +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_lock_fast().");
>  	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
>  	retval = __srcu_read_lock_fast(ssp);
>  	rcu_try_lock_acquire(&ssp->dep_map);
> @@ -295,6 +296,7 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
>  static inline struct srcu_ctr __percpu *srcu_down_read_fast(struct srcu_struct *ssp) __acquires(ssp)
>  {
>  	WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && in_nmi());
> +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_down_read_fast().");
>  	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
>  	return __srcu_read_lock_fast(ssp);
>  }
> @@ -389,6 +391,7 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
>  	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
>  	srcu_lock_release(&ssp->dep_map);
>  	__srcu_read_unlock_fast(ssp, scp);
> +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
>  }
>  
>  /**
> @@ -405,6 +408,7 @@ static inline void srcu_up_read_fast(struct srcu_struct *ssp, struct srcu_ctr __
>  	WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && in_nmi());
>  	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
>  	__srcu_read_unlock_fast(ssp, scp);
> +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_up_read_fast().");
>  }
>  
>  /**
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index bf44d8d1e69ea..043b5a67ef71e 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -244,7 +244,6 @@ static inline struct srcu_ctr __percpu *__srcu_read_lock_fast(struct srcu_struct
>  {
>  	struct srcu_ctr __percpu *scp = READ_ONCE(ssp->srcu_ctrp);
>  
> -	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_lock_fast().");
>  	if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
>  		this_cpu_inc(scp->srcu_locks.counter); /* Y */
>  	else
> @@ -275,7 +274,6 @@ static inline void __srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_
>  		this_cpu_inc(scp->srcu_unlocks.counter);  /* Z */
>  	else
>  		atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  /* Z */
> -	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
>  }
>  
>  void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> -- 
> 2.40.1
> 

[PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Paul E. McKenney]

This commit adds no-trace variants of the srcu_read_lock_fast() and
srcu_read_unlock_fast() functions for tracing use.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/srcu.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 478c73d067f7d..7a692bf8f99b9 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -282,6 +282,20 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
 	return retval;
 }
 
+/*
+ * Used by tracing, cannot be traced and cannot call lockdep.
+ * See srcu_read_lock_fast() for more information.
+ */
+static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
+	__acquires(ssp)
+{
+	struct srcu_ctr __percpu *retval;
+
+	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
+	retval = __srcu_read_lock_fast(ssp);
+	return retval;
+}
+
 /**
  * srcu_down_read_fast - register a new reader for an SRCU-protected structure.
  * @ssp: srcu_struct in which to register the new reader.
@@ -394,6 +408,17 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
 	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
 }
 
+/*
+ * Used by tracing, cannot be traced and cannot call lockdep.
+ * See srcu_read_unlock_fast() for more information.
+ */
+static inline void srcu_read_unlock_fast_notrace(struct srcu_struct *ssp,
+						 struct srcu_ctr __percpu *scp) __releases(ssp)
+{
+	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
+	__srcu_read_unlock_fast(ssp, scp);
+}
+
 /**
  * srcu_up_read_fast - unregister a old reader from an SRCU-protected structure.
  * @ssp: srcu_struct in which to unregister the old reader.
-- 
2.40.1

Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Joel Fernandes]

On Mon, Jul 21, 2025 at 09:24:31AM -0700, Paul E. McKenney wrote:
> This commit adds no-trace variants of the srcu_read_lock_fast() and
> srcu_read_unlock_fast() functions for tracing use.
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/srcu.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 478c73d067f7d..7a692bf8f99b9 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -282,6 +282,20 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
>  	return retval;
>  }
>  
> +/*
> + * Used by tracing, cannot be traced and cannot call lockdep.
> + * See srcu_read_lock_fast() for more information.
> + */
> +static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
> +	__acquires(ssp)

Should these also be marked with 'notrace' attribute?

I am not sure what the precedent is, I do see a few examples of 'notrace' and
'inline' in the same function signature though.

Other than that one nit:
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

thanks,

 - Joel


> +{
> +	struct srcu_ctr __percpu *retval;
> +
> +	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
> +	retval = __srcu_read_lock_fast(ssp);
> +	return retval;
> +}
> +
>  /**
>   * srcu_down_read_fast - register a new reader for an SRCU-protected structure.
>   * @ssp: srcu_struct in which to register the new reader.
> @@ -394,6 +408,17 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
>  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
>  }
>  
> +/*
> + * Used by tracing, cannot be traced and cannot call lockdep.
> + * See srcu_read_unlock_fast() for more information.
> + */
> +static inline void srcu_read_unlock_fast_notrace(struct srcu_struct *ssp,
> +						 struct srcu_ctr __percpu *scp) __releases(ssp)
> +{
> +	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
> +	__srcu_read_unlock_fast(ssp, scp);
> +}
> +
>  /**
>   * srcu_up_read_fast - unregister a old reader from an SRCU-protected structure.
>   * @ssp: srcu_struct in which to unregister the old reader.
> -- 
> 2.40.1
> 

Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Paul E. McKenney]

On Tue, Jul 22, 2025 at 06:11:00PM -0400, Joel Fernandes wrote:
> On Mon, Jul 21, 2025 at 09:24:31AM -0700, Paul E. McKenney wrote:
> > This commit adds no-trace variants of the srcu_read_lock_fast() and
> > srcu_read_unlock_fast() functions for tracing use.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >  include/linux/srcu.h | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index 478c73d067f7d..7a692bf8f99b9 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -282,6 +282,20 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
> >  	return retval;
> >  }
> >  
> > +/*
> > + * Used by tracing, cannot be traced and cannot call lockdep.
> > + * See srcu_read_lock_fast() for more information.
> > + */
> > +static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
> > +	__acquires(ssp)
> 
> Should these also be marked with 'notrace' attribute?
> 
> I am not sure what the precedent is, I do see a few examples of 'notrace' and
> 'inline' in the same function signature though.

Heh!!!

There are six instance of static-inline notrace functions, and eight
instances of static-inline non-notrace functions whose names contain
"_notrace", not counting the srcu_read_lock_fast_notrace() and
srcu_read_unlock_fast() functions currently under review.

My guess is that I should add "notrace" to handle the possible case
where the compiler declines to inline this function.  I will do this
on the next rebase unless I hear otherwise.

Steven, Mathieu, thoughts?

							Thanx, Paul

> Other than that one nit:
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> 
> thanks,
> 
>  - Joel
> 
> 
> > +{
> > +	struct srcu_ctr __percpu *retval;
> > +
> > +	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
> > +	retval = __srcu_read_lock_fast(ssp);
> > +	return retval;
> > +}
> > +
> >  /**
> >   * srcu_down_read_fast - register a new reader for an SRCU-protected structure.
> >   * @ssp: srcu_struct in which to register the new reader.
> > @@ -394,6 +408,17 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
> >  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
> >  }
> >  
> > +/*
> > + * Used by tracing, cannot be traced and cannot call lockdep.
> > + * See srcu_read_unlock_fast() for more information.
> > + */
> > +static inline void srcu_read_unlock_fast_notrace(struct srcu_struct *ssp,
> > +						 struct srcu_ctr __percpu *scp) __releases(ssp)
> > +{
> > +	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
> > +	__srcu_read_unlock_fast(ssp, scp);
> > +}
> > +
> >  /**
> >   * srcu_up_read_fast - unregister a old reader from an SRCU-protected structure.
> >   * @ssp: srcu_struct in which to unregister the old reader.
> > -- 
> > 2.40.1
> > 

Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Steven Rostedt]

On Tue, 22 Jul 2025 15:34:52 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> > > +static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
> > > +	__acquires(ssp)  
> > 
> > Should these also be marked with 'notrace' attribute?
> > 
> > I am not sure what the precedent is, I do see a few examples of 'notrace' and
> > 'inline' in the same function signature though.  
> 
> Heh!!!
> 
> There are six instance of static-inline notrace functions, and eight
> instances of static-inline non-notrace functions whose names contain
> "_notrace", not counting the srcu_read_lock_fast_notrace() and
> srcu_read_unlock_fast() functions currently under review.
> 
> My guess is that I should add "notrace" to handle the possible case
> where the compiler declines to inline this function.  I will do this
> on the next rebase unless I hear otherwise.
> 
> Steven, Mathieu, thoughts?

If you add "__always_inline" then it will include "notrace" as inlined
functions are not traced. But we have removed "notrace" from the generic
"inline" a while ago. If the compiler decides to ignore an "inline" it
*will* be traced.

We probably should fix any "_notrace" functions that are not
"__always_inline" and do not have "notrace".

-- Steve

Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Paul E. McKenney]

On Tue, Jul 22, 2025 at 06:47:36PM -0400, Steven Rostedt wrote:
> On Tue, 22 Jul 2025 15:34:52 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > > > +static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
> > > > +	__acquires(ssp)  
> > > 
> > > Should these also be marked with 'notrace' attribute?
> > > 
> > > I am not sure what the precedent is, I do see a few examples of 'notrace' and
> > > 'inline' in the same function signature though.  
> > 
> > Heh!!!
> > 
> > There are six instance of static-inline notrace functions, and eight
> > instances of static-inline non-notrace functions whose names contain
> > "_notrace", not counting the srcu_read_lock_fast_notrace() and
> > srcu_read_unlock_fast() functions currently under review.
> > 
> > My guess is that I should add "notrace" to handle the possible case
> > where the compiler declines to inline this function.  I will do this
> > on the next rebase unless I hear otherwise.
> > 
> > Steven, Mathieu, thoughts?
> 
> If you add "__always_inline" then it will include "notrace" as inlined
> functions are not traced. But we have removed "notrace" from the generic
> "inline" a while ago. If the compiler decides to ignore an "inline" it
> *will* be traced.
> 
> We probably should fix any "_notrace" functions that are not
> "__always_inline" and do not have "notrace".

Very good, and thank you!

On my next rebase, I will add "notrace" to srcu_read_lock_fast_notrace(),
srcu_read_unlock_fast_notrace(), __srcu_read_lock_fast(), and
__srcu_read_unlock_fast(), all of which are currently just static
inline.

							Thanx, Paul

Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Mathieu Desnoyers]

On 2025-07-22 18:34, Paul E. McKenney wrote:
> On Tue, Jul 22, 2025 at 06:11:00PM -0400, Joel Fernandes wrote:
>> On Mon, Jul 21, 2025 at 09:24:31AM -0700, Paul E. McKenney wrote:
>>> This commit adds no-trace variants of the srcu_read_lock_fast() and
>>> srcu_read_unlock_fast() functions for tracing use.
>>>
>>> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>> ---
>>>   include/linux/srcu.h | 25 +++++++++++++++++++++++++
>>>   1 file changed, 25 insertions(+)
>>>
>>> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
>>> index 478c73d067f7d..7a692bf8f99b9 100644
>>> --- a/include/linux/srcu.h
>>> +++ b/include/linux/srcu.h
>>> @@ -282,6 +282,20 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
>>>   	return retval;
>>>   }
>>>   
>>> +/*
>>> + * Used by tracing, cannot be traced and cannot call lockdep.
>>> + * See srcu_read_lock_fast() for more information.
>>> + */
>>> +static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
>>> +	__acquires(ssp)
>>
>> Should these also be marked with 'notrace' attribute?
>>
>> I am not sure what the precedent is, I do see a few examples of 'notrace' and
>> 'inline' in the same function signature though.
> 
> Heh!!!
> 
> There are six instance of static-inline notrace functions, and eight
> instances of static-inline non-notrace functions whose names contain
> "_notrace", not counting the srcu_read_lock_fast_notrace() and
> srcu_read_unlock_fast() functions currently under review.
> 
> My guess is that I should add "notrace" to handle the possible case
> where the compiler declines to inline this function.  I will do this
> on the next rebase unless I hear otherwise.
> 
> Steven, Mathieu, thoughts?

AFAIR, the always_inline gcc attribute takes care of making sure the
notrace is not needed in addition.

So I suspect that kernel APIs need to abide by the following rules
to be usable by instrumentation code:

- if it's a function call, have a notrace attribute.
- if it's an inline (which the compiler may decide to implement as a
   function call), have a notrace attribute.
- if it's an __always_inline, then there is no way the compiler can
   possibly make it a function call, so the notrace would be useless
   there.

So you may want to choose for either:

- inline notrace, or
- __always_inline

Depending on how much freedom you want to grant the compiler with
respect to its inlining practices.

Thanks,

Mathieu


> 
> 							Thanx, Paul
> 
>> Other than that one nit:
>> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
>>
>> thanks,
>>
>>   - Joel
>>
>>
>>> +{
>>> +	struct srcu_ctr __percpu *retval;
>>> +
>>> +	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
>>> +	retval = __srcu_read_lock_fast(ssp);
>>> +	return retval;
>>> +}
>>> +
>>>   /**
>>>    * srcu_down_read_fast - register a new reader for an SRCU-protected structure.
>>>    * @ssp: srcu_struct in which to register the new reader.
>>> @@ -394,6 +408,17 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
>>>   	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
>>>   }
>>>   
>>> +/*
>>> + * Used by tracing, cannot be traced and cannot call lockdep.
>>> + * See srcu_read_unlock_fast() for more information.
>>> + */
>>> +static inline void srcu_read_unlock_fast_notrace(struct srcu_struct *ssp,
>>> +						 struct srcu_ctr __percpu *scp) __releases(ssp)
>>> +{
>>> +	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
>>> +	__srcu_read_unlock_fast(ssp, scp);
>>> +}
>>> +
>>>   /**
>>>    * srcu_up_read_fast - unregister a old reader from an SRCU-protected structure.
>>>    * @ssp: srcu_struct in which to unregister the old reader.
>>> -- 
>>> 2.40.1
>>>


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()

[Paul E. McKenney]

On Wed, Jul 23, 2025 at 11:23:33AM -0400, Mathieu Desnoyers wrote:
> On 2025-07-22 18:34, Paul E. McKenney wrote:
> > On Tue, Jul 22, 2025 at 06:11:00PM -0400, Joel Fernandes wrote:
> > > On Mon, Jul 21, 2025 at 09:24:31AM -0700, Paul E. McKenney wrote:
> > > > This commit adds no-trace variants of the srcu_read_lock_fast() and
> > > > srcu_read_unlock_fast() functions for tracing use.
> > > > 
> > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > > Cc: Steven Rostedt <rostedt@goodmis.org>
> > > > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > > ---
> > > >   include/linux/srcu.h | 25 +++++++++++++++++++++++++
> > > >   1 file changed, 25 insertions(+)
> > > > 
> > > > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > > > index 478c73d067f7d..7a692bf8f99b9 100644
> > > > --- a/include/linux/srcu.h
> > > > +++ b/include/linux/srcu.h
> > > > @@ -282,6 +282,20 @@ static inline struct srcu_ctr __percpu *srcu_read_lock_fast(struct srcu_struct *
> > > >   	return retval;
> > > >   }
> > > > +/*
> > > > + * Used by tracing, cannot be traced and cannot call lockdep.
> > > > + * See srcu_read_lock_fast() for more information.
> > > > + */
> > > > +static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
> > > > +	__acquires(ssp)
> > > 
> > > Should these also be marked with 'notrace' attribute?
> > > 
> > > I am not sure what the precedent is, I do see a few examples of 'notrace' and
> > > 'inline' in the same function signature though.
> > 
> > Heh!!!
> > 
> > There are six instance of static-inline notrace functions, and eight
> > instances of static-inline non-notrace functions whose names contain
> > "_notrace", not counting the srcu_read_lock_fast_notrace() and
> > srcu_read_unlock_fast() functions currently under review.
> > 
> > My guess is that I should add "notrace" to handle the possible case
> > where the compiler declines to inline this function.  I will do this
> > on the next rebase unless I hear otherwise.
> > 
> > Steven, Mathieu, thoughts?
> 
> AFAIR, the always_inline gcc attribute takes care of making sure the
> notrace is not needed in addition.
> 
> So I suspect that kernel APIs need to abide by the following rules
> to be usable by instrumentation code:
> 
> - if it's a function call, have a notrace attribute.
> - if it's an inline (which the compiler may decide to implement as a
>   function call), have a notrace attribute.
> - if it's an __always_inline, then there is no way the compiler can
>   possibly make it a function call, so the notrace would be useless
>   there.
> 
> So you may want to choose for either:
> 
> - inline notrace, or
> - __always_inline
> 
> Depending on how much freedom you want to grant the compiler with
> respect to its inlining practices.

In this case, I will (uncharacteristically) grant the compiler some
freedom.  And so notrace it is!  ;-)

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > 							Thanx, Paul
> > 
> > > Other than that one nit:
> > > Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> > > 
> > > thanks,
> > > 
> > >   - Joel
> > > 
> > > 
> > > > +{
> > > > +	struct srcu_ctr __percpu *retval;
> > > > +
> > > > +	srcu_check_read_flavor_force(ssp, SRCU_READ_FLAVOR_FAST);
> > > > +	retval = __srcu_read_lock_fast(ssp);
> > > > +	return retval;
> > > > +}
> > > > +
> > > >   /**
> > > >    * srcu_down_read_fast - register a new reader for an SRCU-protected structure.
> > > >    * @ssp: srcu_struct in which to register the new reader.
> > > > @@ -394,6 +408,17 @@ static inline void srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_ct
> > > >   	RCU_LOCKDEP_WARN(!rcu_is_watching(), "RCU must be watching srcu_read_unlock_fast().");
> > > >   }
> > > > +/*
> > > > + * Used by tracing, cannot be traced and cannot call lockdep.
> > > > + * See srcu_read_unlock_fast() for more information.
> > > > + */
> > > > +static inline void srcu_read_unlock_fast_notrace(struct srcu_struct *ssp,
> > > > +						 struct srcu_ctr __percpu *scp) __releases(ssp)
> > > > +{
> > > > +	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_FAST);
> > > > +	__srcu_read_unlock_fast(ssp, scp);
> > > > +}
> > > > +
> > > >   /**
> > > >    * srcu_up_read_fast - unregister a old reader from an SRCU-protected structure.
> > > >    * @ssp: srcu_struct in which to unregister the old reader.
> > > > -- 
> > > > 2.40.1
> > > > 
> 
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com

[PATCH v3 3/4] srcu: Add guards for notrace variants of SRCU-fast readers

[Paul E. McKenney]

This adds the usual scoped_guard(srcu_fast_notrace, &my_srcu) and
guard(srcu_fast_notrace)(&my_srcu).

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/srcu.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 7a692bf8f99b9..ada65b58bc4c5 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -515,4 +515,9 @@ DEFINE_LOCK_GUARD_1(srcu_fast, struct srcu_struct,
 		    srcu_read_unlock_fast(_T->lock, _T->scp),
 		    struct srcu_ctr __percpu *scp)
 
+DEFINE_LOCK_GUARD_1(srcu_fast_notrace, struct srcu_struct,
+		    _T->scp = srcu_read_lock_fast_notrace(_T->lock),
+		    srcu_read_unlock_fast_notrace(_T->lock, _T->scp),
+		    struct srcu_ctr __percpu *scp)
+
 #endif
-- 
2.40.1

Re: [PATCH v3 3/4] srcu: Add guards for notrace variants of SRCU-fast readers

[Joel Fernandes]

On Mon, Jul 21, 2025 at 09:24:32AM -0700, Paul E. McKenney wrote:
> This adds the usual scoped_guard(srcu_fast_notrace, &my_srcu) and
> guard(srcu_fast_notrace)(&my_srcu).
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

thanks,

 - Joel

> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/srcu.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 7a692bf8f99b9..ada65b58bc4c5 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -515,4 +515,9 @@ DEFINE_LOCK_GUARD_1(srcu_fast, struct srcu_struct,
>  		    srcu_read_unlock_fast(_T->lock, _T->scp),
>  		    struct srcu_ctr __percpu *scp)
>  
> +DEFINE_LOCK_GUARD_1(srcu_fast_notrace, struct srcu_struct,
> +		    _T->scp = srcu_read_lock_fast_notrace(_T->lock),
> +		    srcu_read_unlock_fast_notrace(_T->lock, _T->scp),
> +		    struct srcu_ctr __percpu *scp)
> +
>  #endif
> -- 
> 2.40.1
> 

[PATCH v3 4/4] tracing: Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast

[Paul E. McKenney]

The current use of guard(preempt_notrace)() within __DECLARE_TRACE()
to protect invocation of __DO_TRACE_CALL() means that BPF programs
attached to tracepoints are non-preemptible.  This is unhelpful in
real-time systems, whose users apparently wish to use BPF while also
achieving low latencies.  (Who knew?)

One option would be to use preemptible RCU, but this introduces
many opportunities for infinite recursion, which many consider to
be counterproductive, especially given the relatively small stacks
provided by the Linux kernel.  These opportunities could be shut down
by sufficiently energetic duplication of code, but this sort of thing
is considered impolite in some circles.

Therefore, use the shiny new SRCU-fast API, which provides somewhat faster
readers than those of preemptible RCU, at least on my laptop, where
task_struct access is more expensive than access to per-CPU variables.
And SRCU fast provides way faster readers than does SRCU, courtesy of
being able to avoid the read-side use of smp_mb().  Also, it is quite
straightforward to create srcu_read_{,un}lock_fast_notrace() functions.

While in the area, SRCU now supports early boot call_srcu().  Therefore,
remove the checks that used to avoid such use from rcu_free_old_probes()
before this commit was applied:

e53244e2c893 ("tracepoint: Remove SRCU protection")

The current commit can be thought of as an approximate revert of that
commit.

Link: https://lore.kernel.org/all/20250613152218.1924093-1-bigeasy@linutronix.de/
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/tracepoint.h |  6 ++++--
 kernel/tracepoint.c        | 21 ++++++++++++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 826ce3f8e1f85..a22c1ab88560b 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -33,6 +33,8 @@ struct trace_eval_map {
 
 #define TRACEPOINT_DEFAULT_PRIO	10
 
+extern struct srcu_struct tracepoint_srcu;
+
 extern int
 tracepoint_probe_register(struct tracepoint *tp, void *probe, void *data);
 extern int
@@ -115,7 +117,7 @@ void for_each_tracepoint_in_module(struct module *mod,
 static inline void tracepoint_synchronize_unregister(void)
 {
 	synchronize_rcu_tasks_trace();
-	synchronize_rcu();
+	synchronize_srcu(&tracepoint_srcu);
 }
 static inline bool tracepoint_is_faultable(struct tracepoint *tp)
 {
@@ -271,7 +273,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 	static inline void __do_trace_##name(proto)			\
 	{								\
 		if (cond) {						\
-			guard(preempt_notrace)();			\
+			guard(srcu_fast_notrace)(&tracepoint_srcu);	\
 			__DO_TRACE_CALL(name, TP_ARGS(args));		\
 		}							\
 	}								\
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 62719d2941c90..e19973015cbd7 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -25,6 +25,9 @@ enum tp_func_state {
 extern tracepoint_ptr_t __start___tracepoints_ptrs[];
 extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
 
+DEFINE_SRCU(tracepoint_srcu);
+EXPORT_SYMBOL_GPL(tracepoint_srcu);
+
 enum tp_transition_sync {
 	TP_TRANSITION_SYNC_1_0_1,
 	TP_TRANSITION_SYNC_N_2_1,
@@ -34,6 +37,7 @@ enum tp_transition_sync {
 
 struct tp_transition_snapshot {
 	unsigned long rcu;
+	unsigned long srcu_gp;
 	bool ongoing;
 };
 
@@ -46,6 +50,7 @@ static void tp_rcu_get_state(enum tp_transition_sync sync)
 
 	/* Keep the latest get_state snapshot. */
 	snapshot->rcu = get_state_synchronize_rcu();
+	snapshot->srcu_gp = start_poll_synchronize_srcu(&tracepoint_srcu);
 	snapshot->ongoing = true;
 }
 
@@ -56,6 +61,8 @@ static void tp_rcu_cond_sync(enum tp_transition_sync sync)
 	if (!snapshot->ongoing)
 		return;
 	cond_synchronize_rcu(snapshot->rcu);
+	if (!poll_state_synchronize_srcu(&tracepoint_srcu, snapshot->srcu_gp))
+		synchronize_srcu(&tracepoint_srcu);
 	snapshot->ongoing = false;
 }
 
@@ -101,17 +108,29 @@ static inline void *allocate_probes(int count)
 	return p == NULL ? NULL : p->probes;
 }
 
-static void rcu_free_old_probes(struct rcu_head *head)
+static void srcu_free_old_probes(struct rcu_head *head)
 {
 	kfree(container_of(head, struct tp_probes, rcu));
 }
 
+static void rcu_free_old_probes(struct rcu_head *head)
+{
+	call_srcu(&tracepoint_srcu, head, srcu_free_old_probes);
+}
+
 static inline void release_probes(struct tracepoint *tp, struct tracepoint_func *old)
 {
 	if (old) {
 		struct tp_probes *tp_probes = container_of(old,
 			struct tp_probes, probes[0]);
 
+		/*
+		 * Tracepoint probes are protected by either RCU or
+		 * Tasks Trace RCU and also by SRCU.  By calling the SRCU
+		 * callback in the [Tasks Trace] RCU callback we cover
+		 * both cases. So let us chain the SRCU and [Tasks Trace]
+		 * RCU callbacks to wait for both grace periods.
+		 */
 		if (tracepoint_is_faultable(tp))
 			call_rcu_tasks_trace(&tp_probes->rcu, rcu_free_old_probes);
 		else
-- 
2.40.1

[PATCH v3 5/4] srcu: Document __srcu_read_{,un}lock_fast() implicit RCU readers

[Paul E. McKenney]

This commit documents the implicit RCU readers that are implied by the
this_cpu_inc() and atomic_long_inc() operations in __srcu_read_lock_fast()
and __srcu_read_unlock_fast().  While in the area, fix the documentation
of the memory pairing of atomic_long_inc() in __srcu_read_lock_fast().

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: <bpf@vger.kernel.org>

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index 043b5a67ef71e..78e1a7b845ba9 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -245,9 +245,9 @@ static inline struct srcu_ctr __percpu *__srcu_read_lock_fast(struct srcu_struct
 	struct srcu_ctr __percpu *scp = READ_ONCE(ssp->srcu_ctrp);
 
 	if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
-		this_cpu_inc(scp->srcu_locks.counter); /* Y */
+		this_cpu_inc(scp->srcu_locks.counter); // Y, and implicit RCU reader.
 	else
-		atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  /* Z */
+		atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  // Y, and implicit RCU reader.
 	barrier(); /* Avoid leaking the critical section. */
 	return scp;
 }
@@ -271,9 +271,9 @@ static inline void __srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_
 {
 	barrier();  /* Avoid leaking the critical section. */
 	if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
-		this_cpu_inc(scp->srcu_unlocks.counter);  /* Z */
+		this_cpu_inc(scp->srcu_unlocks.counter);  // Z, and implicit RCU reader.
 	else
-		atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  /* Z */
+		atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  // Z, and implicit RCU reader.
 }
 
 void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);

Re: [PATCH v3 5/4] srcu: Document __srcu_read_{,un}lock_fast() implicit RCU readers

[Joel Fernandes]

> On Jul 22, 2025, at 6:17 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> 
> This commit documents the implicit RCU readers that are implied by the
> this_cpu_inc() and atomic_long_inc() operations in __srcu_read_lock_fast()
> and __srcu_read_unlock_fast().  While in the area, fix the documentation
> of the memory pairing of atomic_long_inc() in __srcu_read_lock_fast().

Just to clarify, the implication here is since SRCU-fast uses synchronize_rcu on the update side, these operations result in blocking of classical RCU too. So simply using srcu fast is another way of achieving the previously used pre-empt-disabling in the use cases.

Or is the rationale for this something else?

I would probably spell this out more in a longer comment above the if/else, than modify the inline comments.

But I am probably misunderstood the whole thing. :-(

-Joel

> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: <bpf@vger.kernel.org>
> 
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 043b5a67ef71e..78e1a7b845ba9 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -245,9 +245,9 @@ static inline struct srcu_ctr __percpu *__srcu_read_lock_fast(struct srcu_struct
>    struct srcu_ctr __percpu *scp = READ_ONCE(ssp->srcu_ctrp);
> 
>    if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
> -        this_cpu_inc(scp->srcu_locks.counter); /* Y */
> +        this_cpu_inc(scp->srcu_locks.counter); // Y, and implicit RCU reader.
>    else
> -        atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  /* Z */
> +        atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  // Y, and implicit RCU reader.
>    barrier(); /* Avoid leaking the critical section. */
>    return scp;
> }
> @@ -271,9 +271,9 @@ static inline void __srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_
> {
>    barrier();  /* Avoid leaking the critical section. */
>    if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
> -        this_cpu_inc(scp->srcu_unlocks.counter);  /* Z */
> +        this_cpu_inc(scp->srcu_unlocks.counter);  // Z, and implicit RCU reader.
>    else
> -        atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  /* Z */
> +        atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  // Z, and implicit RCU reader.
> }
> 
> void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> 

Re: [PATCH v3 5/4] srcu: Document __srcu_read_{,un}lock_fast() implicit RCU readers

[Joel Fernandes]

On 7/23/2025 9:32 AM, joelagnelf@nvidia.com wrote:
> 
> 
>> On Jul 22, 2025, at 6:17 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
>>
>> This commit documents the implicit RCU readers that are implied by the
>> this_cpu_inc() and atomic_long_inc() operations in __srcu_read_lock_fast()
>> and __srcu_read_unlock_fast().  While in the area, fix the documentation
>> of the memory pairing of atomic_long_inc() in __srcu_read_lock_fast().
> 
> Just to clarify, the implication here is since SRCU-fast uses synchronize_rcu on the update side, these operations result in blocking of classical RCU too. So simply using srcu fast is another way of achieving the previously used pre-empt-disabling in the use cases.

Hi Paul, it was nice sync'ing with you off-list. Following are my suggestions
and where I am coming from:

1. For someone who doesn't know SRCU-fast depends on synchronize_rcu (me after a
few beers :P), the word 'RCU' in the comment you added to this patch, might come
across as 'which RCU are we referring to - SRCU or classical RCU or some other'.
So I would call it 'classical RCU reader' in the comment.

2. It would be good to call out specifically that, the SRCU-fast critical
section is akin to a classical RCU reader, because of its implementation's
dependence on synchronize_rcu() to overcome the lack of read-side memory barriers.

3. I think since the potential size of these code comment suggestions, it may
make sense to provide a bigger comment suggesting these than providing them
inline as you did. And also calling out the tracing usecase in the comments for
additional usecase clarification.

I could provide a patch to do all this soon, as we discussed, as well (unless
you're Ok with making this change as well).

Thanks!

 - Joel




> 
> Or is the rationale for this something else?
> 
> I would probably spell this out more in a longer comment above the if/else, than modify the inline comments.
> 
> But I am probably misunderstood the whole thing. :-(
> 
> -Joel
> 
>>
>> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> Cc: <bpf@vger.kernel.org>
>>
>> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
>> index 043b5a67ef71e..78e1a7b845ba9 100644
>> --- a/include/linux/srcutree.h
>> +++ b/include/linux/srcutree.h
>> @@ -245,9 +245,9 @@ static inline struct srcu_ctr __percpu *__srcu_read_lock_fast(struct srcu_struct
>>    struct srcu_ctr __percpu *scp = READ_ONCE(ssp->srcu_ctrp);
>>
>>    if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
>> -        this_cpu_inc(scp->srcu_locks.counter); /* Y */
>> +        this_cpu_inc(scp->srcu_locks.counter); // Y, and implicit RCU reader.
>>    else
>> -        atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  /* Z */
>> +        atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  // Y, and implicit RCU reader.
>>    barrier(); /* Avoid leaking the critical section. */
>>    return scp;
>> }
>> @@ -271,9 +271,9 @@ static inline void __srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_
>> {
>>    barrier();  /* Avoid leaking the critical section. */
>>    if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
>> -        this_cpu_inc(scp->srcu_unlocks.counter);  /* Z */
>> +        this_cpu_inc(scp->srcu_unlocks.counter);  // Z, and implicit RCU reader.
>>    else
>> -        atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  /* Z */
>> +        atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  // Z, and implicit RCU reader.
>> }
>>
>> void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
>>

Re: [PATCH v3 5/4] srcu: Document __srcu_read_{,un}lock_fast() implicit RCU readers

[Paul E. McKenney]

On Wed, Jul 23, 2025 at 12:10:46PM -0400, Joel Fernandes wrote:
> 
> 
> On 7/23/2025 9:32 AM, joelagnelf@nvidia.com wrote:
> > 
> > 
> >> On Jul 22, 2025, at 6:17 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> >>
> >> This commit documents the implicit RCU readers that are implied by the
> >> this_cpu_inc() and atomic_long_inc() operations in __srcu_read_lock_fast()
> >> and __srcu_read_unlock_fast().  While in the area, fix the documentation
> >> of the memory pairing of atomic_long_inc() in __srcu_read_lock_fast().
> > 
> > Just to clarify, the implication here is since SRCU-fast uses synchronize_rcu on the update side, these operations result in blocking of classical RCU too. So simply using srcu fast is another way of achieving the previously used pre-empt-disabling in the use cases.
> 
> Hi Paul, it was nice sync'ing with you off-list. Following are my suggestions
> and where I am coming from:
> 
> 1. For someone who doesn't know SRCU-fast depends on synchronize_rcu (me after a
> few beers :P), the word 'RCU' in the comment you added to this patch, might come
> across as 'which RCU are we referring to - SRCU or classical RCU or some other'.
> So I would call it 'classical RCU reader' in the comment.
> 
> 2. It would be good to call out specifically that, the SRCU-fast critical
> section is akin to a classical RCU reader, because of its implementation's
> dependence on synchronize_rcu() to overcome the lack of read-side memory barriers.
> 
> 3. I think since the potential size of these code comment suggestions, it may
> make sense to provide a bigger comment suggesting these than providing them
> inline as you did. And also calling out the tracing usecase in the comments for
> additional usecase clarification.
> 
> I could provide a patch to do all this soon, as we discussed, as well (unless
> you're Ok with making this change as well).

Thank you very much for the clarification, and I will make the changes
with attribution.

							Thanx, Paul

> Thanks!
> 
>  - Joel
> 
> 
> 
> 
> > 
> > Or is the rationale for this something else?
> > 
> > I would probably spell this out more in a longer comment above the if/else, than modify the inline comments.
> > 
> > But I am probably misunderstood the whole thing. :-(
> > 
> > -Joel
> > 
> >>
> >> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> >> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >> Cc: Steven Rostedt <rostedt@goodmis.org>
> >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> >> Cc: <bpf@vger.kernel.org>
> >>
> >> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> >> index 043b5a67ef71e..78e1a7b845ba9 100644
> >> --- a/include/linux/srcutree.h
> >> +++ b/include/linux/srcutree.h
> >> @@ -245,9 +245,9 @@ static inline struct srcu_ctr __percpu *__srcu_read_lock_fast(struct srcu_struct
> >>    struct srcu_ctr __percpu *scp = READ_ONCE(ssp->srcu_ctrp);
> >>
> >>    if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
> >> -        this_cpu_inc(scp->srcu_locks.counter); /* Y */
> >> +        this_cpu_inc(scp->srcu_locks.counter); // Y, and implicit RCU reader.
> >>    else
> >> -        atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  /* Z */
> >> +        atomic_long_inc(raw_cpu_ptr(&scp->srcu_locks));  // Y, and implicit RCU reader.
> >>    barrier(); /* Avoid leaking the critical section. */
> >>    return scp;
> >> }
> >> @@ -271,9 +271,9 @@ static inline void __srcu_read_unlock_fast(struct srcu_struct *ssp, struct srcu_
> >> {
> >>    barrier();  /* Avoid leaking the critical section. */
> >>    if (!IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE))
> >> -        this_cpu_inc(scp->srcu_unlocks.counter);  /* Z */
> >> +        this_cpu_inc(scp->srcu_unlocks.counter);  // Z, and implicit RCU reader.
> >>    else
> >> -        atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  /* Z */
> >> +        atomic_long_inc(raw_cpu_ptr(&scp->srcu_unlocks));  // Z, and implicit RCU reader.
> >> }
> >>
> >> void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> >>
> 

[PATCH v3 6/4] srcu: Document srcu_flip() memory-barrier D relation to SRCU-fast

[Paul E. McKenney]

The smp_mb() memory barrier at the end of srcu_flip() has a comment,
but that comment does not make it clear that this memory barrier is an
optimization, as opposed to being needed for correctness.  This commit
therefore adds this information and points out that it is omitted
for SRCU-fast, where a much heavier weight synchronize_srcu() would
be required.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: <bpf@vger.kernel.org>

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index c5e8ebc493d5e..1ff94b76d91f1 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -1168,6 +1168,16 @@ static void srcu_flip(struct srcu_struct *ssp)
 	 * counter update.  Note that both this memory barrier and the
 	 * one in srcu_readers_active_idx_check() provide the guarantee
 	 * for __srcu_read_lock().
+	 *
+	 * Note that this is a performance optimization, in which we spend
+	 * an otherwise unnecessary smp_mb() in order to reduce the number
+	 * of full per-CPU-variable scans in srcu_readers_lock_idx() and
+	 * srcu_readers_unlock_idx().  But this performance optimization
+	 * is not so optimal for SRCU-fast, where we would be spending
+	 * not smp_mb(), but rather synchronize_rcu().  At the same time,
+	 * the overhead of the smp_mb() is in the noise, so there is no
+	 * point in omitting it in the SRCU-fast case.  So the same code
+	 * is executed either way.
 	 */
 	smp_mb(); /* D */  /* Pairs with C. */
 }