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

Paul E. McKenney posted 4 patches 2 months, 2 weeks ago
[PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()
Posted by Paul E. McKenney 2 months, 2 weeks ago
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()
Posted by Joel Fernandes 2 months, 2 weeks ago
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()
Posted by Paul E. McKenney 2 months, 2 weeks ago
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()
Posted by Mathieu Desnoyers 2 months, 2 weeks ago
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()
Posted by Paul E. McKenney 2 months, 2 weeks ago
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
Re: [PATCH v3 2/4] srcu: Add srcu_read_lock_fast_notrace() and srcu_read_unlock_fast_notrace()
Posted by Steven Rostedt 2 months, 2 weeks ago
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()
Posted by Paul E. McKenney 2 months, 2 weeks ago
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