From: Peter Zijlstra <peterz@infradead.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
linux-kernel@vger.kernel.org,
Michael Jeanson <mjeanson@efficios.com>,
Alexei Starovoitov <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
"Paul E . McKenney" <paulmck@kernel.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
bpf@vger.kernel.org, Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH v4 2/5] tracing/ftrace: Add support for faultable tracepoints
Date: Mon, 20 Nov 2023 23:15:24 +0100 [thread overview]
Message-ID: <20231120221524.GD8262@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20231120205418.334172-3-mathieu.desnoyers@efficios.com>
On Mon, Nov 20, 2023 at 03:54:15PM -0500, Mathieu Desnoyers wrote:
> @@ -380,8 +415,8 @@ static inline notrace int trace_event_get_offsets_##call( \
>
> #include "stages/stage6_event_callback.h"
>
> -#undef DECLARE_EVENT_CLASS
> -#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
> +#undef _DECLARE_EVENT_CLASS
> +#define _DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print, tp_flags) \
> \
> static notrace void \
> trace_event_raw_event_##call(void *__data, proto) \
> @@ -392,8 +427,13 @@ trace_event_raw_event_##call(void *__data, proto) \
> struct trace_event_raw_##call *entry; \
> int __data_size; \
> \
> + if ((tp_flags) & TRACEPOINT_MAY_FAULT) { \
> + might_fault(); \
> + preempt_disable_notrace(); \
> + } \
> + \
> if (trace_trigger_soft_disabled(trace_file)) \
> - return; \
> + goto end; \
> \
> __data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
> \
> @@ -401,14 +441,28 @@ trace_event_raw_event_##call(void *__data, proto) \
> sizeof(*entry) + __data_size); \
> \
> if (!entry) \
> - return; \
> + goto end; \
> \
> tstruct \
> \
> { assign; } \
> \
> trace_event_buffer_commit(&fbuffer); \
> +end: \
> + if ((tp_flags) & TRACEPOINT_MAY_FAULT) \
> + preempt_enable_notrace(); \
> }
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 82cb22ad6d61..954f87515668 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -532,9 +532,16 @@ int trace_event_reg(struct trace_event_call *call,
> WARN_ON(!(call->flags & TRACE_EVENT_FL_TRACEPOINT));
> switch (type) {
> case TRACE_REG_REGISTER:
> - return tracepoint_probe_register(call->tp,
> - call->class->probe,
> - file);
> + if (call->tp->flags & TRACEPOINT_MAY_FAULT)
> + return tracepoint_probe_register_prio_flags(call->tp,
> + call->class->probe,
> + file,
> + TRACEPOINT_DEFAULT_PRIO,
> + TRACEPOINT_MAY_FAULT);
> + else
> + return tracepoint_probe_register(call->tp,
> + call->class->probe,
> + file);
> case TRACE_REG_UNREGISTER:
> tracepoint_probe_unregister(call->tp,
> call->class->probe,
> @@ -543,9 +550,16 @@ int trace_event_reg(struct trace_event_call *call,
>
> #ifdef CONFIG_PERF_EVENTS
> case TRACE_REG_PERF_REGISTER:
> - return tracepoint_probe_register(call->tp,
> - call->class->perf_probe,
> - call);
> + if (call->tp->flags & TRACEPOINT_MAY_FAULT)
> + return tracepoint_probe_register_prio_flags(call->tp,
> + call->class->perf_probe,
> + call,
> + TRACEPOINT_DEFAULT_PRIO,
> + TRACEPOINT_MAY_FAULT);
> + else
> + return tracepoint_probe_register(call->tp,
> + call->class->perf_probe,
> + call);
> case TRACE_REG_PERF_UNREGISTER:
> tracepoint_probe_unregister(call->tp,
> call->class->perf_probe,
I think something like the below (which is on top of the cleanup patch
in tip/locking/core) might just do...
---
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..37cbdb19d81d 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -151,7 +151,9 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
#define DEFINE_GUARD(_name, _type, _lock, _unlock) \
DEFINE_CLASS(_name, _type, if (_T) { _unlock; }, ({ _lock; _T; }), _type _T); \
static inline void * class_##_name##_lock_ptr(class_##_name##_t *_T) \
- { return *_T; }
+ { return *_T; } \
+ static inline class_##_name##_t class_##_name##_null(void) \
+ { return NULL; }
#define DEFINE_GUARD_COND(_name, _ext, _condlock) \
EXTEND_CLASS(_name, _ext, \
@@ -175,6 +177,17 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
if (!__guard_ptr(_name)(&scope)) _fail; \
else
+
+#define __guard_if(_cond, _name, var) \
+ class_##_name##_t var __cleanup(class_##_name##_destructor) = \
+ class_##_name##_null(); \
+ if (_cond) \
+ var = class_##_name##_constructor
+
+#define guard_if(_cond, _name) \
+ __guard_if(_cond, _name, __UNIQUE_ID(guard))
+
+
/*
* Additional helper macros for generating lock guards with types, either for
* locks that don't have a native type (eg. RCU, preempt) or those that need a
@@ -209,6 +222,12 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) \
static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T) \
{ \
return _T->lock; \
+} \
+ \
+static inline class_##_name##_t class_##_name##_null(void) \
+{ \
+ class_##_name##_t _t = { }; \
+ return _t; \
}
diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 558af4960157..8e063c9846e0 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -427,13 +427,10 @@ trace_event_raw_event_##call(void *__data, proto) \
struct trace_event_raw_##call *entry; \
int __data_size; \
\
- if ((tp_flags) & TRACEPOINT_MAY_FAULT) { \
- might_fault(); \
- preempt_disable_notrace(); \
- } \
+ guard_if((tp_flags) & TRACEPOINT_MAY_FAULT, preempt_notrace)(); \
\
if (trace_trigger_soft_disabled(trace_file)) \
- goto end; \
+ return; \
\
__data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
\
@@ -441,16 +438,13 @@ trace_event_raw_event_##call(void *__data, proto) \
sizeof(*entry) + __data_size); \
\
if (!entry) \
- goto end; \
+ return; \
\
tstruct \
\
{ assign; } \
\
trace_event_buffer_commit(&fbuffer); \
-end: \
- if ((tp_flags) & TRACEPOINT_MAY_FAULT) \
- preempt_enable_notrace(); \
}
#undef DECLARE_EVENT_CLASS
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index bf53d0d3eef9..09aec5db2e74 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -532,16 +532,11 @@ int trace_event_reg(struct trace_event_call *call,
WARN_ON(!(call->flags & TRACE_EVENT_FL_TRACEPOINT));
switch (type) {
case TRACE_REG_REGISTER:
- if (call->tp->flags & TRACEPOINT_MAY_FAULT)
- return tracepoint_probe_register_prio_flags(call->tp,
- call->class->probe,
- file,
- TRACEPOINT_DEFAULT_PRIO,
- TRACEPOINT_MAY_FAULT);
- else
- return tracepoint_probe_register(call->tp,
- call->class->probe,
- file);
+ return tracepoint_probe_register_prio_flags(call->tp,
+ call->class->probe,
+ file,
+ TRACEPOINT_DEFAULT_PRIO,
+ call->tp->flags & TRACEPOINT_MAY_FAULT);
case TRACE_REG_UNREGISTER:
tracepoint_probe_unregister(call->tp,
call->class->probe,
@@ -550,16 +545,11 @@ int trace_event_reg(struct trace_event_call *call,
#ifdef CONFIG_PERF_EVENTS
case TRACE_REG_PERF_REGISTER:
- if (call->tp->flags & TRACEPOINT_MAY_FAULT)
- return tracepoint_probe_register_prio_flags(call->tp,
- call->class->perf_probe,
- call,
- TRACEPOINT_DEFAULT_PRIO,
- TRACEPOINT_MAY_FAULT);
- else
- return tracepoint_probe_register(call->tp,
- call->class->perf_probe,
- call);
+ return tracepoint_probe_register_prio_flags(call->tp,
+ call->class->perf_probe,
+ call,
+ TRACEPOINT_DEFAULT_PRIO,
+ call->tp->flags & TRACEPOINT_MAY_FAULT);
case TRACE_REG_PERF_UNREGISTER:
tracepoint_probe_unregister(call->tp,
call->class->perf_probe,
next prev parent reply other threads:[~2023-11-20 22:16 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-20 20:54 [PATCH v4 0/5] Faultable Tracepoints Mathieu Desnoyers
2023-11-20 20:54 ` [PATCH v4 1/5] tracing: Introduce faultable tracepoints Mathieu Desnoyers
2023-11-20 21:47 ` Peter Zijlstra
2023-11-20 22:18 ` Paul E. McKenney
2023-11-20 22:23 ` Peter Zijlstra
2023-11-20 23:56 ` Paul E. McKenney
2023-11-21 8:47 ` Peter Zijlstra
2023-11-21 14:06 ` Mathieu Desnoyers
2023-11-21 14:36 ` Peter Zijlstra
2023-11-21 14:40 ` Mathieu Desnoyers
2023-11-21 14:46 ` Peter Zijlstra
2023-11-21 14:56 ` Mathieu Desnoyers
2023-11-21 15:51 ` Paul E. McKenney
2023-11-21 15:52 ` Peter Zijlstra
2023-11-21 16:00 ` Mathieu Desnoyers
2023-11-21 16:07 ` Steven Rostedt
2023-11-21 16:11 ` Mathieu Desnoyers
2023-11-21 16:43 ` Paul E. McKenney
2023-11-21 16:50 ` Peter Zijlstra
2023-11-21 17:31 ` Peter Zijlstra
2023-11-21 16:41 ` Paul E. McKenney
2023-11-21 14:44 ` Steven Rostedt
2023-11-21 16:45 ` Paul E. McKenney
2023-11-21 15:58 ` Paul E. McKenney
2023-11-21 16:03 ` Peter Zijlstra
2023-11-21 16:46 ` Paul E. McKenney
2023-11-20 22:20 ` Steven Rostedt
2024-06-20 15:38 ` Mathieu Desnoyers
2023-11-20 20:54 ` [PATCH v4 2/5] tracing/ftrace: Add support for " Mathieu Desnoyers
2023-11-20 22:15 ` Peter Zijlstra [this message]
2024-06-20 15:04 ` Mathieu Desnoyers
2023-11-20 20:54 ` [PATCH v4 3/5] tracing/bpf-trace: add " Mathieu Desnoyers
2023-11-20 20:54 ` [PATCH v4 4/5] tracing/perf: " Mathieu Desnoyers
2023-11-20 20:54 ` [PATCH v4 5/5] tracing: convert sys_enter/exit to " Mathieu Desnoyers
2023-11-20 21:46 ` Peter Zijlstra
2024-06-20 15:05 ` Mathieu Desnoyers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231120221524.GD8262@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=joel@joelfernandes.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=mjeanson@efficios.com \
--cc=namhyung@kernel.org \
--cc=paulmck@kernel.org \
--cc=rostedt@goodmis.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox