| Message ID | 20241009180719.778285-4-paulmck@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [rcu,01/12] srcu: Rename srcu_might_be_idle() to srcu_should_expedite() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 10/9/2024 11:37 PM, Paul E. McKenney wrote: > Currently, there are only two flavors of readers, normal and NMI-safe. > Very straightforward state updates suffice to check for erroneous > mixing of reader flavors on a given srcu_struct structure. This commit > upgrades the checking in preparation for the addition of light-weight > (as in memory-barrier-free) readers. > > 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> > --- > kernel/rcu/srcutree.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > index 18f2eae5e14bd..abe55777c4335 100644 > --- a/kernel/rcu/srcutree.c > +++ b/kernel/rcu/srcutree.c > @@ -462,7 +462,7 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx) > if (IS_ENABLED(CONFIG_PROVE_RCU)) > mask = mask | READ_ONCE(cpuc->srcu_reader_flavor); > } > - WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask >> 1)), > + WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)), > "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp); > return sum; > } > @@ -712,8 +712,9 @@ void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor) > sdp = raw_cpu_ptr(ssp->sda); > old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor); > if (!old_reader_flavor_mask) { > - WRITE_ONCE(sdp->srcu_reader_flavor, reader_flavor_mask); > - return; > + old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask); This looks to be separate independent fix? - Neeraj > + if (!old_reader_flavor_mask) > + 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); > }
On Mon, Oct 14, 2024 at 02:42:33PM +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. > > Very straightforward state updates suffice to check for erroneous > > mixing of reader flavors on a given srcu_struct structure. This commit > > upgrades the checking in preparation for the addition of light-weight > > (as in memory-barrier-free) readers. > > > > 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> > > --- > > kernel/rcu/srcutree.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > > index 18f2eae5e14bd..abe55777c4335 100644 > > --- a/kernel/rcu/srcutree.c > > +++ b/kernel/rcu/srcutree.c > > @@ -462,7 +462,7 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx) > > if (IS_ENABLED(CONFIG_PROVE_RCU)) > > mask = mask | READ_ONCE(cpuc->srcu_reader_flavor); > > } > > - WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask >> 1)), > > + WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)), > > "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp); > > return sum; > > } > > @@ -712,8 +712,9 @@ void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor) > > sdp = raw_cpu_ptr(ssp->sda); > > old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor); > > if (!old_reader_flavor_mask) { > > - WRITE_ONCE(sdp->srcu_reader_flavor, reader_flavor_mask); > > - return; > > + old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask); > > This looks to be separate independent fix? I would say that it is part of the upgrade. The old logic worked if there are only two flavors, but the cmpxchg() is required for more than two. Thanx, Paul > - Neeraj > > > + if (!old_reader_flavor_mask) > > + 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); > > } >
On 10/14/2024 10:21 PM, Paul E. McKenney wrote: > On Mon, Oct 14, 2024 at 02:42:33PM +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. >>> Very straightforward state updates suffice to check for erroneous >>> mixing of reader flavors on a given srcu_struct structure. This commit >>> upgrades the checking in preparation for the addition of light-weight >>> (as in memory-barrier-free) readers. >>> >>> 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> >>> --- >>> kernel/rcu/srcutree.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c >>> index 18f2eae5e14bd..abe55777c4335 100644 >>> --- a/kernel/rcu/srcutree.c >>> +++ b/kernel/rcu/srcutree.c >>> @@ -462,7 +462,7 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx) >>> if (IS_ENABLED(CONFIG_PROVE_RCU)) >>> mask = mask | READ_ONCE(cpuc->srcu_reader_flavor); >>> } >>> - WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask >> 1)), >>> + WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)), >>> "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp); >>> return sum; >>> } >>> @@ -712,8 +712,9 @@ void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor) >>> sdp = raw_cpu_ptr(ssp->sda); >>> old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor); >>> if (!old_reader_flavor_mask) { >>> - WRITE_ONCE(sdp->srcu_reader_flavor, reader_flavor_mask); >>> - return; >>> + old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask); >> >> This looks to be separate independent fix? > > I would say that it is part of the upgrade. The old logic worked if there > are only two flavors, but the cmpxchg() is required for more than two. > Ok, I need to check more to understand why it is not required when we have only two flavors. - Neeraj > Thanx, Paul > >> - Neeraj >> >>> + if (!old_reader_flavor_mask) >>> + 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); >>> } >>
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c index 18f2eae5e14bd..abe55777c4335 100644 --- a/kernel/rcu/srcutree.c +++ b/kernel/rcu/srcutree.c @@ -462,7 +462,7 @@ static unsigned long srcu_readers_unlock_idx(struct srcu_struct *ssp, int idx) if (IS_ENABLED(CONFIG_PROVE_RCU)) mask = mask | READ_ONCE(cpuc->srcu_reader_flavor); } - WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask >> 1)), + WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)), "Mixed NMI-safe readers for srcu_struct at %ps.\n", ssp); return sum; } @@ -712,8 +712,9 @@ void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor) sdp = raw_cpu_ptr(ssp->sda); old_reader_flavor_mask = READ_ONCE(sdp->srcu_reader_flavor); if (!old_reader_flavor_mask) { - WRITE_ONCE(sdp->srcu_reader_flavor, reader_flavor_mask); - return; + old_reader_flavor_mask = cmpxchg(&sdp->srcu_reader_flavor, 0, reader_flavor_mask); + if (!old_reader_flavor_mask) + 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); }
Currently, there are only two flavors of readers, normal and NMI-safe. Very straightforward state updates suffice to check for erroneous mixing of reader flavors on a given srcu_struct structure. This commit upgrades the checking in preparation for the addition of light-weight (as in memory-barrier-free) readers. 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> --- kernel/rcu/srcutree.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)