Re: [PATCH rcu 03/12] srcu: Renaming in preparation for additional reader flavor

2024-10-14 Thread Neeraj Upadhyay



On 10/14/2024 10:22 PM, Paul E. McKenney wrote:
> On Mon, Oct 14, 2024 at 02:40:35PM +0530, Neeraj Upadhyay wrote:
>> On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
>>> Currently, there are only two flavors of readers, normal and NMI-safe.
>>> A number of fields, functions, and types reflect this restriction.
>>> This renaming-only commit prepares for the addition of light-weight
>>> (as in memory-barrier-free) readers.  OK, OK, there is also a drive-by
>>> white-space fixeup!
>>>
>>> Signed-off-by: Paul E. McKenney 
>>> Cc: Alexei Starovoitov 
>>> Cc: Andrii Nakryiko 
>>> Cc: Peter Zijlstra 
>>> Cc: Kent Overstreet 
>>> Cc: 
>>> ---
>>>  include/linux/srcu.h | 21 ++---
>>>  include/linux/srcutree.h |  2 +-
>>>  kernel/rcu/srcutree.c| 22 +++---
>>>  3 files changed, 22 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
>>> index 835bbb2d1f88a..06728ef6f32a4 100644
>>> --- a/include/linux/srcu.h
>>> +++ b/include/linux/srcu.h
>>> @@ -181,10 +181,9 @@ static inline int srcu_read_lock_held(const struct 
>>> srcu_struct *ssp)
>>>  #define SRCU_NMI_SAFE  0x2
>>>  
>>>  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
>>> -void srcu_check_nmi_safety(struct srcu_struct *ssp, bool nmi_safe);
>>> +void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
>>>  #else
>>> -static inline void srcu_check_nmi_safety(struct srcu_struct *ssp,
>>> -bool nmi_safe) { }
>>> +static inline void srcu_check_read_flavor(struct srcu_struct *ssp, int 
>>> read_flavor) { }
>>>  #endif
>>>  
>>>  
>>> @@ -245,7 +244,7 @@ static inline int srcu_read_lock(struct srcu_struct 
>>> *ssp) __acquires(ssp)
>>>  {
>>> int retval;
>>>  
>>> -   srcu_check_nmi_safety(ssp, false);
>>> +   srcu_check_read_flavor(ssp, false);
>>
>> As srcu_check_read_flavor() takes an "int" now, passing a macro for the type 
>> of reader would
>> be better here?
> 
> Agreed, and a later commit does introduce macros.
> 
>>> retval = __srcu_read_lock(ssp);
>>> srcu_lock_acquire(&ssp->dep_map);
>>> return retval;
>>> @@ -262,7 +261,7 @@ static inline int srcu_read_lock_nmisafe(struct 
>>> srcu_struct *ssp) __acquires(ssp
>>>  {
>>> int retval;
>>>  
>>> -   srcu_check_nmi_safety(ssp, true);
>>> +   srcu_check_read_flavor(ssp, true);
>>> retval = __srcu_read_lock_nmisafe(ssp);
>>> rcu_try_lock_acquire(&ssp->dep_map);
>>> return retval;
>>> @@ -274,7 +273,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
>>> __acquires(ssp)
>>>  {
>>> int retval;
>>>  
>>> -   srcu_check_nmi_safety(ssp, false);
>>> +   srcu_check_read_flavor(ssp, false);
>>> retval = __srcu_read_lock(ssp);
>>> return retval;
>>>  }
>>> @@ -303,7 +302,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_nmi_safety(ssp, false);
>>> +   srcu_check_read_flavor(ssp, false);
>>> return __srcu_read_lock(ssp);
>>>  }
>>>  
>>> @@ -318,7 +317,7 @@ static inline void srcu_read_unlock(struct srcu_struct 
>>> *ssp, int idx)
>>> __releases(ssp)
>>>  {
>>> WARN_ON_ONCE(idx & ~0x1);
>>> -   srcu_check_nmi_safety(ssp, false);
>>> +   srcu_check_read_flavor(ssp, false);
>>> srcu_lock_release(&ssp->dep_map);
>>> __srcu_read_unlock(ssp, idx);
>>>  }
>>> @@ -334,7 +333,7 @@ static inline void srcu_read_unlock_nmisafe(struct 
>>> srcu_struct *ssp, int idx)
>>> __releases(ssp)
>>>  {
>>> WARN_ON_ONCE(idx & ~0x1);
>>> -   srcu_check_nmi_safety(ssp, true);
>>> +   srcu_check_read_flavor(ssp, true);
>>> rcu_lock_release(&ssp->dep_map);
>>> __srcu_read_unlock_nmisafe(ssp, idx);
>>>  }
>>> @@ -343,7 +342,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_nmi_safety(ssp, false);
>>> +   srcu_check_read_flavor(ssp, false);
>>> __srcu_read_unlock(ssp, idx);
>>>  }
>>>  
>>> @@ -360,7 +359,7 @@ static inline void srcu_up_read(struct srcu_struct 
>>> *ssp, int idx)
>>>  {
>>> WARN_ON_ONCE(idx & ~0x1);
>>> WARN_ON_ONCE(in_nmi());
>>> -   srcu_check_nmi_safety(ssp, false);
>>> +   srcu_check_read_flavor(ssp, false);
>>> __srcu_read_unlock(ssp, idx);
>>>  }
>>>  
>>> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
>>> index ed57598394de3..ab7d8d215b84b 100644
>>> --- a/include/linux/srcutree.h
>>> +++ b/include/linux/srcutree.h
>>> @@ -25,7 +25,7 @@ struct srcu_data {
>>> /* Read-side state. */
>>> atomic_long_t srcu_lock_count[2];   /* Locks per CPU. */
>>> atomic_long_t srcu_unlock_count[2]; /* Unlocks per CPU. */
>>> -   int srcu_nmi_safety;/* NMI-safe srcu_struct 
>>> structure? */
>>> +   int src

Re: [PATCH rcu 03/12] srcu: Renaming in preparation for additional reader flavor

2024-10-14 Thread Paul E. McKenney
On Mon, Oct 14, 2024 at 02:40:35PM +0530, Neeraj Upadhyay wrote:
> On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
> > Currently, there are only two flavors of readers, normal and NMI-safe.
> > A number of fields, functions, and types reflect this restriction.
> > This renaming-only commit prepares for the addition of light-weight
> > (as in memory-barrier-free) readers.  OK, OK, there is also a drive-by
> > white-space fixeup!
> > 
> > Signed-off-by: Paul E. McKenney 
> > Cc: Alexei Starovoitov 
> > Cc: Andrii Nakryiko 
> > Cc: Peter Zijlstra 
> > Cc: Kent Overstreet 
> > Cc: 
> > ---
> >  include/linux/srcu.h | 21 ++---
> >  include/linux/srcutree.h |  2 +-
> >  kernel/rcu/srcutree.c| 22 +++---
> >  3 files changed, 22 insertions(+), 23 deletions(-)
> > 
> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index 835bbb2d1f88a..06728ef6f32a4 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -181,10 +181,9 @@ static inline int srcu_read_lock_held(const struct 
> > srcu_struct *ssp)
> >  #define SRCU_NMI_SAFE  0x2
> >  
> >  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
> > -void srcu_check_nmi_safety(struct srcu_struct *ssp, bool nmi_safe);
> > +void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> >  #else
> > -static inline void srcu_check_nmi_safety(struct srcu_struct *ssp,
> > -bool nmi_safe) { }
> > +static inline void srcu_check_read_flavor(struct srcu_struct *ssp, int 
> > read_flavor) { }
> >  #endif
> >  
> >  
> > @@ -245,7 +244,7 @@ static inline int srcu_read_lock(struct srcu_struct 
> > *ssp) __acquires(ssp)
> >  {
> > int retval;
> >  
> > -   srcu_check_nmi_safety(ssp, false);
> > +   srcu_check_read_flavor(ssp, false);
> 
> As srcu_check_read_flavor() takes an "int" now, passing a macro for the type 
> of reader would
> be better here?

Agreed, and a later commit does introduce macros.

> > retval = __srcu_read_lock(ssp);
> > srcu_lock_acquire(&ssp->dep_map);
> > return retval;
> > @@ -262,7 +261,7 @@ static inline int srcu_read_lock_nmisafe(struct 
> > srcu_struct *ssp) __acquires(ssp
> >  {
> > int retval;
> >  
> > -   srcu_check_nmi_safety(ssp, true);
> > +   srcu_check_read_flavor(ssp, true);
> > retval = __srcu_read_lock_nmisafe(ssp);
> > rcu_try_lock_acquire(&ssp->dep_map);
> > return retval;
> > @@ -274,7 +273,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> > __acquires(ssp)
> >  {
> > int retval;
> >  
> > -   srcu_check_nmi_safety(ssp, false);
> > +   srcu_check_read_flavor(ssp, false);
> > retval = __srcu_read_lock(ssp);
> > return retval;
> >  }
> > @@ -303,7 +302,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_nmi_safety(ssp, false);
> > +   srcu_check_read_flavor(ssp, false);
> > return __srcu_read_lock(ssp);
> >  }
> >  
> > @@ -318,7 +317,7 @@ static inline void srcu_read_unlock(struct srcu_struct 
> > *ssp, int idx)
> > __releases(ssp)
> >  {
> > WARN_ON_ONCE(idx & ~0x1);
> > -   srcu_check_nmi_safety(ssp, false);
> > +   srcu_check_read_flavor(ssp, false);
> > srcu_lock_release(&ssp->dep_map);
> > __srcu_read_unlock(ssp, idx);
> >  }
> > @@ -334,7 +333,7 @@ static inline void srcu_read_unlock_nmisafe(struct 
> > srcu_struct *ssp, int idx)
> > __releases(ssp)
> >  {
> > WARN_ON_ONCE(idx & ~0x1);
> > -   srcu_check_nmi_safety(ssp, true);
> > +   srcu_check_read_flavor(ssp, true);
> > rcu_lock_release(&ssp->dep_map);
> > __srcu_read_unlock_nmisafe(ssp, idx);
> >  }
> > @@ -343,7 +342,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_nmi_safety(ssp, false);
> > +   srcu_check_read_flavor(ssp, false);
> > __srcu_read_unlock(ssp, idx);
> >  }
> >  
> > @@ -360,7 +359,7 @@ static inline void srcu_up_read(struct srcu_struct 
> > *ssp, int idx)
> >  {
> > WARN_ON_ONCE(idx & ~0x1);
> > WARN_ON_ONCE(in_nmi());
> > -   srcu_check_nmi_safety(ssp, false);
> > +   srcu_check_read_flavor(ssp, false);
> > __srcu_read_unlock(ssp, idx);
> >  }
> >  
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index ed57598394de3..ab7d8d215b84b 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -25,7 +25,7 @@ struct srcu_data {
> > /* Read-side state. */
> > atomic_long_t srcu_lock_count[2];   /* Locks per CPU. */
> > atomic_long_t srcu_unlock_count[2]; /* Unlocks per CPU. */
> > -   int srcu_nmi_safety;/* NMI-safe srcu_struct 
> > structure? */
> > +   int srcu_reader_flavor; /* Reader flavor for 
> > sr

Re: [PATCH rcu 03/12] srcu: Renaming in preparation for additional reader flavor

2024-10-14 Thread Neeraj Upadhyay
On 10/9/2024 11:37 PM, Paul E. McKenney wrote:
> Currently, there are only two flavors of readers, normal and NMI-safe.
> A number of fields, functions, and types reflect this restriction.
> This renaming-only commit prepares for the addition of light-weight
> (as in memory-barrier-free) readers.  OK, OK, there is also a drive-by
> white-space fixeup!
> 
> Signed-off-by: Paul E. McKenney 
> Cc: Alexei Starovoitov 
> Cc: Andrii Nakryiko 
> Cc: Peter Zijlstra 
> Cc: Kent Overstreet 
> Cc: 
> ---
>  include/linux/srcu.h | 21 ++---
>  include/linux/srcutree.h |  2 +-
>  kernel/rcu/srcutree.c| 22 +++---
>  3 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 835bbb2d1f88a..06728ef6f32a4 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -181,10 +181,9 @@ static inline int srcu_read_lock_held(const struct 
> srcu_struct *ssp)
>  #define SRCU_NMI_SAFE0x2
>  
>  #if defined(CONFIG_PROVE_RCU) && defined(CONFIG_TREE_SRCU)
> -void srcu_check_nmi_safety(struct srcu_struct *ssp, bool nmi_safe);
> +void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
>  #else
> -static inline void srcu_check_nmi_safety(struct srcu_struct *ssp,
> -  bool nmi_safe) { }
> +static inline void srcu_check_read_flavor(struct srcu_struct *ssp, int 
> read_flavor) { }
>  #endif
>  
>  
> @@ -245,7 +244,7 @@ static inline int srcu_read_lock(struct srcu_struct *ssp) 
> __acquires(ssp)
>  {
>   int retval;
>  
> - srcu_check_nmi_safety(ssp, false);
> + srcu_check_read_flavor(ssp, false);

As srcu_check_read_flavor() takes an "int" now, passing a macro for the type of 
reader would
be better here?


>   retval = __srcu_read_lock(ssp);
>   srcu_lock_acquire(&ssp->dep_map);
>   return retval;
> @@ -262,7 +261,7 @@ static inline int srcu_read_lock_nmisafe(struct 
> srcu_struct *ssp) __acquires(ssp
>  {
>   int retval;
>  
> - srcu_check_nmi_safety(ssp, true);
> + srcu_check_read_flavor(ssp, true);
>   retval = __srcu_read_lock_nmisafe(ssp);
>   rcu_try_lock_acquire(&ssp->dep_map);
>   return retval;
> @@ -274,7 +273,7 @@ srcu_read_lock_notrace(struct srcu_struct *ssp) 
> __acquires(ssp)
>  {
>   int retval;
>  
> - srcu_check_nmi_safety(ssp, false);
> + srcu_check_read_flavor(ssp, false);
>   retval = __srcu_read_lock(ssp);
>   return retval;
>  }
> @@ -303,7 +302,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_nmi_safety(ssp, false);
> + srcu_check_read_flavor(ssp, false);
>   return __srcu_read_lock(ssp);
>  }
>  
> @@ -318,7 +317,7 @@ static inline void srcu_read_unlock(struct srcu_struct 
> *ssp, int idx)
>   __releases(ssp)
>  {
>   WARN_ON_ONCE(idx & ~0x1);
> - srcu_check_nmi_safety(ssp, false);
> + srcu_check_read_flavor(ssp, false);
>   srcu_lock_release(&ssp->dep_map);
>   __srcu_read_unlock(ssp, idx);
>  }
> @@ -334,7 +333,7 @@ static inline void srcu_read_unlock_nmisafe(struct 
> srcu_struct *ssp, int idx)
>   __releases(ssp)
>  {
>   WARN_ON_ONCE(idx & ~0x1);
> - srcu_check_nmi_safety(ssp, true);
> + srcu_check_read_flavor(ssp, true);
>   rcu_lock_release(&ssp->dep_map);
>   __srcu_read_unlock_nmisafe(ssp, idx);
>  }
> @@ -343,7 +342,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_nmi_safety(ssp, false);
> + srcu_check_read_flavor(ssp, false);
>   __srcu_read_unlock(ssp, idx);
>  }
>  
> @@ -360,7 +359,7 @@ static inline void srcu_up_read(struct srcu_struct *ssp, 
> int idx)
>  {
>   WARN_ON_ONCE(idx & ~0x1);
>   WARN_ON_ONCE(in_nmi());
> - srcu_check_nmi_safety(ssp, false);
> + srcu_check_read_flavor(ssp, false);
>   __srcu_read_unlock(ssp, idx);
>  }
>  
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index ed57598394de3..ab7d8d215b84b 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -25,7 +25,7 @@ struct srcu_data {
>   /* Read-side state. */
>   atomic_long_t srcu_lock_count[2];   /* Locks per CPU. */
>   atomic_long_t srcu_unlock_count[2]; /* Unlocks per CPU. */
> - int srcu_nmi_safety;/* NMI-safe srcu_struct 
> structure? */
> + int srcu_reader_flavor; /* Reader flavor for 
> srcu_struct structure? */

This is a mask for the reader flavor, so 
s/srcu_reader_flavor/srcu_reader_flavor_mask ?


- Neeraj

>  
>   /* Update-side state. */
>   spinlock_t __private lock cacheline_internodealigned_in_smp;
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/s