include/linux/tracepoint.h | 50 ++----------------------------- include/trace/events/preemptirq.h | 8 ----- kernel/trace/trace_preemptirq.c | 26 ++++------------ scripts/tags.sh | 2 -- 4 files changed, 8 insertions(+), 78 deletions(-)
From: Steven Rostedt <rostedt@goodmis.org>
The trace_*_rcuidle() variant of a tracepoint was to handle places where a
tracepoint was located but RCU was not "watching". All those locations
have been removed, and RCU should be watching where all tracepoints are
located. We can now remove the trace_*_rcuidle() variant.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://lore.kernel.org/20241003173051.6b178bb3@gandalf.local.home
- Silly me didn't remove the rcuidle logic from __DO_TRACE()
include/linux/tracepoint.h | 50 ++-----------------------------
include/trace/events/preemptirq.h | 8 -----
kernel/trace/trace_preemptirq.c | 26 ++++------------
scripts/tags.sh | 2 --
4 files changed, 8 insertions(+), 78 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 93a9f3070b48..9b85a02b8575 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -196,67 +196,25 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DO_TRACE_CALL(name, args) __traceiter_##name(NULL, args)
#endif /* CONFIG_HAVE_STATIC_CALL */
-/*
- * ARCH_WANTS_NO_INSTR archs are expected to have sanitized entry and idle
- * code that disallow any/all tracing/instrumentation when RCU isn't watching.
- */
-#ifdef CONFIG_ARCH_WANTS_NO_INSTR
-#define RCUIDLE_COND(rcuidle) (rcuidle)
-#else
-/* srcu can't be used from NMI */
-#define RCUIDLE_COND(rcuidle) (rcuidle && in_nmi())
-#endif
-
/*
* it_func[0] is never NULL because there is at least one element in the array
* when the array itself is non NULL.
*/
-#define __DO_TRACE(name, args, cond, rcuidle) \
+#define __DO_TRACE(name, args, cond) \
do { \
int __maybe_unused __idx = 0; \
\
if (!(cond)) \
return; \
\
- if (WARN_ONCE(RCUIDLE_COND(rcuidle), \
- "Bad RCU usage for tracepoint")) \
- return; \
- \
/* keep srcu and sched-rcu usage consistent */ \
preempt_disable_notrace(); \
\
- /* \
- * For rcuidle callers, use srcu since sched-rcu \
- * doesn't work from the idle path. \
- */ \
- if (rcuidle) { \
- __idx = srcu_read_lock_notrace(&tracepoint_srcu);\
- ct_irq_enter_irqson(); \
- } \
- \
__DO_TRACE_CALL(name, TP_ARGS(args)); \
\
- if (rcuidle) { \
- ct_irq_exit_irqson(); \
- srcu_read_unlock_notrace(&tracepoint_srcu, __idx);\
- } \
- \
preempt_enable_notrace(); \
} while (0)
-#ifndef MODULE
-#define __DECLARE_TRACE_RCU(name, proto, args, cond) \
- static inline void trace_##name##_rcuidle(proto) \
- { \
- if (static_key_false(&__tracepoint_##name.key)) \
- __DO_TRACE(name, \
- TP_ARGS(args), \
- TP_CONDITION(cond), 1); \
- }
-#else
-#define __DECLARE_TRACE_RCU(name, proto, args, cond)
-#endif
-
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -277,14 +235,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
if (static_key_false(&__tracepoint_##name.key)) \
__DO_TRACE(name, \
TP_ARGS(args), \
- TP_CONDITION(cond), 0); \
+ TP_CONDITION(cond)); \
if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
WARN_ONCE(!rcu_is_watching(), \
"RCU not watching for tracepoint"); \
} \
} \
- __DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args), \
- PARAMS(cond)) \
static inline int \
register_trace_##name(void (*probe)(data_proto), void *data) \
{ \
@@ -375,8 +331,6 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE(name, proto, args, cond, data_proto) \
static inline void trace_##name(proto) \
{ } \
- static inline void trace_##name##_rcuidle(proto) \
- { } \
static inline int \
register_trace_##name(void (*probe)(data_proto), \
void *data) \
diff --git a/include/trace/events/preemptirq.h b/include/trace/events/preemptirq.h
index 3f249e150c0c..f99562d2b496 100644
--- a/include/trace/events/preemptirq.h
+++ b/include/trace/events/preemptirq.h
@@ -43,8 +43,6 @@ DEFINE_EVENT(preemptirq_template, irq_enable,
#else
#define trace_irq_enable(...)
#define trace_irq_disable(...)
-#define trace_irq_enable_rcuidle(...)
-#define trace_irq_disable_rcuidle(...)
#endif
#ifdef CONFIG_TRACE_PREEMPT_TOGGLE
@@ -58,8 +56,6 @@ DEFINE_EVENT(preemptirq_template, preempt_enable,
#else
#define trace_preempt_enable(...)
#define trace_preempt_disable(...)
-#define trace_preempt_enable_rcuidle(...)
-#define trace_preempt_disable_rcuidle(...)
#endif
#endif /* _TRACE_PREEMPTIRQ_H */
@@ -69,10 +65,6 @@ DEFINE_EVENT(preemptirq_template, preempt_enable,
#else /* !CONFIG_PREEMPTIRQ_TRACEPOINTS */
#define trace_irq_enable(...)
#define trace_irq_disable(...)
-#define trace_irq_enable_rcuidle(...)
-#define trace_irq_disable_rcuidle(...)
#define trace_preempt_enable(...)
#define trace_preempt_disable(...)
-#define trace_preempt_enable_rcuidle(...)
-#define trace_preempt_disable_rcuidle(...)
#endif
diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
index e37446f7916e..5c03633316a6 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -15,20 +15,6 @@
#define CREATE_TRACE_POINTS
#include <trace/events/preemptirq.h>
-/*
- * Use regular trace points on architectures that implement noinstr
- * tooling: these calls will only happen with RCU enabled, which can
- * use a regular tracepoint.
- *
- * On older architectures, use the rcuidle tracing methods (which
- * aren't NMI-safe - so exclude NMI contexts):
- */
-#ifdef CONFIG_ARCH_WANTS_NO_INSTR
-#define trace(point) trace_##point
-#else
-#define trace(point) if (!in_nmi()) trace_##point##_rcuidle
-#endif
-
#ifdef CONFIG_TRACE_IRQFLAGS
/* Per-cpu variable to prevent redundant calls when IRQs already off */
static DEFINE_PER_CPU(int, tracing_irq_cpu);
@@ -42,7 +28,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
void trace_hardirqs_on_prepare(void)
{
if (this_cpu_read(tracing_irq_cpu)) {
- trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1);
+ trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
this_cpu_write(tracing_irq_cpu, 0);
}
@@ -53,7 +39,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
void trace_hardirqs_on(void)
{
if (this_cpu_read(tracing_irq_cpu)) {
- trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1);
+ trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
this_cpu_write(tracing_irq_cpu, 0);
}
@@ -75,7 +61,7 @@ void trace_hardirqs_off_finish(void)
if (!this_cpu_read(tracing_irq_cpu)) {
this_cpu_write(tracing_irq_cpu, 1);
tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
- trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1);
+ trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
}
}
@@ -89,7 +75,7 @@ void trace_hardirqs_off(void)
if (!this_cpu_read(tracing_irq_cpu)) {
this_cpu_write(tracing_irq_cpu, 1);
tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
- trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1);
+ trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
}
}
EXPORT_SYMBOL(trace_hardirqs_off);
@@ -100,13 +86,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
void trace_preempt_on(unsigned long a0, unsigned long a1)
{
- trace(preempt_enable)(a0, a1);
+ trace_preempt_enable(a0, a1);
tracer_preempt_on(a0, a1);
}
void trace_preempt_off(unsigned long a0, unsigned long a1)
{
- trace(preempt_disable)(a0, a1);
+ trace_preempt_disable(a0, a1);
tracer_preempt_off(a0, a1);
}
#endif
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 191e0461d6d5..0d01c1cafb70 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -152,9 +152,7 @@ regex_c=(
'/^BPF_CALL_[0-9]([[:space:]]*\([[:alnum:]_]*\).*/\1/'
'/^COMPAT_SYSCALL_DEFINE[0-9]([[:space:]]*\([[:alnum:]_]*\).*/compat_sys_\1/'
'/^TRACE_EVENT([[:space:]]*\([[:alnum:]_]*\).*/trace_\1/'
- '/^TRACE_EVENT([[:space:]]*\([[:alnum:]_]*\).*/trace_\1_rcuidle/'
'/^DEFINE_EVENT([^,)]*,[[:space:]]*\([[:alnum:]_]*\).*/trace_\1/'
- '/^DEFINE_EVENT([^,)]*,[[:space:]]*\([[:alnum:]_]*\).*/trace_\1_rcuidle/'
'/^DEFINE_INSN_CACHE_OPS([[:space:]]*\([[:alnum:]_]*\).*/get_\1_slot/'
'/^DEFINE_INSN_CACHE_OPS([[:space:]]*\([[:alnum:]_]*\).*/free_\1_slot/'
'/^PAGEFLAG([[:space:]]*\([[:alnum:]_]*\).*/Page\1/'
--
2.45.2On Thu, Oct 3, 2024 at 6:15 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The trace_*_rcuidle() variant of a tracepoint was to handle places where a
> tracepoint was located but RCU was not "watching". All those locations
> have been removed, and RCU should be watching where all tracepoints are
> located. We can now remove the trace_*_rcuidle() variant.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
thanks,
- Joel
> ---
> Changes since v1: https://lore.kernel.org/20241003173051.6b178bb3@gandalf.local.home
>
> - Silly me didn't remove the rcuidle logic from __DO_TRACE()
>
> include/linux/tracepoint.h | 50 ++-----------------------------
> include/trace/events/preemptirq.h | 8 -----
> kernel/trace/trace_preemptirq.c | 26 ++++------------
> scripts/tags.sh | 2 --
> 4 files changed, 8 insertions(+), 78 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 93a9f3070b48..9b85a02b8575 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -196,67 +196,25 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> #define __DO_TRACE_CALL(name, args) __traceiter_##name(NULL, args)
> #endif /* CONFIG_HAVE_STATIC_CALL */
>
> -/*
> - * ARCH_WANTS_NO_INSTR archs are expected to have sanitized entry and idle
> - * code that disallow any/all tracing/instrumentation when RCU isn't watching.
> - */
> -#ifdef CONFIG_ARCH_WANTS_NO_INSTR
> -#define RCUIDLE_COND(rcuidle) (rcuidle)
> -#else
> -/* srcu can't be used from NMI */
> -#define RCUIDLE_COND(rcuidle) (rcuidle && in_nmi())
> -#endif
> -
> /*
> * it_func[0] is never NULL because there is at least one element in the array
> * when the array itself is non NULL.
> */
> -#define __DO_TRACE(name, args, cond, rcuidle) \
> +#define __DO_TRACE(name, args, cond) \
> do { \
> int __maybe_unused __idx = 0; \
> \
> if (!(cond)) \
> return; \
> \
> - if (WARN_ONCE(RCUIDLE_COND(rcuidle), \
> - "Bad RCU usage for tracepoint")) \
> - return; \
> - \
> /* keep srcu and sched-rcu usage consistent */ \
> preempt_disable_notrace(); \
> \
> - /* \
> - * For rcuidle callers, use srcu since sched-rcu \
> - * doesn't work from the idle path. \
> - */ \
> - if (rcuidle) { \
> - __idx = srcu_read_lock_notrace(&tracepoint_srcu);\
> - ct_irq_enter_irqson(); \
> - } \
> - \
> __DO_TRACE_CALL(name, TP_ARGS(args)); \
> \
> - if (rcuidle) { \
> - ct_irq_exit_irqson(); \
> - srcu_read_unlock_notrace(&tracepoint_srcu, __idx);\
> - } \
> - \
> preempt_enable_notrace(); \
> } while (0)
>
> -#ifndef MODULE
> -#define __DECLARE_TRACE_RCU(name, proto, args, cond) \
> - static inline void trace_##name##_rcuidle(proto) \
> - { \
> - if (static_key_false(&__tracepoint_##name.key)) \
> - __DO_TRACE(name, \
> - TP_ARGS(args), \
> - TP_CONDITION(cond), 1); \
> - }
> -#else
> -#define __DECLARE_TRACE_RCU(name, proto, args, cond)
> -#endif
> -
> /*
> * Make sure the alignment of the structure in the __tracepoints section will
> * not add unwanted padding between the beginning of the section and the
> @@ -277,14 +235,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> if (static_key_false(&__tracepoint_##name.key)) \
> __DO_TRACE(name, \
> TP_ARGS(args), \
> - TP_CONDITION(cond), 0); \
> + TP_CONDITION(cond)); \
> if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
> WARN_ONCE(!rcu_is_watching(), \
> "RCU not watching for tracepoint"); \
> } \
> } \
> - __DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args), \
> - PARAMS(cond)) \
> static inline int \
> register_trace_##name(void (*probe)(data_proto), void *data) \
> { \
> @@ -375,8 +331,6 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> #define __DECLARE_TRACE(name, proto, args, cond, data_proto) \
> static inline void trace_##name(proto) \
> { } \
> - static inline void trace_##name##_rcuidle(proto) \
> - { } \
> static inline int \
> register_trace_##name(void (*probe)(data_proto), \
> void *data) \
> diff --git a/include/trace/events/preemptirq.h b/include/trace/events/preemptirq.h
> index 3f249e150c0c..f99562d2b496 100644
> --- a/include/trace/events/preemptirq.h
> +++ b/include/trace/events/preemptirq.h
> @@ -43,8 +43,6 @@ DEFINE_EVENT(preemptirq_template, irq_enable,
> #else
> #define trace_irq_enable(...)
> #define trace_irq_disable(...)
> -#define trace_irq_enable_rcuidle(...)
> -#define trace_irq_disable_rcuidle(...)
> #endif
>
> #ifdef CONFIG_TRACE_PREEMPT_TOGGLE
> @@ -58,8 +56,6 @@ DEFINE_EVENT(preemptirq_template, preempt_enable,
> #else
> #define trace_preempt_enable(...)
> #define trace_preempt_disable(...)
> -#define trace_preempt_enable_rcuidle(...)
> -#define trace_preempt_disable_rcuidle(...)
> #endif
>
> #endif /* _TRACE_PREEMPTIRQ_H */
> @@ -69,10 +65,6 @@ DEFINE_EVENT(preemptirq_template, preempt_enable,
> #else /* !CONFIG_PREEMPTIRQ_TRACEPOINTS */
> #define trace_irq_enable(...)
> #define trace_irq_disable(...)
> -#define trace_irq_enable_rcuidle(...)
> -#define trace_irq_disable_rcuidle(...)
> #define trace_preempt_enable(...)
> #define trace_preempt_disable(...)
> -#define trace_preempt_enable_rcuidle(...)
> -#define trace_preempt_disable_rcuidle(...)
> #endif
> diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
> index e37446f7916e..5c03633316a6 100644
> --- a/kernel/trace/trace_preemptirq.c
> +++ b/kernel/trace/trace_preemptirq.c
> @@ -15,20 +15,6 @@
> #define CREATE_TRACE_POINTS
> #include <trace/events/preemptirq.h>
>
> -/*
> - * Use regular trace points on architectures that implement noinstr
> - * tooling: these calls will only happen with RCU enabled, which can
> - * use a regular tracepoint.
> - *
> - * On older architectures, use the rcuidle tracing methods (which
> - * aren't NMI-safe - so exclude NMI contexts):
> - */
> -#ifdef CONFIG_ARCH_WANTS_NO_INSTR
> -#define trace(point) trace_##point
> -#else
> -#define trace(point) if (!in_nmi()) trace_##point##_rcuidle
> -#endif
> -
> #ifdef CONFIG_TRACE_IRQFLAGS
> /* Per-cpu variable to prevent redundant calls when IRQs already off */
> static DEFINE_PER_CPU(int, tracing_irq_cpu);
> @@ -42,7 +28,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
> void trace_hardirqs_on_prepare(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1);
> + trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -53,7 +39,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
> void trace_hardirqs_on(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1);
> + trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -75,7 +61,7 @@ void trace_hardirqs_off_finish(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1);
> + trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> }
>
> }
> @@ -89,7 +75,7 @@ void trace_hardirqs_off(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1);
> + trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> }
> }
> EXPORT_SYMBOL(trace_hardirqs_off);
> @@ -100,13 +86,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
>
> void trace_preempt_on(unsigned long a0, unsigned long a1)
> {
> - trace(preempt_enable)(a0, a1);
> + trace_preempt_enable(a0, a1);
> tracer_preempt_on(a0, a1);
> }
>
> void trace_preempt_off(unsigned long a0, unsigned long a1)
> {
> - trace(preempt_disable)(a0, a1);
> + trace_preempt_disable(a0, a1);
> tracer_preempt_off(a0, a1);
> }
> #endif
> diff --git a/scripts/tags.sh b/scripts/tags.sh
> index 191e0461d6d5..0d01c1cafb70 100755
> --- a/scripts/tags.sh
> +++ b/scripts/tags.sh
> @@ -152,9 +152,7 @@ regex_c=(
> '/^BPF_CALL_[0-9]([[:space:]]*\([[:alnum:]_]*\).*/\1/'
> '/^COMPAT_SYSCALL_DEFINE[0-9]([[:space:]]*\([[:alnum:]_]*\).*/compat_sys_\1/'
> '/^TRACE_EVENT([[:space:]]*\([[:alnum:]_]*\).*/trace_\1/'
> - '/^TRACE_EVENT([[:space:]]*\([[:alnum:]_]*\).*/trace_\1_rcuidle/'
> '/^DEFINE_EVENT([^,)]*,[[:space:]]*\([[:alnum:]_]*\).*/trace_\1/'
> - '/^DEFINE_EVENT([^,)]*,[[:space:]]*\([[:alnum:]_]*\).*/trace_\1_rcuidle/'
> '/^DEFINE_INSN_CACHE_OPS([[:space:]]*\([[:alnum:]_]*\).*/get_\1_slot/'
> '/^DEFINE_INSN_CACHE_OPS([[:space:]]*\([[:alnum:]_]*\).*/free_\1_slot/'
> '/^PAGEFLAG([[:space:]]*\([[:alnum:]_]*\).*/Page\1/'
> --
> 2.45.2
>
Hi Steven, On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The trace_*_rcuidle() variant of a tracepoint was to handle places where a > tracepoint was located but RCU was not "watching". All those locations > have been removed, and RCU should be watching where all tracepoints are > located. We can now remove the trace_*_rcuidle() variant. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- This patch triggers backtraces with mips qemu emulations. WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 handle_int+0x128/0x178 RCU not watching for tracepoint ... Call Trace: [<ffffffff8012a89c>] show_stack+0x64/0x158 [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 [<ffffffff801550f4>] __warn+0xa4/0x1f8 [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 [<ffffffff801233e8>] handle_int+0x128/0x178 [<ffffffff80f25ff8>] r4k_wait+0x30/0x40 [<ffffffff80f26448>] default_idle_call+0xa0/0x330 [<ffffffff801b6ccc>] do_idle+0xe4/0x1b8 [<ffffffff801b71c4>] cpu_startup_entry+0x34/0x48 [<ffffffff80f2689c>] kernel_init+0x0/0x110 WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x184/0x240 RCU not watching for tracepoint ... Call Trace: [<ffffffff8012a89c>] show_stack+0x64/0x158 [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128 [<ffffffff801550f4>] __warn+0xa4/0x1f8 [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0 [<ffffffff8028fb74>] trace_hardirqs_on+0x184/0x240 [<ffffffff80123030>] restore_partial+0x6c/0x13c Bisect log is attached. Guenter --- # bad: [f6420e2ee9caa3ecf226d0db22572db0d9451977] Merge branch 'fixes-v6.13' into testing # good: [adc218676eef25575469234709c2d87185ca223a] Linux 6.12 git bisect start 'f6420e2ee9ca' 'v6.12' # good: [6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7] Merge tag 'bpf-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next git bisect good 6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7 # good: [071b34dcf71523a559b6c39f5d21a268a9531b50] Merge tag 'sound-6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound git bisect good 071b34dcf71523a559b6c39f5d21a268a9531b50 # good: [8a07b2623e7ff04856f8d4470d002675049b2065] Merge tag 'drm-misc-next-2024-10-31' of https://gitlab.freedesktop.org/drm/misc/kernel into drm-next git bisect good 8a07b2623e7ff04856f8d4470d002675049b2065 # bad: [80739fd00c7ea1315d362ce889bef499452913ef] Merge tag 'mfd-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd git bisect bad 80739fd00c7ea1315d362ce889bef499452913ef # good: [e175800137f588688ac6aae30ce491e098f30c45] Merge tag 'exynos-drm-next-for-v6.13-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next git bisect good e175800137f588688ac6aae30ce491e098f30c45 # good: [ade5add00da20de40f63d097345bddea24d924f4] Merge tag 'amd-drm-next-6.13-2024-11-15' of https://gitlab.freedesktop.org/agd5f/linux into drm-next git bisect good ade5add00da20de40f63d097345bddea24d924f4 # bad: [06afb0f36106ecb839c5e2509905e68c1e2677de] Merge tag 'trace-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace git bisect bad 06afb0f36106ecb839c5e2509905e68c1e2677de # good: [f1db825805d48cee6826b7dc082a04112c1f0c8d] Merge tag 'trace-ring-buffer-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace git bisect good f1db825805d48cee6826b7dc082a04112c1f0c8d # bad: [61c6fefa92bb4ed7a34163b94f6ffac628237a29] bpf: decouple BPF link/attach hook and BPF program sleepable semantics git bisect bad 61c6fefa92bb4ed7a34163b94f6ffac628237a29 # bad: [cdb537ac417938408ee819992f432c410f2d01a2] tracing/perf: Add might_fault check to syscall probes git bisect bad cdb537ac417938408ee819992f432c410f2d01a2 # bad: [e53244e2c8931f9e80c1841293aea86ef8ad32a3] tracepoint: Remove SRCU protection git bisect bad e53244e2c8931f9e80c1841293aea86ef8ad32a3 # good: [49e4154f4b16345da5e219b23ed9737a6e735bc1] tracing: Remove TRACE_EVENT_FL_FILTERED logic git bisect good 49e4154f4b16345da5e219b23ed9737a6e735bc1 # bad: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle() git bisect bad 48bcda6848232667f13b4e97588de488c83c37d4 # good: [4a8840af5f53f2902eba91130fae650879f18e7a] tracepoints: Use new static branch API git bisect good 4a8840af5f53f2902eba91130fae650879f18e7a # first bad commit: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle()
[ Adding MIPS maintainers ]
On Tue, 3 Dec 2024 12:39:08 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> Hi Steven,
>
> On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote:
> > From: Steven Rostedt <rostedt@goodmis.org>
> >
> > The trace_*_rcuidle() variant of a tracepoint was to handle places where a
> > tracepoint was located but RCU was not "watching". All those locations
> > have been removed, and RCU should be watching where all tracepoints are
> > located. We can now remove the trace_*_rcuidle() variant.
> >
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > ---
>
> This patch triggers backtraces with mips qemu emulations.
>
> WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 handle_int+0x128/0x178
> RCU not watching for tracepoint
> ...
> Call Trace:
> [<ffffffff8012a89c>] show_stack+0x64/0x158
> [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128
> [<ffffffff801550f4>] __warn+0xa4/0x1f8
> [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0
> [<ffffffff801233e8>] handle_int+0x128/0x178
> [<ffffffff80f25ff8>] r4k_wait+0x30/0x40
> [<ffffffff80f26448>] default_idle_call+0xa0/0x330
> [<ffffffff801b6ccc>] do_idle+0xe4/0x1b8
> [<ffffffff801b71c4>] cpu_startup_entry+0x34/0x48
> [<ffffffff80f2689c>] kernel_init+0x0/0x110
>
> WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x184/0x240
> RCU not watching for tracepoint
> ...
> Call Trace:
> [<ffffffff8012a89c>] show_stack+0x64/0x158
> [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128
> [<ffffffff801550f4>] __warn+0xa4/0x1f8
> [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0
> [<ffffffff8028fb74>] trace_hardirqs_on+0x184/0x240
> [<ffffffff80123030>] restore_partial+0x6c/0x13c
>
> Bisect log is attached.
I'm guessing MIPS needs to implement something like what arm64 did with:
7cd1ea1010acb ("arm64: entry: fix non-NMI kernel<->kernel transitions")
Because we do not want to bring back the trace_*_rcuidle() crap.
-- Steve
>
> Guenter
>
> ---
> # bad: [f6420e2ee9caa3ecf226d0db22572db0d9451977] Merge branch 'fixes-v6.13' into testing
> # good: [adc218676eef25575469234709c2d87185ca223a] Linux 6.12
> git bisect start 'f6420e2ee9ca' 'v6.12'
> # good: [6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7] Merge tag 'bpf-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
> git bisect good 6e95ef0258ff4ee23ae3b06bf6b00b33dbbd5ef7
> # good: [071b34dcf71523a559b6c39f5d21a268a9531b50] Merge tag 'sound-6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
> git bisect good 071b34dcf71523a559b6c39f5d21a268a9531b50
> # good: [8a07b2623e7ff04856f8d4470d002675049b2065] Merge tag 'drm-misc-next-2024-10-31' of https://gitlab.freedesktop.org/drm/misc/kernel into drm-next
> git bisect good 8a07b2623e7ff04856f8d4470d002675049b2065
> # bad: [80739fd00c7ea1315d362ce889bef499452913ef] Merge tag 'mfd-next-6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd
> git bisect bad 80739fd00c7ea1315d362ce889bef499452913ef
> # good: [e175800137f588688ac6aae30ce491e098f30c45] Merge tag 'exynos-drm-next-for-v6.13-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next
> git bisect good e175800137f588688ac6aae30ce491e098f30c45
> # good: [ade5add00da20de40f63d097345bddea24d924f4] Merge tag 'amd-drm-next-6.13-2024-11-15' of https://gitlab.freedesktop.org/agd5f/linux into drm-next
> git bisect good ade5add00da20de40f63d097345bddea24d924f4
> # bad: [06afb0f36106ecb839c5e2509905e68c1e2677de] Merge tag 'trace-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
> git bisect bad 06afb0f36106ecb839c5e2509905e68c1e2677de
> # good: [f1db825805d48cee6826b7dc082a04112c1f0c8d] Merge tag 'trace-ring-buffer-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
> git bisect good f1db825805d48cee6826b7dc082a04112c1f0c8d
> # bad: [61c6fefa92bb4ed7a34163b94f6ffac628237a29] bpf: decouple BPF link/attach hook and BPF program sleepable semantics
> git bisect bad 61c6fefa92bb4ed7a34163b94f6ffac628237a29
> # bad: [cdb537ac417938408ee819992f432c410f2d01a2] tracing/perf: Add might_fault check to syscall probes
> git bisect bad cdb537ac417938408ee819992f432c410f2d01a2
> # bad: [e53244e2c8931f9e80c1841293aea86ef8ad32a3] tracepoint: Remove SRCU protection
> git bisect bad e53244e2c8931f9e80c1841293aea86ef8ad32a3
> # good: [49e4154f4b16345da5e219b23ed9737a6e735bc1] tracing: Remove TRACE_EVENT_FL_FILTERED logic
> git bisect good 49e4154f4b16345da5e219b23ed9737a6e735bc1
> # bad: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle()
> git bisect bad 48bcda6848232667f13b4e97588de488c83c37d4
> # good: [4a8840af5f53f2902eba91130fae650879f18e7a] tracepoints: Use new static branch API
> git bisect good 4a8840af5f53f2902eba91130fae650879f18e7a
> # first bad commit: [48bcda6848232667f13b4e97588de488c83c37d4] tracing: Remove definition of trace_*_rcuidle()
On 12/3/24 12:55, Steven Rostedt wrote:
>
> [ Adding MIPS maintainers ]
>
> On Tue, 3 Dec 2024 12:39:08 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
>> Hi Steven,
>>
>> On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote:
>>> From: Steven Rostedt <rostedt@goodmis.org>
>>>
>>> The trace_*_rcuidle() variant of a tracepoint was to handle places where a
>>> tracepoint was located but RCU was not "watching". All those locations
>>> have been removed, and RCU should be watching where all tracepoints are
>>> located. We can now remove the trace_*_rcuidle() variant.
>>>
>>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>>> ---
>>
>> This patch triggers backtraces with mips qemu emulations.
>>
>> WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36 handle_int+0x128/0x178
>> RCU not watching for tracepoint
>> ...
>> Call Trace:
>> [<ffffffff8012a89c>] show_stack+0x64/0x158
>> [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128
>> [<ffffffff801550f4>] __warn+0xa4/0x1f8
>> [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0
>> [<ffffffff801233e8>] handle_int+0x128/0x178
>> [<ffffffff80f25ff8>] r4k_wait+0x30/0x40
>> [<ffffffff80f26448>] default_idle_call+0xa0/0x330
>> [<ffffffff801b6ccc>] do_idle+0xe4/0x1b8
>> [<ffffffff801b71c4>] cpu_startup_entry+0x34/0x48
>> [<ffffffff80f2689c>] kernel_init+0x0/0x110
>>
>> WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:40 trace_hardirqs_on+0x184/0x240
>> RCU not watching for tracepoint
>> ...
>> Call Trace:
>> [<ffffffff8012a89c>] show_stack+0x64/0x158
>> [<ffffffff8011e998>] dump_stack_lvl+0xb4/0x128
>> [<ffffffff801550f4>] __warn+0xa4/0x1f8
>> [<ffffffff80155328>] warn_slowpath_fmt+0xe0/0x1d0
>> [<ffffffff8028fb74>] trace_hardirqs_on+0x184/0x240
>> [<ffffffff80123030>] restore_partial+0x6c/0x13c
>>
>> Bisect log is attached.
>
> I'm guessing MIPS needs to implement something like what arm64 did with:
>
> 7cd1ea1010acb ("arm64: entry: fix non-NMI kernel<->kernel transitions")
>
> Because we do not want to bring back the trace_*_rcuidle() crap.
>
Hmm. If you say so. Note that powerpc has the same or a similar problem.
[ 0.142039][ T0] RCU not watching for tracepoint
[ 0.142488][ T0]
[ 0.142659][ T0] =============================
[ 0.142755][ T0] WARNING: suspicious RCU usage
[ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
[ 0.143082][ T0] -----------------------------
[ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent!
[ 0.152733][ T0] RCU not watching for tracepoint
[ 0.152770][ T0]
[ 0.152995][ T0] =============================
[ 0.153092][ T0] WARNING: suspicious RCU usage
[ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
[ 0.153301][ T0] -----------------------------
[ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle!
[ 0.165396][ T0] RCU not watching for tracepoint
[ 0.165540][ T0]
[ 0.165712][ T0] =============================
[ 0.165811][ T0] WARNING: suspicious RCU usage
[ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
[ 0.166026][ T0] -----------------------------
[ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle!
and many more.
Guenter
On Tue, 3 Dec 2024 17:48:33 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> Hmm. If you say so. Note that powerpc has the same or a similar problem.
>
> [ 0.142039][ T0] RCU not watching for tracepoint
> [ 0.142488][ T0]
> [ 0.142659][ T0] =============================
> [ 0.142755][ T0] WARNING: suspicious RCU usage
> [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> [ 0.143082][ T0] -----------------------------
> [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent!
>
>
> [ 0.152733][ T0] RCU not watching for tracepoint
> [ 0.152770][ T0]
> [ 0.152995][ T0] =============================
> [ 0.153092][ T0] WARNING: suspicious RCU usage
> [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> [ 0.153301][ T0] -----------------------------
> [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle!
>
> [ 0.165396][ T0] RCU not watching for tracepoint
> [ 0.165540][ T0]
> [ 0.165712][ T0] =============================
> [ 0.165811][ T0] WARNING: suspicious RCU usage
> [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> [ 0.166026][ T0] -----------------------------
> [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle!
>
> and many more.
Grumble. It's just that one file. I wonder if we could just do a hack like
this?
Paul?
diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
index 5c03633316a6..58098873efa9 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -10,11 +10,42 @@
#include <linux/module.h>
#include <linux/ftrace.h>
#include <linux/kprobes.h>
+#include <linux/hardirq.h>
#include "trace.h"
#define CREATE_TRACE_POINTS
#include <trace/events/preemptirq.h>
+/*
+ * Use regular trace points on architectures that implement noinstr
+ * tooling: these calls will only happen with RCU enabled, which can
+ * use a regular tracepoint.
+ *
+ * On older architectures, RCU may not be watching in idle. In that
+ * case, wake up RCU to watch while calling the tracepoint. These
+ * aren't NMI-safe - so exclude NMI contexts:
+ */
+#ifdef CONFIG_ARCH_WANTS_NO_INSTR
+#define trace(point, args) trace_##point(args)
+#else
+#define trace(point, args) \
+ do { \
+ if (trace_##point##_enabled()) { \
+ bool exit_rcu = false; \
+ if (in_nmi()) \
+ break; \
+ if (!IS_ENABLED(CONFIG_TINY_RCU) && \
+ is_idle_task(current)) { \
+ ct_irq_enter(); \
+ exit_rcu = true; \
+ } \
+ trace_##point(args); \
+ if (exit_rcu) \
+ ct_irq_exit(); \
+ } \
+ } while (0)
+#endif
+
#ifdef CONFIG_TRACE_IRQFLAGS
/* Per-cpu variable to prevent redundant calls when IRQs already off */
static DEFINE_PER_CPU(int, tracing_irq_cpu);
@@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
void trace_hardirqs_on_prepare(void)
{
if (this_cpu_read(tracing_irq_cpu)) {
- trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
+ trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
this_cpu_write(tracing_irq_cpu, 0);
}
@@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
void trace_hardirqs_on(void)
{
if (this_cpu_read(tracing_irq_cpu)) {
- trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
+ trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
this_cpu_write(tracing_irq_cpu, 0);
}
@@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void)
if (!this_cpu_read(tracing_irq_cpu)) {
this_cpu_write(tracing_irq_cpu, 1);
tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
- trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
+ trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
}
}
@@ -75,7 +106,7 @@ void trace_hardirqs_off(void)
if (!this_cpu_read(tracing_irq_cpu)) {
this_cpu_write(tracing_irq_cpu, 1);
tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
- trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
+ trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
}
}
EXPORT_SYMBOL(trace_hardirqs_off);
@@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
void trace_preempt_on(unsigned long a0, unsigned long a1)
{
- trace_preempt_enable(a0, a1);
+ trace(preempt_enable, TP_ARGS(a0, a1));
tracer_preempt_on(a0, a1);
}
void trace_preempt_off(unsigned long a0, unsigned long a1)
{
- trace_preempt_disable(a0, a1);
+ trace(preempt_disable, TP_ARGS(a0, a1));
tracer_preempt_off(a0, a1);
}
#endif
I tested this by forcing x86 to use this code, and it appeared to work.
-- Steve
On 12/4/24 8:31 AM, Steven Rostedt wrote:
> On Tue, 3 Dec 2024 17:48:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
>> Hmm. If you say so. Note that powerpc has the same or a similar problem.
>>
>> [ 0.142039][ T0] RCU not watching for tracepoint
>> [ 0.142488][ T0]
>> [ 0.142659][ T0] =============================
>> [ 0.142755][ T0] WARNING: suspicious RCU usage
>> [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
>> [ 0.143082][ T0] -----------------------------
>> [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent!
>>
>>
>> [ 0.152733][ T0] RCU not watching for tracepoint
>> [ 0.152770][ T0]
>> [ 0.152995][ T0] =============================
>> [ 0.153092][ T0] WARNING: suspicious RCU usage
>> [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
>> [ 0.153301][ T0] -----------------------------
>> [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle!
>>
>> [ 0.165396][ T0] RCU not watching for tracepoint
>> [ 0.165540][ T0]
>> [ 0.165712][ T0] =============================
>> [ 0.165811][ T0] WARNING: suspicious RCU usage
>> [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
>> [ 0.166026][ T0] -----------------------------
>> [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle!
>>
>> and many more.
>
> Grumble. It's just that one file. I wonder if we could just do a hack like
> this?
>
Below patch fixes the issue in powerpc
Tested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Thanks
maddy
> Paul?
>
> diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
> index 5c03633316a6..58098873efa9 100644
> --- a/kernel/trace/trace_preemptirq.c
> +++ b/kernel/trace/trace_preemptirq.c
> @@ -10,11 +10,42 @@
> #include <linux/module.h>
> #include <linux/ftrace.h>
> #include <linux/kprobes.h>
> +#include <linux/hardirq.h>
> #include "trace.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/preemptirq.h>
>
> +/*
> + * Use regular trace points on architectures that implement noinstr
> + * tooling: these calls will only happen with RCU enabled, which can
> + * use a regular tracepoint.
> + *
> + * On older architectures, RCU may not be watching in idle. In that
> + * case, wake up RCU to watch while calling the tracepoint. These
> + * aren't NMI-safe - so exclude NMI contexts:
> + */
> +#ifdef CONFIG_ARCH_WANTS_NO_INSTR
> +#define trace(point, args) trace_##point(args)
> +#else
> +#define trace(point, args) \
> + do { \
> + if (trace_##point##_enabled()) { \
> + bool exit_rcu = false; \
> + if (in_nmi()) \
> + break; \
> + if (!IS_ENABLED(CONFIG_TINY_RCU) && \
> + is_idle_task(current)) { \
> + ct_irq_enter(); \
> + exit_rcu = true; \
> + } \
> + trace_##point(args); \
> + if (exit_rcu) \
> + ct_irq_exit(); \
> + } \
> + } while (0)
> +#endif
> +
> #ifdef CONFIG_TRACE_IRQFLAGS
> /* Per-cpu variable to prevent redundant calls when IRQs already off */
> static DEFINE_PER_CPU(int, tracing_irq_cpu);
> @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
> void trace_hardirqs_on_prepare(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
> void trace_hardirqs_on(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
>
> }
> @@ -75,7 +106,7 @@ void trace_hardirqs_off(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
> }
> EXPORT_SYMBOL(trace_hardirqs_off);
> @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
>
> void trace_preempt_on(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_enable(a0, a1);
> + trace(preempt_enable, TP_ARGS(a0, a1));
> tracer_preempt_on(a0, a1);
> }
>
> void trace_preempt_off(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_disable(a0, a1);
> + trace(preempt_disable, TP_ARGS(a0, a1));
> tracer_preempt_off(a0, a1);
> }
> #endif
>
>
> I tested this by forcing x86 to use this code, and it appeared to work.
>
> -- Steve
>
>
Hi Steven,
On Wed, Dec 4, 2024 at 4:31 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 3 Dec 2024 17:48:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > Hmm. If you say so. Note that powerpc has the same or a similar problem.
> >
> > [ 0.142039][ T0] RCU not watching for tracepoint
> > [ 0.142488][ T0]
> > [ 0.142659][ T0] =============================
> > [ 0.142755][ T0] WARNING: suspicious RCU usage
> > [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> > [ 0.143082][ T0] -----------------------------
> > [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent!
> >
> >
> > [ 0.152733][ T0] RCU not watching for tracepoint
> > [ 0.152770][ T0]
> > [ 0.152995][ T0] =============================
> > [ 0.153092][ T0] WARNING: suspicious RCU usage
> > [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> > [ 0.153301][ T0] -----------------------------
> > [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle!
> >
> > [ 0.165396][ T0] RCU not watching for tracepoint
> > [ 0.165540][ T0]
> > [ 0.165712][ T0] =============================
> > [ 0.165811][ T0] WARNING: suspicious RCU usage
> > [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> > [ 0.166026][ T0] -----------------------------
> > [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle!
> >
> > and many more.
>
> Grumble. It's just that one file. I wonder if we could just do a hack like
> this?
Thanks, this fixes the issue during ARM s2ram, so
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- a/kernel/trace/trace_preemptirq.c
> +++ b/kernel/trace/trace_preemptirq.c
> @@ -10,11 +10,42 @@
> #include <linux/module.h>
> #include <linux/ftrace.h>
> #include <linux/kprobes.h>
> +#include <linux/hardirq.h>
> #include "trace.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/preemptirq.h>
>
> +/*
> + * Use regular trace points on architectures that implement noinstr
> + * tooling: these calls will only happen with RCU enabled, which can
> + * use a regular tracepoint.
> + *
> + * On older architectures, RCU may not be watching in idle. In that
> + * case, wake up RCU to watch while calling the tracepoint. These
> + * aren't NMI-safe - so exclude NMI contexts:
> + */
> +#ifdef CONFIG_ARCH_WANTS_NO_INSTR
> +#define trace(point, args) trace_##point(args)
> +#else
> +#define trace(point, args) \
> + do { \
> + if (trace_##point##_enabled()) { \
> + bool exit_rcu = false; \
> + if (in_nmi()) \
> + break; \
> + if (!IS_ENABLED(CONFIG_TINY_RCU) && \
> + is_idle_task(current)) { \
> + ct_irq_enter(); \
> + exit_rcu = true; \
> + } \
> + trace_##point(args); \
> + if (exit_rcu) \
> + ct_irq_exit(); \
> + } \
> + } while (0)
> +#endif
> +
> #ifdef CONFIG_TRACE_IRQFLAGS
> /* Per-cpu variable to prevent redundant calls when IRQs already off */
> static DEFINE_PER_CPU(int, tracing_irq_cpu);
> @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
> void trace_hardirqs_on_prepare(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
> void trace_hardirqs_on(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
>
> }
> @@ -75,7 +106,7 @@ void trace_hardirqs_off(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
> }
> EXPORT_SYMBOL(trace_hardirqs_off);
> @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
>
> void trace_preempt_on(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_enable(a0, a1);
> + trace(preempt_enable, TP_ARGS(a0, a1));
> tracer_preempt_on(a0, a1);
> }
>
> void trace_preempt_off(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_disable(a0, a1);
> + trace(preempt_disable, TP_ARGS(a0, a1));
> tracer_preempt_off(a0, a1);
> }
> #endif
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 12/3/24 19:01, Steven Rostedt wrote:
> On Tue, 3 Dec 2024 17:48:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
>> Hmm. If you say so. Note that powerpc has the same or a similar problem.
>>
>> [ 0.142039][ T0] RCU not watching for tracepoint
>> [ 0.142488][ T0]
>> [ 0.142659][ T0] =============================
>> [ 0.142755][ T0] WARNING: suspicious RCU usage
>> [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
>> [ 0.143082][ T0] -----------------------------
>> [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent!
>>
>>
>> [ 0.152733][ T0] RCU not watching for tracepoint
>> [ 0.152770][ T0]
>> [ 0.152995][ T0] =============================
>> [ 0.153092][ T0] WARNING: suspicious RCU usage
>> [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
>> [ 0.153301][ T0] -----------------------------
>> [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle!
>>
>> [ 0.165396][ T0] RCU not watching for tracepoint
>> [ 0.165540][ T0]
>> [ 0.165712][ T0] =============================
>> [ 0.165811][ T0] WARNING: suspicious RCU usage
>> [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
>> [ 0.166026][ T0] -----------------------------
>> [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle!
>>
>> and many more.
>
> Grumble. It's just that one file. I wonder if we could just do a hack like
> this?
>
The code below fixes the problem for both mips and powerpc.
Tested-by: Guenter Roeck <linux@roeck-us.net>
Thanks,
Guenter
> Paul?
>
> diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
> index 5c03633316a6..58098873efa9 100644
> --- a/kernel/trace/trace_preemptirq.c
> +++ b/kernel/trace/trace_preemptirq.c
> @@ -10,11 +10,42 @@
> #include <linux/module.h>
> #include <linux/ftrace.h>
> #include <linux/kprobes.h>
> +#include <linux/hardirq.h>
> #include "trace.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/preemptirq.h>
>
> +/*
> + * Use regular trace points on architectures that implement noinstr
> + * tooling: these calls will only happen with RCU enabled, which can
> + * use a regular tracepoint.
> + *
> + * On older architectures, RCU may not be watching in idle. In that
> + * case, wake up RCU to watch while calling the tracepoint. These
> + * aren't NMI-safe - so exclude NMI contexts:
> + */
> +#ifdef CONFIG_ARCH_WANTS_NO_INSTR
> +#define trace(point, args) trace_##point(args)
> +#else
> +#define trace(point, args) \
> + do { \
> + if (trace_##point##_enabled()) { \
> + bool exit_rcu = false; \
> + if (in_nmi()) \
> + break; \
> + if (!IS_ENABLED(CONFIG_TINY_RCU) && \
> + is_idle_task(current)) { \
> + ct_irq_enter(); \
> + exit_rcu = true; \
> + } \
> + trace_##point(args); \
> + if (exit_rcu) \
> + ct_irq_exit(); \
> + } \
> + } while (0)
> +#endif
> +
> #ifdef CONFIG_TRACE_IRQFLAGS
> /* Per-cpu variable to prevent redundant calls when IRQs already off */
> static DEFINE_PER_CPU(int, tracing_irq_cpu);
> @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
> void trace_hardirqs_on_prepare(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
> void trace_hardirqs_on(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
>
> }
> @@ -75,7 +106,7 @@ void trace_hardirqs_off(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
> }
> EXPORT_SYMBOL(trace_hardirqs_off);
> @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
>
> void trace_preempt_on(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_enable(a0, a1);
> + trace(preempt_enable, TP_ARGS(a0, a1));
> tracer_preempt_on(a0, a1);
> }
>
> void trace_preempt_off(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_disable(a0, a1);
> + trace(preempt_disable, TP_ARGS(a0, a1));
> tracer_preempt_off(a0, a1);
> }
> #endif
>
>
> I tested this by forcing x86 to use this code, and it appeared to work.
>
> -- Steve
On Tue, Dec 03, 2024 at 10:01:53PM -0500, Steven Rostedt wrote:
> On Tue, 3 Dec 2024 17:48:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > Hmm. If you say so. Note that powerpc has the same or a similar problem.
> >
> > [ 0.142039][ T0] RCU not watching for tracepoint
> > [ 0.142488][ T0]
> > [ 0.142659][ T0] =============================
> > [ 0.142755][ T0] WARNING: suspicious RCU usage
> > [ 0.142914][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> > [ 0.143082][ T0] -----------------------------
> > [ 0.143178][ T0] kernel/notifier.c:586 notify_die called but RCU thinks we're quiescent!
> >
> >
> > [ 0.152733][ T0] RCU not watching for tracepoint
> > [ 0.152770][ T0]
> > [ 0.152995][ T0] =============================
> > [ 0.153092][ T0] WARNING: suspicious RCU usage
> > [ 0.153187][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> > [ 0.153301][ T0] -----------------------------
> > [ 0.153394][ T0] include/linux/rcupdate.h:850 rcu_read_lock() used illegally while idle!
> >
> > [ 0.165396][ T0] RCU not watching for tracepoint
> > [ 0.165540][ T0]
> > [ 0.165712][ T0] =============================
> > [ 0.165811][ T0] WARNING: suspicious RCU usage
> > [ 0.165909][ T0] 6.13.0-rc1-00058-ge75ce84aa5d3 #1 Not tainted
> > [ 0.166026][ T0] -----------------------------
> > [ 0.166122][ T0] include/linux/rcupdate.h:878 rcu_read_unlock() used illegally while idle!
> >
> > and many more.
>
> Grumble. It's just that one file. I wonder if we could just do a hack like
> this?
>
> Paul?
Looks plausible to me, though I don't understand why the introduction
of trace() doesn't permit removal of the corresponding current code.
(Or did I miss a previous patch that did just that?)
Thanx, Paul
> diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
> index 5c03633316a6..58098873efa9 100644
> --- a/kernel/trace/trace_preemptirq.c
> +++ b/kernel/trace/trace_preemptirq.c
> @@ -10,11 +10,42 @@
> #include <linux/module.h>
> #include <linux/ftrace.h>
> #include <linux/kprobes.h>
> +#include <linux/hardirq.h>
> #include "trace.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/preemptirq.h>
>
> +/*
> + * Use regular trace points on architectures that implement noinstr
> + * tooling: these calls will only happen with RCU enabled, which can
> + * use a regular tracepoint.
> + *
> + * On older architectures, RCU may not be watching in idle. In that
> + * case, wake up RCU to watch while calling the tracepoint. These
> + * aren't NMI-safe - so exclude NMI contexts:
> + */
> +#ifdef CONFIG_ARCH_WANTS_NO_INSTR
> +#define trace(point, args) trace_##point(args)
> +#else
> +#define trace(point, args) \
> + do { \
> + if (trace_##point##_enabled()) { \
> + bool exit_rcu = false; \
> + if (in_nmi()) \
> + break; \
> + if (!IS_ENABLED(CONFIG_TINY_RCU) && \
> + is_idle_task(current)) { \
> + ct_irq_enter(); \
> + exit_rcu = true; \
> + } \
> + trace_##point(args); \
> + if (exit_rcu) \
> + ct_irq_exit(); \
> + } \
> + } while (0)
> +#endif
> +
> #ifdef CONFIG_TRACE_IRQFLAGS
> /* Per-cpu variable to prevent redundant calls when IRQs already off */
> static DEFINE_PER_CPU(int, tracing_irq_cpu);
> @@ -28,7 +59,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
> void trace_hardirqs_on_prepare(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -39,7 +70,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepare);
> void trace_hardirqs_on(void)
> {
> if (this_cpu_read(tracing_irq_cpu)) {
> - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_enable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1);
> this_cpu_write(tracing_irq_cpu, 0);
> }
> @@ -61,7 +92,7 @@ void trace_hardirqs_off_finish(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
>
> }
> @@ -75,7 +106,7 @@ void trace_hardirqs_off(void)
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1);
> - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1);
> + trace(irq_disable, TP_ARGS(CALLER_ADDR0, CALLER_ADDR1));
> }
> }
> EXPORT_SYMBOL(trace_hardirqs_off);
> @@ -86,13 +117,13 @@ NOKPROBE_SYMBOL(trace_hardirqs_off);
>
> void trace_preempt_on(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_enable(a0, a1);
> + trace(preempt_enable, TP_ARGS(a0, a1));
> tracer_preempt_on(a0, a1);
> }
>
> void trace_preempt_off(unsigned long a0, unsigned long a1)
> {
> - trace_preempt_disable(a0, a1);
> + trace(preempt_disable, TP_ARGS(a0, a1));
> tracer_preempt_off(a0, a1);
> }
> #endif
>
>
> I tested this by forcing x86 to use this code, and it appeared to work.
>
> -- Steve
On Tue, 3 Dec 2024 21:01:59 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote: > > Paul? > > Looks plausible to me, though I don't understand why the introduction > of trace() doesn't permit removal of the corresponding current code. > (Or did I miss a previous patch that did just that?) > I removed the trace_*_rcuidle() code, but this file still used it. I didn't realize that removing the trace_*_rcuidle() in this file would break other architectures. This patch is a work around to not need to re-introduce the trace_*_rcuidle() code. -- Steve
On Wed, Dec 04, 2024 at 09:33:56AM -0500, Steven Rostedt wrote: > On Tue, 3 Dec 2024 21:01:59 -0800 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > Paul? > > > > Looks plausible to me, though I don't understand why the introduction > > of trace() doesn't permit removal of the corresponding current code. > > (Or did I miss a previous patch that did just that?) > > > > I removed the trace_*_rcuidle() code, but this file still used it. I didn't > realize that removing the trace_*_rcuidle() in this file would break other > architectures. > > This patch is a work around to not need to re-introduce the > trace_*_rcuidle() code. Works for me! Acked-by: Paul E. McKenney <paulmck@kernel.org> Thanx, Paul
Hi Steven,
On Fri, Oct 4, 2024 at 12:15 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The trace_*_rcuidle() variant of a tracepoint was to handle places where a
> tracepoint was located but RCU was not "watching". All those locations
> have been removed, and RCU should be watching where all tracepoints are
> located. We can now remove the trace_*_rcuidle() variant.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> Changes since v1: https://lore.kernel.org/20241003173051.6b178bb3@gandalf.local.home
>
> - Silly me didn't remove the rcuidle logic from __DO_TRACE()
Thanks for your patch, which is now commit 48bcda6848232667
("tracing: Remove definition of trace_*_rcuidle()") in v6.13-rc1.
This is causing warnings during s2ram on various ARM platforms.
E.g. on Koelsch:
Freezing user space processes
Freezing user space processes completed (elapsed 0.001 seconds)
OOM killer disabled.
Freezing remaining freezable tasks
-Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
-PM: suspend devices took 0.110 seconds
+Freezing remaining freezable tasks completed (elapsed 0.003 seconds)
+------------[ cut here ]------------
+------------[ cut here ]------------
+WARNING: CPU: 0 PID: 0 at include/trace/events/preemptirq.h:36
__irq_svc+0x48/0xac
+WARNING: CPU: 1 PID: 0 at include/trace/events/preemptirq.h:40
trace_hardirqs_on+0x40/0x7c
+RCU not watching for tracepoint
+RCU not watching for tracepoint
+Modules linked in:
+Modules linked in:
+CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G W N
6.12.0-rc2-koelsch-00005-g48bcda684823 #2094
+
+Tainted: [W]=WARN, [N]=TEST
+Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
+Call trace:
+ unwind_backtrace from show_stack+0x10/0x14
+ show_stack from dump_stack_lvl+0x78/0xa8
+ dump_stack_lvl from __warn+0x80/0x198
+ __warn from warn_slowpath_fmt+0xbc/0x120
+ warn_slowpath_fmt from __irq_svc+0x48/0xac
+Exception stack(0xc1201f20 to 0xc1201f68)
+1f20: c026570c ebf8fe7c 00000000 00000000 c120c0c0 c026570c c120905c c1209020
+1f40: c120c0c0 c106d640 c120905c c10438f8 fffffff8 c1201f70 c0a1c4fc c0a1c510
+1f60: 60000013 ffffffff
+ __irq_svc from cpu_idle_poll+0xd0/0x130
+ cpu_idle_poll from do_idle+0xc0/0x2a0
+ do_idle from cpu_startup_entry+0x28/0x2c
+ cpu_startup_entry from rest_init+0x150/0x178
+ rest_init from start_kernel+0x57c/0x610
+irq event stamp: 24900
+CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: G W N
6.12.0-rc2-koelsch-00005-g48bcda684823 #2094
+hardirqs last enabled at (24899): [<c0a1c658>] default_idle_call+0xe8/0x104
+Tainted: [W]=WARN, [N]=TEST
+hardirqs last disabled at (24900): [<c0200b68>] __irq_svc+0x48/0xac
+Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
+softirqs last enabled at (24832): [<c0226bd4>] handle_softirqs+0x170/0x3cc
+Call trace:
+softirqs last disabled at (24821): [<c0227400>] __irq_exit_rcu+0xf0/0x148
+ unwind_backtrace from show_stack+0x10/0x14
+---[ end trace 0000000000000000 ]---
+ show_stack from dump_stack_lvl+0x78/0xa8
+ dump_stack_lvl from __warn+0x80/0x198
+ __warn from warn_slowpath_fmt+0xbc/0x120
+ warn_slowpath_fmt from trace_hardirqs_on+0x40/0x7c
+ trace_hardirqs_on from __irq_svc+0x94/0xac
+Exception stack(0xf0861f48 to 0xf0861f90)
+1f40: c026570c ebfa1e7c 00000000 00000001 c2220000 c026570c
+1f60: c120905c c1209020 c2220000 c106d640 c120905c 00000000 fffffff8 f0861f98
+1f80: c0a1c4fc c0a1c554 20000113 ffffffff
+ __irq_svc from cpu_idle_poll+0x114/0x130
+ cpu_idle_poll from do_idle+0xc0/0x2a0
+ do_idle from cpu_startup_entry+0x28/0x2c
+ cpu_startup_entry from secondary_start_kernel+0xec/0x104
+ secondary_start_kernel from 0x40201334
+irq event stamp: 16791
+hardirqs last enabled at (16791): [<c0265764>] do_idle+0x118/0x2a0
+hardirqs last disabled at (16790): [<c02656d8>] do_idle+0x8c/0x2a0
+softirqs last enabled at (16782): [<c0226bd4>] handle_softirqs+0x170/0x3cc
+softirqs last disabled at (16765): [<c0227400>] __irq_exit_rcu+0xf0/0x148
+---[ end trace 0000000000000000 ]---
+PM: suspend devices took 0.430 seconds
Disabling non-boot CPUs ...
Enabling non-boot CPUs ...
CPU1 is up
Unfortunately it cannot be reverted easily.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Thu, Oct 03, 2024 at 06:16:29PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The trace_*_rcuidle() variant of a tracepoint was to handle places where a > tracepoint was located but RCU was not "watching". All those locations > have been removed, and RCU should be watching where all tracepoints are > located. We can now remove the trace_*_rcuidle() variant. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- W00t Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
© 2016 - 2026 Red Hat, Inc.