| 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
| ||
|
Message-ID: <CAEXW_YQQGoXHQhwCu+m6z_O_0r=qj05KLqsQOF36sP-NP88JqQ@mail.gmail.com>
Date: Thu, 5 Dec 2024 12:32:52 -0500
From: Joel Fernandes <joel@...lfernandes.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Mark Rutland <mark.rutland@....com>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v2] tracing: Remove definition of trace_*_rcuidle()
On Thu, Oct 3, 2024 at 6:15 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> From: Steven Rostedt <rostedt@...dmis.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@...dmis.org>
Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.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
>
Powered by blists - more mailing lists