Re: [PATCH rcu 05/12] srcu: Standardize srcu_data pointers to "sdp" and similar

2024-10-14 Thread Neeraj Upadhyay



On 10/15/2024 5:59 AM, Paul E. McKenney wrote:
> On Mon, Oct 14, 2024 at 09:49:52AM -0700, Paul E. McKenney wrote:
>> On Mon, Oct 14, 2024 at 02:45:50PM +0530, Neeraj Upadhyay wrote:
>>> On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
 This commit changes a few "cpuc" variables to "sdp" to align wiht usage
 elsewhere.

>>>
>>> s/wiht/with/
>>
>> Good eyes!
> 
> And fixed.
> >>> This commit is doing a lot more than renaming "cpuc".
>>
>> Indeed, it does look like I forgot to commit between two changes.
>>
>> It looks like this commit log goes with the changes to the
>> functions srcu_readers_lock_idx(), srcu_readers_unlock_idx(), and
>> srcu_readers_active().  With the exception of the change from "NMI-safe"
>> to "reader flavors in the WARN_ONCE() string in srcu_readers_unlock_idx().
>>
>> How would you suggest that I split up the non-s/cpuc/sdp/ changes?
> 
> As a first step, I split it three ways, as you can see on the updated "dev"
> branch in the -rcu tree.
> 
> Thoughts?
> 

Split looks good to me!


- Neeraj

>   Thanx, Paul
> 
>>> - Neeraj
>>>
 Signed-off-by: Paul E. McKenney 
 Cc: Alexei Starovoitov 
 Cc: Andrii Nakryiko 
 Cc: Peter Zijlstra 
 Cc: Kent Overstreet 
 Cc: 
 ---
  include/linux/srcu.h | 35 ++
  include/linux/srcutree.h |  4 
  kernel/rcu/srcutree.c| 41 
  3 files changed, 44 insertions(+), 36 deletions(-)

 diff --git a/include/linux/srcu.h b/include/linux/srcu.h
 index 06728ef6f32a4..84daaa33ea0ab 100644
 --- a/include/linux/srcu.h
 +++ b/include/linux/srcu.h
 @@ -176,10 +176,6 @@ static inline int srcu_read_lock_held(const struct 
 srcu_struct *ssp)
  
  #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
  
 -#define SRCU_NMI_UNKNOWN  0x0
 -#define SRCU_NMI_UNSAFE   0x1
 -#define SRCU_NMI_SAFE 0x2
 -
  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
  void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
  #else
 @@ -235,16 +231,19 @@ static inline void srcu_check_read_flavor(struct 
 srcu_struct *ssp, int read_flav
   * a mutex that is held elsewhere while calling synchronize_srcu() or
   * synchronize_srcu_expedited().
   *
 - * Note that srcu_read_lock() and the matching srcu_read_unlock() must
 - * occur in the same context, for example, it is illegal to invoke
 - * srcu_read_unlock() in an irq handler if the matching srcu_read_lock()
 - * was invoked in process context.
 + * The return value from srcu_read_lock() must be passed unaltered
 + * to the matching srcu_read_unlock().  Note that srcu_read_lock() and
 + * the matching srcu_read_unlock() must occur in the same context, for
 + * example, it is illegal to invoke srcu_read_unlock() in an irq handler
 + * if the matching srcu_read_lock() was invoked in process context.  Or,
 + * for that matter to invoke srcu_read_unlock() from one task and the
 + * matching srcu_read_lock() from another.
   */
  static inline int srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp)
  {
int retval;
  
 -  srcu_check_read_flavor(ssp, false);
 +  srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
retval = __srcu_read_lock(ssp);
srcu_lock_acquire(&ssp->dep_map);
return retval;
 @@ -256,12 +255,16 @@ static inline int srcu_read_lock(struct srcu_struct 
 *ssp) __acquires(ssp)
   *
   * Enter an SRCU read-side critical section, but in an NMI-safe manner.
   * See srcu_read_lock() for more information.
 + *
 + * If srcu_read_lock_nmisafe() is ever used on an srcu_struct structure,
 + * then none of the other flavors may be used, whether before, during,
 + * or after.
   */
  static inline int srcu_read_lock_nmisafe(struct srcu_struct *ssp) 
 __acquires(ssp)
  {
int retval;
  
 -  srcu_check_read_flavor(ssp, true);
 +  srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NMI);
retval = __srcu_read_lock_nmisafe(ssp);
rcu_try_lock_acquire(&ssp->dep_map);
return retval;
 @@ -273,7 +276,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
 __acquires(ssp)
  {
int retval;
  
 -  srcu_check_read_flavor(ssp, false);
 +  srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
retval = __srcu_read_lock(ssp);
return retval;
  }
 @@ -302,7 +305,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
 __acquires(ssp)
  static inline int srcu_down_read(struct srcu_struct *ssp) __acquires(ssp)
  {
WARN_ON_ONCE(in_nmi());
 -  srcu_check_read_flavor(ssp, false);
 +  srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
return __srcu_read_lock(ssp);
  }
  
>

Re: [PATCH rcu 05/12] srcu: Standardize srcu_data pointers to "sdp" and similar

2024-10-14 Thread Paul E. McKenney
On Mon, Oct 14, 2024 at 09:49:52AM -0700, Paul E. McKenney wrote:
> On Mon, Oct 14, 2024 at 02:45:50PM +0530, Neeraj Upadhyay wrote:
> > On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
> > > This commit changes a few "cpuc" variables to "sdp" to align wiht usage
> > > elsewhere.
> > > 
> > 
> > s/wiht/with/
> 
> Good eyes!

And fixed.

> > This commit is doing a lot more than renaming "cpuc".
> 
> Indeed, it does look like I forgot to commit between two changes.
> 
> It looks like this commit log goes with the changes to the
> functions srcu_readers_lock_idx(), srcu_readers_unlock_idx(), and
> srcu_readers_active().  With the exception of the change from "NMI-safe"
> to "reader flavors in the WARN_ONCE() string in srcu_readers_unlock_idx().
> 
> How would you suggest that I split up the non-s/cpuc/sdp/ changes?

As a first step, I split it three ways, as you can see on the updated "dev"
branch in the -rcu tree.

Thoughts?

Thanx, Paul

> > - Neeraj
> > 
> > > Signed-off-by: Paul E. McKenney 
> > > Cc: Alexei Starovoitov 
> > > Cc: Andrii Nakryiko 
> > > Cc: Peter Zijlstra 
> > > Cc: Kent Overstreet 
> > > Cc: 
> > > ---
> > >  include/linux/srcu.h | 35 ++
> > >  include/linux/srcutree.h |  4 
> > >  kernel/rcu/srcutree.c| 41 
> > >  3 files changed, 44 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > > index 06728ef6f32a4..84daaa33ea0ab 100644
> > > --- a/include/linux/srcu.h
> > > +++ b/include/linux/srcu.h
> > > @@ -176,10 +176,6 @@ static inline int srcu_read_lock_held(const struct 
> > > srcu_struct *ssp)
> > >  
> > >  #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > >  
> > > -#define SRCU_NMI_UNKNOWN 0x0
> > > -#define SRCU_NMI_UNSAFE  0x1
> > > -#define SRCU_NMI_SAFE0x2
> > > -
> > >  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
> > >  void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> > >  #else
> > > @@ -235,16 +231,19 @@ static inline void srcu_check_read_flavor(struct 
> > > srcu_struct *ssp, int read_flav
> > >   * a mutex that is held elsewhere while calling synchronize_srcu() or
> > >   * synchronize_srcu_expedited().
> > >   *
> > > - * Note that srcu_read_lock() and the matching srcu_read_unlock() must
> > > - * occur in the same context, for example, it is illegal to invoke
> > > - * srcu_read_unlock() in an irq handler if the matching srcu_read_lock()
> > > - * was invoked in process context.
> > > + * The return value from srcu_read_lock() must be passed unaltered
> > > + * to the matching srcu_read_unlock().  Note that srcu_read_lock() and
> > > + * the matching srcu_read_unlock() must occur in the same context, for
> > > + * example, it is illegal to invoke srcu_read_unlock() in an irq handler
> > > + * if the matching srcu_read_lock() was invoked in process context.  Or,
> > > + * for that matter to invoke srcu_read_unlock() from one task and the
> > > + * matching srcu_read_lock() from another.
> > >   */
> > >  static inline int srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp)
> > >  {
> > >   int retval;
> > >  
> > > - srcu_check_read_flavor(ssp, false);
> > > + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > >   retval = __srcu_read_lock(ssp);
> > >   srcu_lock_acquire(&ssp->dep_map);
> > >   return retval;
> > > @@ -256,12 +255,16 @@ static inline int srcu_read_lock(struct srcu_struct 
> > > *ssp) __acquires(ssp)
> > >   *
> > >   * Enter an SRCU read-side critical section, but in an NMI-safe manner.
> > >   * See srcu_read_lock() for more information.
> > > + *
> > > + * If srcu_read_lock_nmisafe() is ever used on an srcu_struct structure,
> > > + * then none of the other flavors may be used, whether before, during,
> > > + * or after.
> > >   */
> > >  static inline int srcu_read_lock_nmisafe(struct srcu_struct *ssp) 
> > > __acquires(ssp)
> > >  {
> > >   int retval;
> > >  
> > > - srcu_check_read_flavor(ssp, true);
> > > + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NMI);
> > >   retval = __srcu_read_lock_nmisafe(ssp);
> > >   rcu_try_lock_acquire(&ssp->dep_map);
> > >   return retval;
> > > @@ -273,7 +276,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> > > __acquires(ssp)
> > >  {
> > >   int retval;
> > >  
> > > - srcu_check_read_flavor(ssp, false);
> > > + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > >   retval = __srcu_read_lock(ssp);
> > >   return retval;
> > >  }
> > > @@ -302,7 +305,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> > > __acquires(ssp)
> > >  static inline int srcu_down_read(struct srcu_struct *ssp) __acquires(ssp)
> > >  {
> > >   WARN_ON_ONCE(in_nmi());
> > > - srcu_check_read_flavor(ssp, false);
> > > + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > >   return __srcu_read_lock(ssp);
> > >  }
> > >  
> > > @@ -317,7 +320,7 @@ static inline 

Re: [PATCH rcu 05/12] srcu: Standardize srcu_data pointers to "sdp" and similar

2024-10-14 Thread Paul E. McKenney
On Mon, Oct 14, 2024 at 02:45:50PM +0530, Neeraj Upadhyay wrote:
> On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
> > This commit changes a few "cpuc" variables to "sdp" to align wiht usage
> > elsewhere.
> > 
> 
> s/wiht/with/

Good eyes!

> This commit is doing a lot more than renaming "cpuc".

Indeed, it does look like I forgot to commit between two changes.

It looks like this commit log goes with the changes to the
functions srcu_readers_lock_idx(), srcu_readers_unlock_idx(), and
srcu_readers_active().  With the exception of the change from "NMI-safe"
to "reader flavors in the WARN_ONCE() string in srcu_readers_unlock_idx().

How would you suggest that I split up the non-s/cpuc/sdp/ changes?

Thanx, Paul

> - Neeraj
> 
> > Signed-off-by: Paul E. McKenney 
> > Cc: Alexei Starovoitov 
> > Cc: Andrii Nakryiko 
> > Cc: Peter Zijlstra 
> > Cc: Kent Overstreet 
> > Cc: 
> > ---
> >  include/linux/srcu.h | 35 ++
> >  include/linux/srcutree.h |  4 
> >  kernel/rcu/srcutree.c| 41 
> >  3 files changed, 44 insertions(+), 36 deletions(-)
> > 
> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index 06728ef6f32a4..84daaa33ea0ab 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -176,10 +176,6 @@ static inline int srcu_read_lock_held(const struct 
> > srcu_struct *ssp)
> >  
> >  #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> >  
> > -#define SRCU_NMI_UNKNOWN   0x0
> > -#define SRCU_NMI_UNSAFE0x1
> > -#define SRCU_NMI_SAFE  0x2
> > -
> >  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
> >  void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> >  #else
> > @@ -235,16 +231,19 @@ static inline void srcu_check_read_flavor(struct 
> > srcu_struct *ssp, int read_flav
> >   * a mutex that is held elsewhere while calling synchronize_srcu() or
> >   * synchronize_srcu_expedited().
> >   *
> > - * Note that srcu_read_lock() and the matching srcu_read_unlock() must
> > - * occur in the same context, for example, it is illegal to invoke
> > - * srcu_read_unlock() in an irq handler if the matching srcu_read_lock()
> > - * was invoked in process context.
> > + * The return value from srcu_read_lock() must be passed unaltered
> > + * to the matching srcu_read_unlock().  Note that srcu_read_lock() and
> > + * the matching srcu_read_unlock() must occur in the same context, for
> > + * example, it is illegal to invoke srcu_read_unlock() in an irq handler
> > + * if the matching srcu_read_lock() was invoked in process context.  Or,
> > + * for that matter to invoke srcu_read_unlock() from one task and the
> > + * matching srcu_read_lock() from another.
> >   */
> >  static inline int srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp)
> >  {
> > int retval;
> >  
> > -   srcu_check_read_flavor(ssp, false);
> > +   srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > retval = __srcu_read_lock(ssp);
> > srcu_lock_acquire(&ssp->dep_map);
> > return retval;
> > @@ -256,12 +255,16 @@ static inline int srcu_read_lock(struct srcu_struct 
> > *ssp) __acquires(ssp)
> >   *
> >   * Enter an SRCU read-side critical section, but in an NMI-safe manner.
> >   * See srcu_read_lock() for more information.
> > + *
> > + * If srcu_read_lock_nmisafe() is ever used on an srcu_struct structure,
> > + * then none of the other flavors may be used, whether before, during,
> > + * or after.
> >   */
> >  static inline int srcu_read_lock_nmisafe(struct srcu_struct *ssp) 
> > __acquires(ssp)
> >  {
> > int retval;
> >  
> > -   srcu_check_read_flavor(ssp, true);
> > +   srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NMI);
> > retval = __srcu_read_lock_nmisafe(ssp);
> > rcu_try_lock_acquire(&ssp->dep_map);
> > return retval;
> > @@ -273,7 +276,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> > __acquires(ssp)
> >  {
> > int retval;
> >  
> > -   srcu_check_read_flavor(ssp, false);
> > +   srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > retval = __srcu_read_lock(ssp);
> > return retval;
> >  }
> > @@ -302,7 +305,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> > __acquires(ssp)
> >  static inline int srcu_down_read(struct srcu_struct *ssp) __acquires(ssp)
> >  {
> > WARN_ON_ONCE(in_nmi());
> > -   srcu_check_read_flavor(ssp, false);
> > +   srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > return __srcu_read_lock(ssp);
> >  }
> >  
> > @@ -317,7 +320,7 @@ static inline void srcu_read_unlock(struct srcu_struct 
> > *ssp, int idx)
> > __releases(ssp)
> >  {
> > WARN_ON_ONCE(idx & ~0x1);
> > -   srcu_check_read_flavor(ssp, false);
> > +   srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > srcu_lock_release(&ssp->dep_map);
> > __srcu_read_unlock(ssp, idx);
> >  }
> > @@ -333,7 +336,7 @@ static inline void srcu_read_unl

Re: [PATCH rcu 05/12] srcu: Standardize srcu_data pointers to "sdp" and similar

2024-10-14 Thread Neeraj Upadhyay
On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
> This commit changes a few "cpuc" variables to "sdp" to align wiht usage
> elsewhere.
> 

s/wiht/with/

This commit is doing a lot more than renaming "cpuc".



- Neeraj

> Signed-off-by: Paul E. McKenney 
> Cc: Alexei Starovoitov 
> Cc: Andrii Nakryiko 
> Cc: Peter Zijlstra 
> Cc: Kent Overstreet 
> Cc: 
> ---
>  include/linux/srcu.h | 35 ++
>  include/linux/srcutree.h |  4 
>  kernel/rcu/srcutree.c| 41 
>  3 files changed, 44 insertions(+), 36 deletions(-)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 06728ef6f32a4..84daaa33ea0ab 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -176,10 +176,6 @@ static inline int srcu_read_lock_held(const struct 
> srcu_struct *ssp)
>  
>  #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
>  
> -#define SRCU_NMI_UNKNOWN 0x0
> -#define SRCU_NMI_UNSAFE  0x1
> -#define SRCU_NMI_SAFE0x2
> -
>  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
>  void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
>  #else
> @@ -235,16 +231,19 @@ static inline void srcu_check_read_flavor(struct 
> srcu_struct *ssp, int read_flav
>   * a mutex that is held elsewhere while calling synchronize_srcu() or
>   * synchronize_srcu_expedited().
>   *
> - * Note that srcu_read_lock() and the matching srcu_read_unlock() must
> - * occur in the same context, for example, it is illegal to invoke
> - * srcu_read_unlock() in an irq handler if the matching srcu_read_lock()
> - * was invoked in process context.
> + * The return value from srcu_read_lock() must be passed unaltered
> + * to the matching srcu_read_unlock().  Note that srcu_read_lock() and
> + * the matching srcu_read_unlock() must occur in the same context, for
> + * example, it is illegal to invoke srcu_read_unlock() in an irq handler
> + * if the matching srcu_read_lock() was invoked in process context.  Or,
> + * for that matter to invoke srcu_read_unlock() from one task and the
> + * matching srcu_read_lock() from another.
>   */
>  static inline int srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp)
>  {
>   int retval;
>  
> - srcu_check_read_flavor(ssp, false);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
>   retval = __srcu_read_lock(ssp);
>   srcu_lock_acquire(&ssp->dep_map);
>   return retval;
> @@ -256,12 +255,16 @@ static inline int srcu_read_lock(struct srcu_struct 
> *ssp) __acquires(ssp)
>   *
>   * Enter an SRCU read-side critical section, but in an NMI-safe manner.
>   * See srcu_read_lock() for more information.
> + *
> + * If srcu_read_lock_nmisafe() is ever used on an srcu_struct structure,
> + * then none of the other flavors may be used, whether before, during,
> + * or after.
>   */
>  static inline int srcu_read_lock_nmisafe(struct srcu_struct *ssp) 
> __acquires(ssp)
>  {
>   int retval;
>  
> - srcu_check_read_flavor(ssp, true);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NMI);
>   retval = __srcu_read_lock_nmisafe(ssp);
>   rcu_try_lock_acquire(&ssp->dep_map);
>   return retval;
> @@ -273,7 +276,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> __acquires(ssp)
>  {
>   int retval;
>  
> - srcu_check_read_flavor(ssp, false);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
>   retval = __srcu_read_lock(ssp);
>   return retval;
>  }
> @@ -302,7 +305,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> __acquires(ssp)
>  static inline int srcu_down_read(struct srcu_struct *ssp) __acquires(ssp)
>  {
>   WARN_ON_ONCE(in_nmi());
> - srcu_check_read_flavor(ssp, false);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
>   return __srcu_read_lock(ssp);
>  }
>  
> @@ -317,7 +320,7 @@ static inline void srcu_read_unlock(struct srcu_struct 
> *ssp, int idx)
>   __releases(ssp)
>  {
>   WARN_ON_ONCE(idx & ~0x1);
> - srcu_check_read_flavor(ssp, false);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
>   srcu_lock_release(&ssp->dep_map);
>   __srcu_read_unlock(ssp, idx);
>  }
> @@ -333,7 +336,7 @@ static inline void srcu_read_unlock_nmisafe(struct 
> srcu_struct *ssp, int idx)
>   __releases(ssp)
>  {
>   WARN_ON_ONCE(idx & ~0x1);
> - srcu_check_read_flavor(ssp, true);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NMI);
>   rcu_lock_release(&ssp->dep_map);
>   __srcu_read_unlock_nmisafe(ssp, idx);
>  }
> @@ -342,7 +345,7 @@ static inline void srcu_read_unlock_nmisafe(struct 
> srcu_struct *ssp, int idx)
>  static inline notrace void
>  srcu_read_unlock_notrace(struct srcu_struct *ssp, int idx) __releases(ssp)
>  {
> - srcu_check_read_flavor(ssp, false);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
>   __srcu_read_unlock(ssp, idx);
>  }
>  
> @@ -359,7 +362,7 @@ static inline void srcu_