This commit changes a few "cpuc" variables to "sdp" to align wiht usage
elsewhere.
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: <bpf@vger.kernel.org>
---
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);
}
@@ -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_up_read(struct srcu_struct *ssp, int idx)
{
WARN_ON_ONCE(idx & ~0x1);
WARN_ON_ONCE(in_nmi());
- srcu_check_read_flavor(ssp, false);
+ srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
__srcu_read_unlock(ssp, idx);
}
diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index ab7d8d215b84b..79ad809c7f035 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -43,6 +43,10 @@ struct srcu_data {
struct srcu_struct *ssp;
};
+/* Values for ->srcu_reader_flavor. */
+#define SRCU_READ_FLAVOR_NORMAL 0x1 // srcu_read_lock().
+#define SRCU_READ_FLAVOR_NMI 0x2 // srcu_read_lock_nmisafe().
+
/*
* Node in SRCU combining tree, similar in function to rcu_data.
*/
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index abe55777c4335..4c51be484b48a 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -438,9 +438,9 @@ static unsigned long srcu_readers_lock_idx(struct srcu_struct *ssp, int idx)
unsigned long sum = 0;
for_each_possible_cpu(cpu) {
- struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
+ struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
- sum += atomic_long_read(&cpuc->srcu_lock_count[idx]);
+ sum += atomic_long_read(&sdp->srcu_lock_count[idx]);
}
return sum;
}
@@ -456,14 +456,14 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx)
unsigned long sum = 0;
for_each_possible_cpu(cpu) {
- struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
+ struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
- sum += atomic_long_read(&cpuc->srcu_unlock_count[idx]);
+ sum += atomic_long_read(&sdp->srcu_unlock_count[idx]);
if (IS_ENABLED(CONFIG_PROVE_RCU))
- mask = mask | READ_ONCE(cpuc->srcu_reader_flavor);
+ mask = mask | READ_ONCE(sdp->srcu_reader_flavor);
}
WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)),
- "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp);
+ "Mixed reader flavors for srcu_struct at %ps.\n", ssp);
return sum;
}
@@ -564,12 +564,12 @@ static bool srcu_readers_active(struct srcu_struct *ssp)
unsigned long sum = 0;
for_each_possible_cpu(cpu) {
- struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
+ struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
- sum += atomic_long_read(&cpuc->srcu_lock_count[0]);
- sum += atomic_long_read(&cpuc->srcu_lock_count[1]);
- sum -= atomic_long_read(&cpuc->srcu_unlock_count[0]);
- sum -= atomic_long_read(&cpuc->srcu_unlock_count[1]);
+ sum += atomic_long_read(&sdp->srcu_lock_count[0]);
+ sum += atomic_long_read(&sdp->srcu_lock_count[1]);
+ sum -= atomic_long_read(&sdp->srcu_unlock_count[0]);
+ sum -= atomic_long_read(&sdp->srcu_unlock_count[1]);
}
return sum;
}
@@ -703,20 +703,21 @@ EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
*/
void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
{
- int reader_flavor_mask = 1 << read_flavor;
- int old_reader_flavor_mask;
+ int old_read_flavor;
struct srcu_data *sdp;
- /* NMI-unsafe use in NMI is a bad sign */
- WARN_ON_ONCE(!read_flavor && in_nmi());
+ /* NMI-unsafe use in NMI is a bad sign, as is multi-bit read_flavor values. */
+ WARN_ON_ONCE((read_flavor != SRCU_READ_FLAVOR_NMI) && in_nmi());
+ WARN_ON_ONCE(read_flavor & (read_flavor - 1));
+
sdp = raw_cpu_ptr(ssp->sda);
- old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor);
- if (!old_reader_flavor_mask) {
- old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask);
- if (!old_reader_flavor_mask)
+ old_read_flavor = READ_ONCE(sdp->srcu_reader_flavor);
+ if (!old_read_flavor) {
+ old_read_flavor = cmpxchg(&sdp->srcu_reader_flavor, 0, read_flavor);
+ if (!old_read_flavor)
return;
}
- WARN_ONCE(old_reader_flavor_mask != reader_flavor_mask, "CPU %d old state %d new state %d\n", sdp->cpu, old_reader_flavor_mask, reader_flavor_mask);
+ WARN_ONCE(old_read_flavor != read_flavor, "CPU %d old state %d new state %d\n", sdp->cpu, old_read_flavor, read_flavor);
}
EXPORT_SYMBOL_GPL(srcu_check_read_flavor);
#endif /* CONFIG_PROVE_RCU */
--
2.40.1On 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 <paulmck@kernel.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: <bpf@vger.kernel.org>
> ---
> 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);
> }
>
> @@ -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_up_read(struct srcu_struct *ssp, int idx)
> {
> WARN_ON_ONCE(idx & ~0x1);
> WARN_ON_ONCE(in_nmi());
> - srcu_check_read_flavor(ssp, false);
> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> __srcu_read_unlock(ssp, idx);
> }
>
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index ab7d8d215b84b..79ad809c7f035 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -43,6 +43,10 @@ struct srcu_data {
> struct srcu_struct *ssp;
> };
>
> +/* Values for ->srcu_reader_flavor. */
> +#define SRCU_READ_FLAVOR_NORMAL 0x1 // srcu_read_lock().
> +#define SRCU_READ_FLAVOR_NMI 0x2 // srcu_read_lock_nmisafe().
> +
> /*
> * Node in SRCU combining tree, similar in function to rcu_data.
> */
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index abe55777c4335..4c51be484b48a 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -438,9 +438,9 @@ static unsigned long srcu_readers_lock_idx(struct srcu_struct *ssp, int idx)
> unsigned long sum = 0;
>
> for_each_possible_cpu(cpu) {
> - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>
> - sum += atomic_long_read(&cpuc->srcu_lock_count[idx]);
> + sum += atomic_long_read(&sdp->srcu_lock_count[idx]);
> }
> return sum;
> }
> @@ -456,14 +456,14 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx)
> unsigned long sum = 0;
>
> for_each_possible_cpu(cpu) {
> - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>
> - sum += atomic_long_read(&cpuc->srcu_unlock_count[idx]);
> + sum += atomic_long_read(&sdp->srcu_unlock_count[idx]);
> if (IS_ENABLED(CONFIG_PROVE_RCU))
> - mask = mask | READ_ONCE(cpuc->srcu_reader_flavor);
> + mask = mask | READ_ONCE(sdp->srcu_reader_flavor);
> }
> WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)),
> - "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp);
> + "Mixed reader flavors for srcu_struct at %ps.\n", ssp);
> return sum;
> }
>
> @@ -564,12 +564,12 @@ static bool srcu_readers_active(struct srcu_struct *ssp)
> unsigned long sum = 0;
>
> for_each_possible_cpu(cpu) {
> - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>
> - sum += atomic_long_read(&cpuc->srcu_lock_count[0]);
> - sum += atomic_long_read(&cpuc->srcu_lock_count[1]);
> - sum -= atomic_long_read(&cpuc->srcu_unlock_count[0]);
> - sum -= atomic_long_read(&cpuc->srcu_unlock_count[1]);
> + sum += atomic_long_read(&sdp->srcu_lock_count[0]);
> + sum += atomic_long_read(&sdp->srcu_lock_count[1]);
> + sum -= atomic_long_read(&sdp->srcu_unlock_count[0]);
> + sum -= atomic_long_read(&sdp->srcu_unlock_count[1]);
> }
> return sum;
> }
> @@ -703,20 +703,21 @@ EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
> */
> void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
> {
> - int reader_flavor_mask = 1 << read_flavor;
> - int old_reader_flavor_mask;
> + int old_read_flavor;
> struct srcu_data *sdp;
>
> - /* NMI-unsafe use in NMI is a bad sign */
> - WARN_ON_ONCE(!read_flavor && in_nmi());
> + /* NMI-unsafe use in NMI is a bad sign, as is multi-bit read_flavor values. */
> + WARN_ON_ONCE((read_flavor != SRCU_READ_FLAVOR_NMI) && in_nmi());
> + WARN_ON_ONCE(read_flavor & (read_flavor - 1));
> +
> sdp = raw_cpu_ptr(ssp->sda);
> - old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor);
> - if (!old_reader_flavor_mask) {
> - old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask);
> - if (!old_reader_flavor_mask)
> + old_read_flavor = READ_ONCE(sdp->srcu_reader_flavor);
> + if (!old_read_flavor) {
> + old_read_flavor = cmpxchg(&sdp->srcu_reader_flavor, 0, read_flavor);
> + if (!old_read_flavor)
> return;
> }
> - WARN_ONCE(old_reader_flavor_mask != reader_flavor_mask, "CPU %d old state %d new state %d\n", sdp->cpu, old_reader_flavor_mask, reader_flavor_mask);
> + WARN_ONCE(old_read_flavor != read_flavor, "CPU %d old state %d new state %d\n", sdp->cpu, old_read_flavor, read_flavor);
> }
> EXPORT_SYMBOL_GPL(srcu_check_read_flavor);
> #endif /* CONFIG_PROVE_RCU */
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 <paulmck@kernel.org>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Andrii Nakryiko <andrii@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Kent Overstreet <kent.overstreet@linux.dev>
> > Cc: <bpf@vger.kernel.org>
> > ---
> > 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);
> > }
> >
> > @@ -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_up_read(struct srcu_struct *ssp, int idx)
> > {
> > WARN_ON_ONCE(idx & ~0x1);
> > WARN_ON_ONCE(in_nmi());
> > - srcu_check_read_flavor(ssp, false);
> > + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > __srcu_read_unlock(ssp, idx);
> > }
> >
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index ab7d8d215b84b..79ad809c7f035 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -43,6 +43,10 @@ struct srcu_data {
> > struct srcu_struct *ssp;
> > };
> >
> > +/* Values for ->srcu_reader_flavor. */
> > +#define SRCU_READ_FLAVOR_NORMAL 0x1 // srcu_read_lock().
> > +#define SRCU_READ_FLAVOR_NMI 0x2 // srcu_read_lock_nmisafe().
> > +
> > /*
> > * Node in SRCU combining tree, similar in function to rcu_data.
> > */
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index abe55777c4335..4c51be484b48a 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -438,9 +438,9 @@ static unsigned long srcu_readers_lock_idx(struct srcu_struct *ssp, int idx)
> > unsigned long sum = 0;
> >
> > for_each_possible_cpu(cpu) {
> > - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> > + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
> >
> > - sum += atomic_long_read(&cpuc->srcu_lock_count[idx]);
> > + sum += atomic_long_read(&sdp->srcu_lock_count[idx]);
> > }
> > return sum;
> > }
> > @@ -456,14 +456,14 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx)
> > unsigned long sum = 0;
> >
> > for_each_possible_cpu(cpu) {
> > - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> > + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
> >
> > - sum += atomic_long_read(&cpuc->srcu_unlock_count[idx]);
> > + sum += atomic_long_read(&sdp->srcu_unlock_count[idx]);
> > if (IS_ENABLED(CONFIG_PROVE_RCU))
> > - mask = mask | READ_ONCE(cpuc->srcu_reader_flavor);
> > + mask = mask | READ_ONCE(sdp->srcu_reader_flavor);
> > }
> > WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)),
> > - "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp);
> > + "Mixed reader flavors for srcu_struct at %ps.\n", ssp);
> > return sum;
> > }
> >
> > @@ -564,12 +564,12 @@ static bool srcu_readers_active(struct srcu_struct *ssp)
> > unsigned long sum = 0;
> >
> > for_each_possible_cpu(cpu) {
> > - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> > + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
> >
> > - sum += atomic_long_read(&cpuc->srcu_lock_count[0]);
> > - sum += atomic_long_read(&cpuc->srcu_lock_count[1]);
> > - sum -= atomic_long_read(&cpuc->srcu_unlock_count[0]);
> > - sum -= atomic_long_read(&cpuc->srcu_unlock_count[1]);
> > + sum += atomic_long_read(&sdp->srcu_lock_count[0]);
> > + sum += atomic_long_read(&sdp->srcu_lock_count[1]);
> > + sum -= atomic_long_read(&sdp->srcu_unlock_count[0]);
> > + sum -= atomic_long_read(&sdp->srcu_unlock_count[1]);
> > }
> > return sum;
> > }
> > @@ -703,20 +703,21 @@ EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
> > */
> > void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
> > {
> > - int reader_flavor_mask = 1 << read_flavor;
> > - int old_reader_flavor_mask;
> > + int old_read_flavor;
> > struct srcu_data *sdp;
> >
> > - /* NMI-unsafe use in NMI is a bad sign */
> > - WARN_ON_ONCE(!read_flavor && in_nmi());
> > + /* NMI-unsafe use in NMI is a bad sign, as is multi-bit read_flavor values. */
> > + WARN_ON_ONCE((read_flavor != SRCU_READ_FLAVOR_NMI) && in_nmi());
> > + WARN_ON_ONCE(read_flavor & (read_flavor - 1));
> > +
> > sdp = raw_cpu_ptr(ssp->sda);
> > - old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor);
> > - if (!old_reader_flavor_mask) {
> > - old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask);
> > - if (!old_reader_flavor_mask)
> > + old_read_flavor = READ_ONCE(sdp->srcu_reader_flavor);
> > + if (!old_read_flavor) {
> > + old_read_flavor = cmpxchg(&sdp->srcu_reader_flavor, 0, read_flavor);
> > + if (!old_read_flavor)
> > return;
> > }
> > - WARN_ONCE(old_reader_flavor_mask != reader_flavor_mask, "CPU %d old state %d new state %d\n", sdp->cpu, old_reader_flavor_mask, reader_flavor_mask);
> > + WARN_ONCE(old_read_flavor != read_flavor, "CPU %d old state %d new state %d\n", sdp->cpu, old_read_flavor, read_flavor);
> > }
> > EXPORT_SYMBOL_GPL(srcu_check_read_flavor);
> > #endif /* CONFIG_PROVE_RCU */
>
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 <paulmck@kernel.org>
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Andrii Nakryiko <andrii@kernel.org>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Kent Overstreet <kent.overstreet@linux.dev>
> > > Cc: <bpf@vger.kernel.org>
> > > ---
> > > 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);
> > > }
> > >
> > > @@ -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_up_read(struct srcu_struct *ssp, int idx)
> > > {
> > > WARN_ON_ONCE(idx & ~0x1);
> > > WARN_ON_ONCE(in_nmi());
> > > - srcu_check_read_flavor(ssp, false);
> > > + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
> > > __srcu_read_unlock(ssp, idx);
> > > }
> > >
> > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > index ab7d8d215b84b..79ad809c7f035 100644
> > > --- a/include/linux/srcutree.h
> > > +++ b/include/linux/srcutree.h
> > > @@ -43,6 +43,10 @@ struct srcu_data {
> > > struct srcu_struct *ssp;
> > > };
> > >
> > > +/* Values for ->srcu_reader_flavor. */
> > > +#define SRCU_READ_FLAVOR_NORMAL 0x1 // srcu_read_lock().
> > > +#define SRCU_READ_FLAVOR_NMI 0x2 // srcu_read_lock_nmisafe().
> > > +
> > > /*
> > > * Node in SRCU combining tree, similar in function to rcu_data.
> > > */
> > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > index abe55777c4335..4c51be484b48a 100644
> > > --- a/kernel/rcu/srcutree.c
> > > +++ b/kernel/rcu/srcutree.c
> > > @@ -438,9 +438,9 @@ static unsigned long srcu_readers_lock_idx(struct srcu_struct *ssp, int idx)
> > > unsigned long sum = 0;
> > >
> > > for_each_possible_cpu(cpu) {
> > > - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> > > + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
> > >
> > > - sum += atomic_long_read(&cpuc->srcu_lock_count[idx]);
> > > + sum += atomic_long_read(&sdp->srcu_lock_count[idx]);
> > > }
> > > return sum;
> > > }
> > > @@ -456,14 +456,14 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx)
> > > unsigned long sum = 0;
> > >
> > > for_each_possible_cpu(cpu) {
> > > - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> > > + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
> > >
> > > - sum += atomic_long_read(&cpuc->srcu_unlock_count[idx]);
> > > + sum += atomic_long_read(&sdp->srcu_unlock_count[idx]);
> > > if (IS_ENABLED(CONFIG_PROVE_RCU))
> > > - mask = mask | READ_ONCE(cpuc->srcu_reader_flavor);
> > > + mask = mask | READ_ONCE(sdp->srcu_reader_flavor);
> > > }
> > > WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)),
> > > - "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp);
> > > + "Mixed reader flavors for srcu_struct at %ps.\n", ssp);
> > > return sum;
> > > }
> > >
> > > @@ -564,12 +564,12 @@ static bool srcu_readers_active(struct srcu_struct *ssp)
> > > unsigned long sum = 0;
> > >
> > > for_each_possible_cpu(cpu) {
> > > - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
> > > + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
> > >
> > > - sum += atomic_long_read(&cpuc->srcu_lock_count[0]);
> > > - sum += atomic_long_read(&cpuc->srcu_lock_count[1]);
> > > - sum -= atomic_long_read(&cpuc->srcu_unlock_count[0]);
> > > - sum -= atomic_long_read(&cpuc->srcu_unlock_count[1]);
> > > + sum += atomic_long_read(&sdp->srcu_lock_count[0]);
> > > + sum += atomic_long_read(&sdp->srcu_lock_count[1]);
> > > + sum -= atomic_long_read(&sdp->srcu_unlock_count[0]);
> > > + sum -= atomic_long_read(&sdp->srcu_unlock_count[1]);
> > > }
> > > return sum;
> > > }
> > > @@ -703,20 +703,21 @@ EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
> > > */
> > > void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
> > > {
> > > - int reader_flavor_mask = 1 << read_flavor;
> > > - int old_reader_flavor_mask;
> > > + int old_read_flavor;
> > > struct srcu_data *sdp;
> > >
> > > - /* NMI-unsafe use in NMI is a bad sign */
> > > - WARN_ON_ONCE(!read_flavor && in_nmi());
> > > + /* NMI-unsafe use in NMI is a bad sign, as is multi-bit read_flavor values. */
> > > + WARN_ON_ONCE((read_flavor != SRCU_READ_FLAVOR_NMI) && in_nmi());
> > > + WARN_ON_ONCE(read_flavor & (read_flavor - 1));
> > > +
> > > sdp = raw_cpu_ptr(ssp->sda);
> > > - old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor);
> > > - if (!old_reader_flavor_mask) {
> > > - old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask);
> > > - if (!old_reader_flavor_mask)
> > > + old_read_flavor = READ_ONCE(sdp->srcu_reader_flavor);
> > > + if (!old_read_flavor) {
> > > + old_read_flavor = cmpxchg(&sdp->srcu_reader_flavor, 0, read_flavor);
> > > + if (!old_read_flavor)
> > > return;
> > > }
> > > - WARN_ONCE(old_reader_flavor_mask != reader_flavor_mask, "CPU %d old state %d new state %d\n", sdp->cpu, old_reader_flavor_mask, reader_flavor_mask);
> > > + WARN_ONCE(old_read_flavor != read_flavor, "CPU %d old state %d new state %d\n", sdp->cpu, old_read_flavor, read_flavor);
> > > }
> > > EXPORT_SYMBOL_GPL(srcu_check_read_flavor);
> > > #endif /* CONFIG_PROVE_RCU */
> >
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 <paulmck@kernel.org>
>>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>>> Cc: Andrii Nakryiko <andrii@kernel.org>
>>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>>> Cc: Kent Overstreet <kent.overstreet@linux.dev>
>>>> Cc: <bpf@vger.kernel.org>
>>>> ---
>>>> 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);
>>>> }
>>>>
>>>> @@ -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_up_read(struct srcu_struct *ssp, int idx)
>>>> {
>>>> WARN_ON_ONCE(idx & ~0x1);
>>>> WARN_ON_ONCE(in_nmi());
>>>> - srcu_check_read_flavor(ssp, false);
>>>> + srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_NORMAL);
>>>> __srcu_read_unlock(ssp, idx);
>>>> }
>>>>
>>>> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
>>>> index ab7d8d215b84b..79ad809c7f035 100644
>>>> --- a/include/linux/srcutree.h
>>>> +++ b/include/linux/srcutree.h
>>>> @@ -43,6 +43,10 @@ struct srcu_data {
>>>> struct srcu_struct *ssp;
>>>> };
>>>>
>>>> +/* Values for ->srcu_reader_flavor. */
>>>> +#define SRCU_READ_FLAVOR_NORMAL 0x1 // srcu_read_lock().
>>>> +#define SRCU_READ_FLAVOR_NMI 0x2 // srcu_read_lock_nmisafe().
>>>> +
>>>> /*
>>>> * Node in SRCU combining tree, similar in function to rcu_data.
>>>> */
>>>> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
>>>> index abe55777c4335..4c51be484b48a 100644
>>>> --- a/kernel/rcu/srcutree.c
>>>> +++ b/kernel/rcu/srcutree.c
>>>> @@ -438,9 +438,9 @@ static unsigned long srcu_readers_lock_idx(struct srcu_struct *ssp, int idx)
>>>> unsigned long sum = 0;
>>>>
>>>> for_each_possible_cpu(cpu) {
>>>> - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
>>>> + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>>>>
>>>> - sum += atomic_long_read(&cpuc->srcu_lock_count[idx]);
>>>> + sum += atomic_long_read(&sdp->srcu_lock_count[idx]);
>>>> }
>>>> return sum;
>>>> }
>>>> @@ -456,14 +456,14 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx)
>>>> unsigned long sum = 0;
>>>>
>>>> for_each_possible_cpu(cpu) {
>>>> - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
>>>> + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>>>>
>>>> - sum += atomic_long_read(&cpuc->srcu_unlock_count[idx]);
>>>> + sum += atomic_long_read(&sdp->srcu_unlock_count[idx]);
>>>> if (IS_ENABLED(CONFIG_PROVE_RCU))
>>>> - mask = mask | READ_ONCE(cpuc->srcu_reader_flavor);
>>>> + mask = mask | READ_ONCE(sdp->srcu_reader_flavor);
>>>> }
>>>> WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)),
>>>> - "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp);
>>>> + "Mixed reader flavors for srcu_struct at %ps.\n", ssp);
>>>> return sum;
>>>> }
>>>>
>>>> @@ -564,12 +564,12 @@ static bool srcu_readers_active(struct srcu_struct *ssp)
>>>> unsigned long sum = 0;
>>>>
>>>> for_each_possible_cpu(cpu) {
>>>> - struct srcu_data *cpuc = per_cpu_ptr(ssp->sda, cpu);
>>>> + struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>>>>
>>>> - sum += atomic_long_read(&cpuc->srcu_lock_count[0]);
>>>> - sum += atomic_long_read(&cpuc->srcu_lock_count[1]);
>>>> - sum -= atomic_long_read(&cpuc->srcu_unlock_count[0]);
>>>> - sum -= atomic_long_read(&cpuc->srcu_unlock_count[1]);
>>>> + sum += atomic_long_read(&sdp->srcu_lock_count[0]);
>>>> + sum += atomic_long_read(&sdp->srcu_lock_count[1]);
>>>> + sum -= atomic_long_read(&sdp->srcu_unlock_count[0]);
>>>> + sum -= atomic_long_read(&sdp->srcu_unlock_count[1]);
>>>> }
>>>> return sum;
>>>> }
>>>> @@ -703,20 +703,21 @@ EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
>>>> */
>>>> void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
>>>> {
>>>> - int reader_flavor_mask = 1 << read_flavor;
>>>> - int old_reader_flavor_mask;
>>>> + int old_read_flavor;
>>>> struct srcu_data *sdp;
>>>>
>>>> - /* NMI-unsafe use in NMI is a bad sign */
>>>> - WARN_ON_ONCE(!read_flavor && in_nmi());
>>>> + /* NMI-unsafe use in NMI is a bad sign, as is multi-bit read_flavor values. */
>>>> + WARN_ON_ONCE((read_flavor != SRCU_READ_FLAVOR_NMI) && in_nmi());
>>>> + WARN_ON_ONCE(read_flavor & (read_flavor - 1));
>>>> +
>>>> sdp = raw_cpu_ptr(ssp->sda);
>>>> - old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor);
>>>> - if (!old_reader_flavor_mask) {
>>>> - old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask);
>>>> - if (!old_reader_flavor_mask)
>>>> + old_read_flavor = READ_ONCE(sdp->srcu_reader_flavor);
>>>> + if (!old_read_flavor) {
>>>> + old_read_flavor = cmpxchg(&sdp->srcu_reader_flavor, 0, read_flavor);
>>>> + if (!old_read_flavor)
>>>> return;
>>>> }
>>>> - WARN_ONCE(old_reader_flavor_mask != reader_flavor_mask, "CPU %d old state %d new state %d\n", sdp->cpu, old_reader_flavor_mask, reader_flavor_mask);
>>>> + WARN_ONCE(old_read_flavor != read_flavor, "CPU %d old state %d new state %d\n", sdp->cpu, old_read_flavor, read_flavor);
>>>> }
>>>> EXPORT_SYMBOL_GPL(srcu_check_read_flavor);
>>>> #endif /* CONFIG_PROVE_RCU */
>>>
© 2016 - 2026 Red Hat, Inc.