tracing: Updates to the last pull request for 5.18
Subsystems (6)
Steven Rostedt
maintainercover tracing: Updates to the last pull request for 5.18
Updates to Tracing:
- Rename the staging files to give them some meaning. Just stage1,stag2,etc, does not show what they are for
- Check for NULL from allocation in bootconfig
- Hold event mutex for dyn_event call in user events
- Mark user events to broken (to work on the API)
- Remove eBPF updates from user events
- Remove user events from uapi header to keep it from being installed.
- Move ftrace_graph_is_dead() into inline as it is called from hot paths and also convert it into a static branch.
Beau Belgrave (2):
tracing/user_events: Hold event_mutex during dyn_event_add tracing/user_events: Remove eBPF interfaces
Christophe Leroy (1):
ftrace: Make ftrace_graph_is_dead() a static branch
Lv Ruyi (1):
proc: bootconfig: Add null pointer check
Steven Rostedt (Google) (3):
tracing: Rename the staging files for trace_events
tracing: Set user_events to BROKEN
tracing: Move user_events.h temporarily out of include/uapi
----
Documentation/trace/user_events.rst | 14 +---
fs/proc/bootconfig.c | 2 +
include/linux/ftrace.h | 16 +++-
include/{uapi => }/linux/user_events.h | 53 -------------
.../{stage1_defines.h => stage1_struct_define.h} | 0
.../{stage2_defines.h => stage2_data_offsets.h} | 0
.../{stage3_defines.h => stage3_trace_output.h} | 0
.../{stage4_defines.h => stage4_event_fields.h} | 0
.../{stage5_defines.h => stage5_get_offsets.h} | 0
.../{stage6_defines.h => stage6_event_callback.h} | 0
.../{stage7_defines.h => stage7_class_define.h} | 0
include/trace/trace_custom_events.h | 14 ++--
include/trace/trace_events.h | 14 ++--
kernel/trace/Kconfig | 1 +
kernel/trace/fgraph.c | 17 +----
kernel/trace/trace_events_user.c | 86 +++-------------------
16 files changed, 50 insertions(+), 167 deletions(-)
rename include/{uapi => }/linux/user_events.h (58%)
rename include/trace/stages/{stage1_defines.h => stage1_struct_define.h} (100%)
rename include/trace/stages/{stage2_defines.h => stage2_data_offsets.h} (100%)
rename include/trace/stages/{stage3_defines.h => stage3_trace_output.h} (100%)
rename include/trace/stages/{stage4_defines.h => stage4_event_fields.h} (100%)
rename include/trace/stages/{stage5_defines.h => stage5_get_offsets.h} (100%)
rename include/trace/stages/{stage6_defines.h => stage6_event_callback.h} (100%)
rename include/trace/stages/{stage7_defines.h => stage7_class_define.h} (100%)Steven Rostedt
maintainerPATCH 1/7 tracing: Rename the staging files for trace_events
From: "Steven Rostedt (Google)" <[email protected]>
When looking for implementation of different phases of the creation of the TRACE_EVENT() macro, it is pretty useless when all helper macro redefinitions are in files labeled "stageX_defines.h". Rename them to state which phase the files are for. For instance, when looking for the defines that are used to create the event fields, seeing "stage4_event_fields.h" gives the developer a good idea that the defines are in that file.
---
.../{stage1_defines.h => stage1_struct_define.h} | 0
.../{stage2_defines.h => stage2_data_offsets.h} | 0
.../{stage3_defines.h => stage3_trace_output.h} | 0
.../{stage4_defines.h => stage4_event_fields.h} | 0
.../{stage5_defines.h => stage5_get_offsets.h} | 0
.../{stage6_defines.h => stage6_event_callback.h} | 0
.../{stage7_defines.h => stage7_class_define.h} | 0
include/trace/trace_custom_events.h | 14 +++++++-------
include/trace/trace_events.h | 14 +++++++-------
9 files changed, 14 insertions(+), 14 deletions(-)
rename include/trace/stages/{stage1_defines.h => stage1_struct_define.h} (100%)
rename include/trace/stages/{stage2_defines.h => stage2_data_offsets.h} (100%)
rename include/trace/stages/{stage3_defines.h => stage3_trace_output.h} (100%)
rename include/trace/stages/{stage4_defines.h => stage4_event_fields.h} (100%)
rename include/trace/stages/{stage5_defines.h => stage5_get_offsets.h} (100%)
rename include/trace/stages/{stage6_defines.h => stage6_event_callback.h} (100%)
rename include/trace/stages/{stage7_defines.h => stage7_class_define.h} (100%)include/trace/stages/stage1_struct_define.h renamed from include/trace/stages/stage1_defines.h +0 -0
include/trace/stages/stage2_data_offsets.h renamed from include/trace/stages/stage2_defines.h +0 -0
include/trace/stages/stage3_trace_output.h renamed from include/trace/stages/stage3_defines.h +0 -0
include/trace/stages/stage4_event_fields.h renamed from include/trace/stages/stage4_defines.h +0 -0
include/trace/stages/stage5_get_offsets.h renamed from include/trace/stages/stage5_defines.h +0 -0
include/trace/stages/stage6_event_callback.h renamed from include/trace/stages/stage6_defines.h +0 -0
include/trace/stages/stage7_class_define.h renamed from include/trace/stages/stage7_defines.h +0 -0
include/trace/trace_custom_events.h +7 -7
@@ -35,7 +35,7 @@/* Stage 1 creates the structure of the recorded event layout */ -#include "stages/stage1_defines.h" +#include "stages/stage1_struct_define.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (name, proto, args, tstruct, assign, print) \
@@ -56,7 +56,7 @@/* Stage 2 creates the custom class */ -#include "stages/stage2_defines.h" +#include "stages/stage2_data_offsets.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -71,7 +71,7 @@/* Stage 3 create the way to print the custom event */ -#include "stages/stage3_defines.h" +#include "stages/stage3_trace_output.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -102,7 +102,7 @@ static struct trace_event_functions trace_custom_event_type_funcs_##call = { \/* Stage 4 creates the offset layout for the fields */ -#include "stages/stage4_defines.h" +#include "stages/stage4_event_fields.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (call, proto, args, tstruct, func, print) \
@@ -114,7 +114,7 @@ static struct trace_event_fields trace_custom_event_fields_##call[] = { \/* Stage 5 creates the helper function for dynamic fields */ -#include "stages/stage5_defines.h" +#include "stages/stage5_get_offsets.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -134,7 +134,7 @@ static inline notrace int trace_custom_event_get_offsets_##call( \/* Stage 6 creates the probe function that records the event */ -#include "stages/stage6_defines.h" +#include "stages/stage6_event_callback.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -182,7 +182,7 @@ static inline void ftrace_test_custom_probe_##call(void) \/* Stage 7 creates the actual class and event structure for the custom event */ -#include "stages/stage7_defines.h" +#include "stages/stage7_class_define.h" #undef DECLARE_CUSTOM_EVENT_CLASS#define DECLARE_CUSTOM_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
include/trace/trace_events.h +7 -7
@@ -45,7 +45,7 @@ PARAMS(print)); \DEFINE_EVENT (name, name,PARAMS (proto),PARAMS (args)); -#include "stages/stage1_defines.h" +#include "stages/stage1_struct_define.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (name, proto, args, tstruct, assign, print) \
@@ -109,7 +109,7 @@* The size of an array is also encoded, in the higher 16 bits of <item>. */ -#include "stages/stage2_defines.h" +#include "stages/stage2_data_offsets.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -181,7 +181,7 @@* in binary. */ -#include "stages/stage3_defines.h" +#include "stages/stage3_trace_output.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -236,7 +236,7 @@ static struct trace_event_functions trace_event_type_funcs_##call = { \#include TRACE_INCLUDE (TRACE_INCLUDE_FILE ) -#include "stages/stage4_defines.h" +#include "stages/stage4_event_fields.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (call, proto, args, tstruct, func, print) \
@@ -249,7 +249,7 @@ static struct trace_event_fields trace_event_fields_##call[] = { \#include TRACE_INCLUDE (TRACE_INCLUDE_FILE ) -#include "stages/stage5_defines.h" +#include "stages/stage5_get_offsets.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -372,7 +372,7 @@ static inline notrace int trace_event_get_offsets_##call( \#define _TRACE_PERF_INIT (call)#endif /* CONFIG_PERF_EVENTS */ -#include "stages/stage6_defines.h" +#include "stages/stage6_event_callback.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
@@ -418,7 +418,7 @@ static inline void ftrace_test_probe_##call(void) \#include TRACE_INCLUDE (TRACE_INCLUDE_FILE ) -#include "stages/stage7_defines.h" +#include "stages/stage7_class_define.h" #undef DECLARE_EVENT_CLASS#define DECLARE_EVENT_CLASS (call, proto, args, tstruct, assign, print) \
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 2/7 proc: bootconfig: Add null pointer check
In reply to Steven RostedtFrom: Lv Ruyi <[email protected]>
kzalloc is a memory allocation function which can return NULL when some internal memory errors happen. It is safer to add null pointer check.
--- fs/proc/bootconfig.c | 2 ++ 1 file changed, 2 insertions(+)
fs/proc/bootconfig.c +2 -0
@@ -32,6 +32,8 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size)int ret =0 ; key =kzalloc (XBC_KEYLEN_MAX ,GFP_KERNEL ); +if (!key) +return -ENOMEM ;xbc_for_each_key_value (leaf, val) { ret =xbc_node_compose_key (leaf, key,XBC_KEYLEN_MAX );
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 3/7 tracing/user_events: Hold event_mutex during dyn_event_add
In reply to Steven RostedtFrom: Beau Belgrave <[email protected]>
Make sure the event_mutex is properly held during dyn_event_add call. This is required when adding dynamic events.
--- kernel/trace/trace_events_user.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
kernel/trace/trace_events_user.c +6 -2
@@ -1165,11 +1165,11 @@ static int user_event_parse(char *name, char *args, char *flags,#endif mutex_lock (&event_mutex); + ret =user_event_trace_register (user); -mutex_unlock (&event_mutex);if (ret) - goto put_user; + goto put_user_lock; user->index = index;
@@ -1181,8 +1181,12 @@ static int user_event_parse(char *name, char *args, char *flags,set_bit (user->index, page_bitmap);hash_add (register_table, &user->node, key); +mutex_unlock (&event_mutex); + *newuser = user;return 0 ; +put_user_lock: +mutex_unlock (&event_mutex); put_user:user_event_destroy_fields (user);user_event_destroy_validators (user);
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 4/7 tracing/user_events: Remove eBPF interfaces
In reply to Steven RostedtFrom: Beau Belgrave <[email protected]>
Remove eBPF interfaces within user_events to ensure they are fully reviewed.
--- Documentation/trace/user_events.rst | 14 ++---- include/uapi/linux/user_events.h | 53 --------------------- kernel/trace/trace_events_user.c | 73 +---------------------------- 3 files changed, 4 insertions(+), 136 deletions(-)
Documentation/trace/user_events.rst +3 -11
@@ -7,7 +7,7 @@ user_events: User-based Event Tracing Overview -------- User based trace events allow user processes to create events and trace data -that can be viewed via existing tools, such as ftrace, perf and eBPF. +that can be viewed via existing tools, such as ftrace and perf. To enable this feature, build your kernel with CONFIG_USER_EVENTS=y. Programs can view status of the events via
@@ -67,8 +67,7 @@ The command string format is as follows:: Supported Flags ^^^^^^^^^^^^^^^ -**BPF_ITER** - EBPF programs attached to this event will get the raw iovec -struct instead of any data copies for max performance. +None yet Field Format ^^^^^^^^^^^^
@@ -160,7 +159,7 @@ The following values are defined to aid in checking what has been attached: **EVENT_STATUS_FTRACE** - Bit set if ftrace has been attached (Bit 0). -**EVENT_STATUS_PERF** - Bit set if perf/eBPF has been attached (Bit 1). +**EVENT_STATUS_PERF** - Bit set if perf has been attached (Bit 1). Writing Data ------------
@@ -204,13 +203,6 @@ It's advised for user programs to do the following:: **NOTE:** *The write_index is not emitted out into the trace being recorded.* -EBPF ----- -EBPF programs that attach to a user-based event tracepoint are given a pointer -to a struct user_bpf_context. The bpf context contains the data type (which can -be a user or kernel buffer, or can be a pointer to the iovec) and the data -length that was emitted (minus the write_index). - Example Code ------------ See sample code in samples/user_events.
include/uapi/linux/user_events.h +0 -53
63 lines not on this page · raw message
kernel/trace/trace_events_user.c +1 -72
110 lines not on this page · raw message
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 5/7 tracing: Set user_events to BROKEN
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
After being merged, user_events become more visible to a wider audience that have concerns with the current API. It is too late to fix this for this release, but instead of a full revert, just mark it as BROKEN (which prevents it from being selected in make config). Then we can work finding a better API. If that fails, then it will need to be completely reverted.
--- kernel/trace/Kconfig | 1 + 1 file changed, 1 insertion(+)
kernel/trace/Kconfig +1 -0
@@ -741,6 +741,7 @@ config USER_EVENTS bool "User trace events" select TRACING select DYNAMIC_EVENTS + depends on BROKEN || COMPILE_TEST # API needs to be straighten out help User trace events are user-defined trace events that can be used like an existing kernel trace event. User trace
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 6/7 ftrace: Make ftrace_graph_is_dead() a static branch
In reply to Steven RostedtFrom: Christophe Leroy <[email protected]>
ftrace_graph_is_dead() is used on hot paths, it just reads a variable in memory and is not worth suffering function call constraints.
For instance, at entry of prepare_ftrace_return(), inlining it avoids saving prepare_ftrace_return() parameters to stack and restoring them after calling ftrace_graph_is_dead().
While at it using a static branch is even more performant and is rather well adapted considering that the returned value will almost never change.
Inline ftrace_graph_is_dead() and replace 'kill_ftrace_graph' bool by a static branch.
The performance improvement is noticeable.
--- include/linux/ftrace.h | 16 +++++++++++++++- kernel/trace/fgraph.c | 17 +++-------------- 2 files changed, 18 insertions(+), 15 deletions(-)
include/linux/ftrace.h +15 -1
@@ -9,6 +9,7 @@#include <linux/trace_recursion.h> #include <linux/trace_clock.h> +#include <linux/jump_label.h> #include <linux/kallsyms.h> #include <linux/linkage.h> #include <linux/bitops.h>
@@ -1015,7 +1016,20 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,extern int register_ftrace_graph (struct fgraph_ops *ops);extern void unregister_ftrace_graph (struct fgraph_ops *ops); -extern bool ftrace_graph_is_dead (void ); +/** +* ftrace_graph_is_dead - returns true if ftrace_graph_stop() was called +* +* ftrace_graph_stop() is called when a severe error is detected in +* the function graph tracing. This function is called by the critical +* paths of function graph to keep those paths from doing any more harm. +*/ +DECLARE_STATIC_KEY_FALSE (kill_ftrace_graph); + +static inline bool ftrace_graph_is_dead (void ) +{ +return static_branch_unlikely (&kill_ftrace_graph); +} +extern void ftrace_graph_stop (void );/* The current handlers in use */
kernel/trace/fgraph.c +3 -14
@@ -7,6 +7,7 @@* * Highly modified by Steven Rostedt (VMware). */ +#include <linux/jump_label.h> #include <linux/suspend.h> #include <linux/ftrace.h> #include <linux/slab.h>
@@ -23,24 +24,12 @@#define ASSIGN_OPS_HASH (opsname, val)#endif -static bool kill_ftrace_graph; +DEFINE_STATIC_KEY_FALSE (kill_ftrace_graph);int ftrace_graph_active;/* Both enabled by default (can be cleared by function_graph tracer flags */ static bool fgraph_sleep_time = true; -/** -* ftrace_graph_is_dead - returns true if ftrace_graph_stop() was called -* -* ftrace_graph_stop() is called when a severe error is detected in -* the function graph tracing. This function is called by the critical -* paths of function graph to keep those paths from doing any more harm. -*/ -bool ftrace_graph_is_dead (void ) -{ -return kill_ftrace_graph; -} -/** * ftrace_graph_stop - set to permanently disable function graph tracing *
@@ -51,7 +40,7 @@ bool ftrace_graph_is_dead(void)*/ void ftrace_graph_stop (void ) { - kill_ftrace_graph = true; +static_branch_enable (&kill_ftrace_graph); }/* Add a function return address to the trace stack on thread info.*/
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 7/7 tracing: Move user_events.h temporarily out of include/uapi
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
While user_events API is under development and has been marked for broken to not let the API become fixed, move the header file out of the uapi directory. This is to prevent it from being installed, then later changed, and then have an old distro user space update with a new kernel, where applications see the user_events being available, but the old header is in place, and then they get compiled incorrectly.
Also, surround the include with CONFIG_COMPILE_TEST to the current location, but when the BROKEN tag is taken off, it will use the uapi directory, and fail to compile. This is a good way to remind us to move the header back.
---
include/{uapi => }/linux/user_events.h | 0
kernel/trace/trace_events_user.c | 5 +++++
2 files changed, 5 insertions(+)
rename include/{uapi => }/linux/user_events.h (100%)include/linux/user_events.h renamed from include/uapi/linux/user_events.h +0 -0
kernel/trace/trace_events_user.c +5 -0
@@ -18,7 +18,12 @@#include <linux/tracefs.h> #include <linux/types.h> #include <linux/uaccess.h> +/* Reminder to move to uapi when everything works */ +#ifdef CONFIG_COMPILE_TEST +#include <linux/user_events.h> +#else #include <uapi/linux/user_events.h> +#endif #include "trace.h" #include "trace_dynevent.h"
Signature · 1 line
2.35.1