Re: [PATCH v4 2/5] tracing/ftrace: Add support for faultable tracepoints
From: Peter Zijlstra
Date: Mon Nov 20 2023 - 17:17:39 EST
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,