[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f36375fb-fdd2-4be6-3d74-5137533b6264@efficios.com>
Date: Wed, 14 Dec 2022 15:37:13 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: paulmck@...nel.org, Masami Hiramatsu <mhiramat@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
LKML <linux-kernel@...r.kernel.org>,
Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] tracing: Do not synchronize freeing of trigger filter on
boot up
On 2022-12-14 15:03, Paul E. McKenney wrote:
> On Wed, Dec 14, 2022 at 08:49:54AM +0900, Masami Hiramatsu wrote:
>> On Tue, 13 Dec 2022 17:24:29 -0500
>> Steven Rostedt <rostedt@...dmis.org> wrote:
>>
>>> From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
>>>
>>> If a trigger filter on the kernel command line fails to apply (due to
>>> syntax error), it will be freed. The freeing will call
>>> tracepoint_synchronize_unregister(), but this is not needed during early
>>> boot up, and will even trigger a lockdep splat.
>>>
>>> Avoid calling the synchronization function when system_state is
>>> SYSTEM_BOOTING.
>>
>> Shouldn't this be done inside tracepoint_synchronize_unregister()?
>> Then, it will prevent similar warnings if we expand boot time feature.
>
> How about the following wide-spectrum fix within RCU_LOCKDEP_WARN()?
> Just in case there are ever additional issues of this sort?
Hi Paul,
Your approach makes sense. Thanks for looking into this.
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit d493ffca2df6c1963bd1d7b8f8c652a172f095ae
> Author: Paul E. McKenney <paulmck@...nel.org>
> Date: Wed Dec 14 11:41:44 2022 -0800
>
> rcu: Make RCU_LOCKDEP_WARN() avoid early lockdep checks
>
> Currently, RCU_LOCKDEP_WARN() checks the condition before checking
> to see if lockdep is still enabled. This is necessary to avoid the
> false-positive splats fixed by commit 3066820034b5dd ("rcu: Reject
> RCU_LOCKDEP_WARN() false positives"). However, the current state can
> result in false-positive splats during early boot before lockdep is fully
> initialized. This commit therefore checks debug_lockdep_rcu_enabled()
> both before and after checking the condition, thus avoiding both sets
> of false-positive error reports.
>
> Reported-by: Steven Rostedt <rostedt@...dmis.org>
> Reported-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> Reported-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Matthew Wilcox <willy@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index aa1b1c3546d7a..1aec1d53b0c91 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -364,11 +364,18 @@ static inline int debug_lockdep_rcu_enabled(void)
> * RCU_LOCKDEP_WARN - emit lockdep splat if specified condition is met
> * @c: condition to check
> * @s: informative message
> + *
> + * This checks debug_lockdep_rcu_enabled() before checking (c) to
> + * prevent early boot splats due to lockdep not yet being initialized,
> + * and rechecks it after checking (c) to prevent false-positive splats
> + * due to races with lockdep being disabled. See commit 3066820034b5dd
> + * ("rcu: Reject RCU_LOCKDEP_WARN() false positives") for more detail.
> */
> #define RCU_LOCKDEP_WARN(c, s) \
> do { \
> static bool __section(".data.unlikely") __warned; \
> - if ((c) && debug_lockdep_rcu_enabled() && !__warned) { \
> + if (debug_lockdep_rcu_enabled() && (c) && \
> + debug_lockdep_rcu_enabled() && !__warned) { \
> __warned = true; \
> lockdep_rcu_suspicious(__FILE__, __LINE__, s); \
> } \
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Powered by blists - more mailing lists