| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d0ec401f-f857-4fbb-89f3-f2d13eb34b5d@amd.com>
Date: Mon, 14 Oct 2024 14:45:50 +0530
From: Neeraj Upadhyay <Neeraj.Upadhyay@....com>
To: "Paul E. McKenney" <paulmck@...nel.org>, frederic@...nel.org,
rcu@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, kernel-team@...a.com, rostedt@...dmis.org,
Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Kent Overstreet <kent.overstreet@...ux.dev>, bpf@...r.kernel.org
Subject: Re: [PATCH rcu 05/12] srcu: Standardize srcu_data pointers to "sdp"
and similar
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 <paulmck@...nel.org>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Andrii Nakryiko <andrii@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Kent Overstreet <kent.overstreet@...ux.dev>
> Cc: <bpf@...r.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 */
Powered by blists - more mailing lists