linux-kernel - Re: [PATCH V3] rcu: Update jiffies locally in rcu_cpu_stall

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230822153416.GA72567@google.com>
Date:   Tue, 22 Aug 2023 15:34:16 +0000
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Huacai Chen <chenhuacai@...ngson.cn>
Cc:     "Paul E . McKenney" <paulmck@...nel.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Neeraj Upadhyay <quic_neeraju@...cinc.com>,
        Josh Triplett <josh@...htriplett.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Zqiang <qiang.zhang1211@...il.com>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        chenhuacai@...nel.org, rcu@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Binbin Zhou <zhoubinbin@...ngson.cn>
Subject: Re: [PATCH V3] rcu: Update jiffies locally in rcu_cpu_stall_reset()

On Tue, Aug 22, 2023 at 12:02:48PM +0800, Huacai Chen wrote:
> The KGDB initial breakpoint gets an rcu stall warning after commit
> a80be428fbc1f1f3bc9ed924 ("rcu: Do not disable GP stall detection in
> rcu_cpu_stall_reset()").
> 
> [   53.452051] rcu: INFO: rcu_preempt self-detected stall on CPU
[...]
> 
> [1] https://lore.kernel.org/rcu/20230814020045.51950-1-chenhuacai@loongson.cn/T/#t
> 
> Cc: stable@...r.kernel.org
> Fixes: a80be428fbc1f1f3bc9ed924 ("rcu: Do not disable GP stall detection in rcu_cpu_stall_reset()")
> Reported-off-by: Binbin Zhou <zhoubinbin@...ngson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> ---
> V2: Use NMI safe functions.
> V3: Add comments to explain why.
> 
>  kernel/rcu/tree_stall.h | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index b10b8349bb2a..e4e53113d062 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -150,11 +150,26 @@ static void panic_on_rcu_stall(void)
>   * rcu_cpu_stall_reset - restart stall-warning timeout for current grace period
>   *
>   * The caller must disable hard irqs.
> + *
> + * The jiffies updating may be delayed for a very long time due to tickless and
> + * irq disabled, especially in the KGDB case, so we need to add the delayed time
> + * (delta) to rcu_state.jiffies_stall.
> + *
> + * This function may be called in NMI context, so we cannot use ktime_get_ns()
> + * and ktime_get_coarse_ns(). Instead, we use their inaccurate but safe friends
> + * ktime_get_mono_fast_ns() and ktime_get_seconds() which will cause rcu_state.
> + * jiffies_stall to be a little large than expected (harmless and safer).
>   */
>  void rcu_cpu_stall_reset(void)
>  {
> +	u64 curr, last, delta;
> +
> +	curr = ktime_get_mono_fast_ns();
> +	last = ktime_get_seconds() * NSEC_PER_SEC;
> +	delta = nsecs_to_jiffies(curr - last);
> +
>  	WRITE_ONCE(rcu_state.jiffies_stall,
> -		   jiffies + rcu_jiffies_till_stall_check());
> +		   jiffies + delta + rcu_jiffies_till_stall_check());
>  }

I prefer the following diff on top of your patch to take advantage of UBSAN
detecting overflows.

If you take my diff, feel free to add:
Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>

---8<-----------------------

diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
index 5e9e4779bdf1..3398cf2d19c5 100644
--- a/kernel/rcu/tree_stall.h
+++ b/kernel/rcu/tree_stall.h
@@ -162,14 +162,15 @@ static void panic_on_rcu_stall(void)
  */
 void rcu_cpu_stall_reset(void)
 {
-	u64 curr, last, delta;
+	ktime_t last, delta_ns;
+	u64 delta_jiff;
 
-	curr = ktime_get_mono_fast_ns();
 	last = ktime_get_seconds() * NSEC_PER_SEC;
-	delta = nsecs_to_jiffies(curr - last);
+	delta_ns = ktime_sub(ktime_get_mono_fast_ns(), last);
+	delta_jiff = nsecs_to_jiffies(delta_ns);
 
 	WRITE_ONCE(rcu_state.jiffies_stall,
-		   jiffies + delta + rcu_jiffies_till_stall_check());
+		   jiffies + delta_jiff + rcu_jiffies_till_stall_check());
 }
 
 //////////////////////////////////////////////////////////////////////////////