On Sun, Feb 23, 2025 at 08:43:09PM -0800, Boqun Feng wrote:
> From: "Paul E. McKenney" <[email protected]>
>
> The rcutorture_one_extend_check() function's last check assumes that
> if cur_ops->readlock_nesting() returns greater than zero, either the
> RCUTORTURE_RDR_RCU_1 or the RCUTORTURE_RDR_RCU_2 bit must be set, that
> is, there must be at least one rcu_read_lock() in effect.
>
> This works for preemptible RCU and for non-preemptible RCU running in
> a non-preemptible kernel. But it fails for non-preemptible RCU running
> in a preemptible kernel because then RCU's cur_ops->readlock_nesting()
> function, which is rcu_torture_readlock_nesting(), will return
> the PREEMPT_MASK mask bits from preempt_count(). The result will
> be greater than zero if preemption is disabled, including by the
> RCUTORTURE_RDR_PREEMPT and RCUTORTURE_RDR_SCHED bits.
>
> This commit therefore adjusts this check to take into account the case
> fo non-preemptible RCU running in a preemptible kernel.
>
> [boqun: Fix the if condition and add comment]
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-lkp/[email protected]
> Co-developed-by: Boqun Feng <[email protected]>
> Signed-off-by: Boqun Feng <[email protected]>
> Co-developed-by: Joel Fernandes <[email protected]>
> Signed-off-by: Joel Fernandes <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Boqun Feng <[email protected]>
> ---
> kernel/rcu/rcutorture.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index d26fb1d33ed9..280bff706017 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -1873,6 +1873,8 @@ static void rcu_torture_reader_do_mbchk(long myid,
> struct rcu_torture *rtp,
> #define ROEC_ARGS "%s %s: Current %#x To add %#x To remove %#x
> preempt_count() %#x\n", __func__, s, curstate, new, old, preempt_count()
> static void rcutorture_one_extend_check(char *s, int curstate, int new, int
> old, bool insoftirq)
> {
> + int mask;
> +
> if (!IS_ENABLED(CONFIG_RCU_TORTURE_TEST_CHK_RDR_STATE))
> return;
>
> @@ -1902,8 +1904,16 @@ static void rcutorture_one_extend_check(char *s, int
> curstate, int new, int old,
> WARN_ONCE(cur_ops->extendables &&
> !(curstate & (RCUTORTURE_RDR_PREEMPT | RCUTORTURE_RDR_SCHED))
> &&
> (preempt_count() & PREEMPT_MASK), ROEC_ARGS);
> - WARN_ONCE(cur_ops->readlock_nesting &&
> - !(curstate & (RCUTORTURE_RDR_RCU_1 | RCUTORTURE_RDR_RCU_2)) &&
> +
> + /*
> + * non-preemptible RCU in a preemptible kernel uses "preempt_count() &
> + * PREEMPT_MASK" as ->readlock_nesting().
> + */
> + mask = RCUTORTURE_RDR_RCU_1 | RCUTORTURE_RDR_RCU_2;
> + if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
Good catch, thank you, and it looks good to me!
Oliver, you are right, I was looking at the wrong console output.
One of those days, I guess... :-/
Thanx, Paul
> + mask |= RCUTORTURE_RDR_PREEMPT | RCUTORTURE_RDR_SCHED;
> +
> + WARN_ONCE(cur_ops->readlock_nesting && !(curstate & mask) &&
> cur_ops->readlock_nesting() > 0, ROEC_ARGS);
> }
>
> --
> 2.39.5 (Apple Git-154)
>
