user_events: Enable user processes to create and write to trace events
Subsystems (3)
PATCH v9 3/12 user_events: Handle matching arguments from dyn_events
In reply to Beau BelgraveEnsures that when dynamic events requests a match with arguments that they match what is in the user_event.
--- kernel/trace/trace_events_user.c | 77 +++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)
kernel/trace/trace_events_user.c +76 -1
@@ -39,6 +39,7 @@#define MAX_EVENT_DESC 512#define EVENT_NAME (user_event) ((user_event)->tracepoint.name)#define MAX_FIELD_ARRAY_SIZE 1024 +#define MAX_FIELD_ARG_NAME 256static char *register_page_data;
@@ -695,13 +696,87 @@ static int user_event_free(struct dyn_event *ev)return destroy_user_event (user); } +static bool user_field_match (struct ftrace_event_field *field,int argc, +const char **argv,int *iout) +{ +char *field_name, *arg_name; +int len, pos, i = *iout; +bool colon = false, match = false; + +if (i >= argc) +return false; + + len =MAX_FIELD_ARG_NAME ; + field_name =kmalloc (len,GFP_KERNEL ); + arg_name =kmalloc (len,GFP_KERNEL ); + +if (!arg_name || !field_name) + goto out; + + pos =0 ; + +for (; i < argc; ++i) { +if (i != *iout) + pos +=snprintf (arg_name + pos, len - pos," " ); + + pos +=snprintf (arg_name + pos, len - pos, argv[i]); + +if (strchr (argv[i],';' )) { + ++i; + colon = true; +break ; + } + } + + pos =0 ; + + pos +=snprintf (field_name + pos, len - pos, field->type); + pos +=snprintf (field_name + pos, len - pos," " ); + pos +=snprintf (field_name + pos, len - pos, field->name); + +if (colon) + pos +=snprintf (field_name + pos, len - pos,";" ); + + *iout = i; + + match =strcmp (arg_name, field_name) ==0 ; +out: +kfree (arg_name); +kfree (field_name); + +return match; +} + +static bool user_fields_match (struct user_event *user,int argc, +const char **argv) +{ +struct ftrace_event_field *field, *next; +struct list_head *head = &user->fields; +int i =0 ; + +list_for_each_entry_safe_reverse (field, next, head, link) +if (!user_field_match (field, argc, argv, &i)) +return false; + +if (i != argc) +return false; + +return true; +} +static bool user_event_match (const char *system,const char *event,int argc,const char **argv,struct dyn_event *ev) { struct user_event *user = container_of(ev, struct user_event, devent); +bool match; -return strcmp (EVENT_NAME (user), event) ==0 && + match =strcmp (EVENT_NAME (user), event) ==0 && (!system ||strcmp (system,USER_EVENTS_SYSTEM ) ==0 ); + +if (match && argc >0 ) + match =user_fields_match (user, argc, argv); + +return match; }static struct dyn_event_operations user_event_dops = {
Signature · 1 line
2.17.1
v9 cover user_events: Enable user processes to create and write to trace events
User mode processes that wish to use trace events to get data into ftrace, perf, eBPF, etc are limited to uprobes today. The user events features enables an ABI for user mode processes to create and write to trace events that are isolated from kernel level trace events. This enables a faster path for tracing from user mode data as well as opens managed code to participate in trace events, where stub locations are dynamic.
User processes often want to trace only when it's useful. To enable this a set of pages are mapped into the user process space that indicate the current state of the user events that have been registered. User processes can check if their event is hooked to a trace/probe, and if it is, emit the event data out via the write() syscall.
Two new files are introduced into tracefs to accomplish this:user_events_status - This file is mmap'd into participating user mode processes to indicate event status.
user_events_data - This file is opened and register/delete ioctl's are issued to create/open/delete trace events that can be used for tracing.
The typical scenario is on process start to mmap user_events_status. Processes then register the events they plan to use via the REG ioctl. The ioctl reads and updates the passed in user_reg struct. The status_index of the struct is used to know the byte in the status page to check for that event. The write_index of the struct is used to describe that event when writing out to the fd that was used for the ioctl call. The data must always include this index first when writing out data for an event. Data can be written either by write() or by writev().
For example, in memory:
int index;
char data[];
Psuedo code example of typical usage:
struct user_reg reg;
int page_fd = open("user_events_status", O_RDWR);
char *page_data = mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED, page_fd, 0); close(page_fd);
int data_fd = open("user_events_data", O_RDWR);
reg.size = sizeof(reg);
reg.name_args = (__u64)"test";
ioctl(data_fd, DIAG_IOCSREG, ®);
int status_id = reg.status_index;
int write_id = reg.write_index;
struct iovec io[2];
io[0].iov_base = &write_id;
io[0].iov_len = sizeof(write_id);
io[1].iov_base = payload;
io[1].iov_len = sizeof(payload);
if (page_data[status_id])
writev(data_fd, io, 2);
User events are also exposed via the dynamic_events tracefs file for both create and delete. Current status is exposed via the user_events_status tracefs file.
Simple example to register a user event via dynamic_events:
echo u:test >> dynamic_events
cat dynamic_events
u:test
If an event is hooked to a probe, the probe hooked shows up:
echo 1 > events/user_events/test/enable cat user_events_status 1:test # Used by ftrace Active: 1 Busy: 1 Max: 4096
If an event is not hooked to a probe, no probe status shows up:
echo 0 > events/user_events/test/enable cat user_events_status 1:test Active: 1 Busy: 0 Max: 4096
Users can describe the trace event format via the following format:
name[:FLAG1[,FLAG2...] [field1[;field2...]]
Each field has the following format:
type name
Example for char array with a size of 20 named msg:
echo 'u:detailed char[20] msg' >> dynamic_events cat dynamic_events
u:detailed char[20] msg
Data offsets are based on the data written out via write() and will be updated to reflect the correct offset in the trace_event fields. For dynamic data it is recommended to use the new __rel_loc data type. This type will be the same as __data_loc, but the offset is relative to this entry. This allows user_events to not worry about what common fields are being inserted before the data.
The above format is valid for both the ioctl and the dynamic_events file.
V2:
Fixed kmalloc vs kzalloc for register_page.
Renamed user_event_mmap to user_event_status.
Renamed user_event prefix from ue to u.
Added seq_* operations to user_event_status to enable cat output. Aligned field parsing to synth_events format (+ size specifier for custom/user types).
Added uapi header user_events.h to align kernel and user ABI definitions.
V3:
Updated ABI to handle single FD into many events via an int header. Added iovec/writev support to enable int header without payload changes. Updated bpf context to describe if data is coming from user, kernel or raw iovec.
Added flag support for registering event, allows forcing BPF to always recieve the direct iovecs for sensitive code paths that do not want copies.
V4:
Moved to struct user_reg for registering events via ioctl.
Added unit tests for ftrace, dyn_events and perf integration. Added print_fmt generation and proper dyn_events matching statements. Reduced time in preemption disabled paths.
Added documentation file.
Pre-fault in data when preemption is enabled and use no-fault copy in probes. Fixed MIPs missing PAGE_READONLY define.
V5:
Rebase to linux-trace for-next branch.
Added sample code into samples/user_events.
Switched to str_has_prefix in various locations.
Allow hex in array sizes and ensure reasonable sizes are used. Moved lifetime of name buffer when parsing to the caller for failure paths. Fixed documentation nits and index.
Ensure event isn't busy before freeing through dyn_events. Properly handle failure case for ftrace and perf in fault cases for buffers. Ensure write data is over min size and null terminated for dynamic arrays.
V6:
Fixed endian issue with dyn loc decoding (use u32).
Fixed size_t conversion warning on hexagon arch (min vs min_t). Handle cases for __get_str vs __get_rel_str in print_fmt generation. Add additional comments around various event member lifetimes. Reduced max field array size to 1K.
V7:
Acquire reg_mutex during release, ensure refs cannot change under any situation. Remove default n from Kconfig.
Move from static 0644 mode to TRACE_MODE_WRITE.
V8:
Squashed UABI header into ftrace minimal patch thread.
Moved pagefault_disable/enable into copy_nofault.
Moved to strscpy vs custom copy when getting array size from type. Made patch bisect friendly by ensuring tests are split from kernel code.
V9:
Rebase to linux-trace ftrace/core branch.
Added comments for user_reg and other structs in user_events.h. Moved from delayed seq_file to pre-created seq_file for status file. Added deleting events to documentation and expanded registering section. Reordered patches to make reviewing easier.
Fixed nitpicks.
Beau Belgrave (12):
user_events: Add minimal support for trace_event into ftrace
user_events: Add print_fmt generation support for basic types
user_events: Handle matching arguments from dyn_events
user_events: Add basic perf and eBPF support
user_events: Optimize writing events by only copying data once
user_events: Validate user payloads for size and null termination
user_events: Add self-test for ftrace integration
user_events: Add self-test for dynamic_events integration
user_events: Add self-test for perf_event integration
user_events: Add self-test for validator boundaries
user_events: Add sample code for typical usage
user_events: Add documentation file
Documentation/trace/index.rst | 1 + Documentation/trace/user_events.rst | 216 +++ include/uapi/linux/user_events.h | 116 ++ kernel/trace/Kconfig | 14 + kernel/trace/Makefile | 1 + kernel/trace/trace_events_user.c | 1609 +++++++++++++++++ samples/user_events/Makefile | 5 + samples/user_events/example.c | 91 + tools/testing/selftests/user_events/Makefile | 9 + .../testing/selftests/user_events/dyn_test.c | 130 ++ .../selftests/user_events/ftrace_test.c | 452 +++++ .../testing/selftests/user_events/perf_test.c | 168 ++ tools/testing/selftests/user_events/settings | 1 + 13 files changed, 2813 insertions(+) create mode 100644 Documentation/trace/user_events.rst create mode 100644 include/uapi/linux/user_events.h create mode 100644 kernel/trace/trace_events_user.c create mode 100644 samples/user_events/Makefile create mode 100644 samples/user_events/example.c create mode 100644 tools/testing/selftests/user_events/Makefile create mode 100644 tools/testing/selftests/user_events/dyn_test.c create mode 100644 tools/testing/selftests/user_events/ftrace_test.c create mode 100644 tools/testing/selftests/user_events/perf_test.c create mode 100644 tools/testing/selftests/user_events/settings
base-commit: 85c62c8c3749eec02ba81217bdcac26867dc262e
Signature · 1 line
2.17.1
PATCH v9 1/12 user_events: Add minimal support for trace_event into ftrace
Minimal support for interacting with dynamic events, trace_event and ftrace. Core outline of flow between user process, ioctl and trace_event APIs.
User mode processes that wish to use trace events to get data into ftrace, perf, eBPF, etc are limited to uprobes today. The user events features enables an ABI for user mode processes to create and write to trace events that are isolated from kernel level trace events. This enables a faster path for tracing from user mode data as well as opens managed code to participate in trace events, where stub locations are dynamic.
User processes often want to trace only when it's useful. To enable this a set of pages are mapped into the user process space that indicate the current state of the user events that have been registered. User processes can check if their event is hooked to a trace/probe, and if it is, emit the event data out via the write() syscall.
Two new files are introduced into tracefs to accomplish this:user_events_status - This file is mmap'd into participating user mode processes to indicate event status.
user_events_data - This file is opened and register/delete ioctl's are issued to create/open/delete trace events that can be used for tracing.
The typical scenario is on process start to mmap user_events_status. Processes then register the events they plan to use via the REG ioctl. The ioctl reads and updates the passed in user_reg struct. The status_index of the struct is used to know the byte in the status page to check for that event. The write_index of the struct is used to describe that event when writing out to the fd that was used for the ioctl call. The data must always include this index first when writing out data for an event. Data can be written either by write() or by writev().
For example, in memory:
int index;
char data[];
Psuedo code example of typical usage:
struct user_reg reg;
int page_fd = open("user_events_status", O_RDWR);
char *page_data = mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED, page_fd, 0); close(page_fd);
int data_fd = open("user_events_data", O_RDWR);
reg.size = sizeof(reg);
reg.name_args = (__u64)"test";
ioctl(data_fd, DIAG_IOCSREG, ®);
int status_id = reg.status_index;
int write_id = reg.write_index;
struct iovec io[2];
io[0].iov_base = &write_id;
io[0].iov_len = sizeof(write_id);
io[1].iov_base = payload;
io[1].iov_len = sizeof(payload);
if (page_data[status_id])
writev(data_fd, io, 2);
User events are also exposed via the dynamic_events tracefs file for both create and delete. Current status is exposed via the user_events_status tracefs file.
Simple example to register a user event via dynamic_events:
echo u:test >> dynamic_events
cat dynamic_events
u:test
If an event is hooked to a probe, the probe hooked shows up:
echo 1 > events/user_events/test/enable cat user_events_status 1:test # Used by ftrace Active: 1 Busy: 1 Max: 4096
If an event is not hooked to a probe, no probe status shows up:
echo 0 > events/user_events/test/enable cat user_events_status 1:test Active: 1 Busy: 0 Max: 4096
Users can describe the trace event format via the following format:
name[:FLAG1[,FLAG2...] [field1[;field2...]]
Each field has the following format:
type name
Example for char array with a size of 20 named msg:
echo 'u:detailed char[20] msg' >> dynamic_events cat dynamic_events
u:detailed char[20] msg
Data offsets are based on the data written out via write() and will be updated to reflect the correct offset in the trace_event fields. For dynamic data it is recommended to use the new __rel_loc data type. This type will be the same as __data_loc, but the offset is relative to this entry. This allows user_events to not worry about what common fields are being inserted before the data.
The above format is valid for both the ioctl and the dynamic_events file.
--- include/uapi/linux/user_events.h | 116 +++ kernel/trace/Kconfig | 14 + kernel/trace/Makefile | 1 + kernel/trace/trace_events_user.c | 1179 ++++++++++++++++++++++++++++++ 4 files changed, 1310 insertions(+) create mode 100644 include/uapi/linux/user_events.h create mode 100644 kernel/trace/trace_events_user.c
include/uapi/linux/user_events.h new file +116 -0
@@ -0,0 +1,116 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* +* Copyright (c) 2021, Microsoft Corporation. +* +* Authors: +* Beau Belgrave <[email protected]> +*/ +#ifndef _UAPI_LINUX_USER_EVENTS_H +#define _UAPI_LINUX_USER_EVENTS_H + +#include <linux/types.h> +#include <linux/ioctl.h> + +#ifdef __KERNEL__ +#include <linux/uio.h> +#else +#include <sys/uio.h> +#endif + +#define USER_EVENTS_SYSTEM "user_events" +#define USER_EVENTS_PREFIX "u:" + +/* Bits 0-6 are for known probe types, Bit 7 is for unknown probes */ +#define EVENT_BIT_FTRACE 0 +#define EVENT_BIT_PERF 1 +#define EVENT_BIT_OTHER 7 + +#define EVENT_STATUS_FTRACE (1 << EVENT_BIT_FTRACE) +#define EVENT_STATUS_PERF (1 << EVENT_BIT_PERF) +#define EVENT_STATUS_OTHER (1 << EVENT_BIT_OTHER) + +/* Create dynamic location entry within a 32-bit value */ +#define DYN_LOC (offset, size) ((size) << 16 | (offset)) + +/* Use raw iterator for attached BPF program(s), no affect on ftrace/perf */ +#define FLAG_BPF_ITER (1 << 0) + +/* +* Describes an event registration and stores the results of the registration. +* This structure is passed to the DIAG_IOCSREG ioctl, callers at a minimum +* must set the size and name_args before invocation. +*/ +struct user_reg { + +/* Input: Size of the user_reg structure being used */ +__u32 size; + +/* Input: Pointer to string with event name, description and flags */ +__u64 name_args; + +/* Output: Byte index of the event within the status page */ +__u32 status_index; + +/* Output: Index of the event to use when writing data */ +__u32 write_index; +}; + +#define DIAG_IOC_MAGIC '*' + +/* Requests to register a user_event */ +#define DIAG_IOCSREG _IOWR(DIAG_IOC_MAGIC, 0, struct user_reg*) + +/* Requests to delete a user_event */ +#define DIAG_IOCSDEL _IOW(DIAG_IOC_MAGIC, 1, char*) + +/* Data type that was passed to the BPF program */ +enum { +/* Data resides in kernel space */ +USER_BPF_DATA_KERNEL , + +/* Data resides in user space */ +USER_BPF_DATA_USER , + +/* Data is a pointer to a user_bpf_iter structure */ +USER_BPF_DATA_ITER , +}; + +/* +* Describes an iovec iterator that BPF programs can use to access data for +* a given user_event write() / writev() call. +*/ +struct user_bpf_iter { + +/* Offset of the data within the first iovec */ +__u32 iov_offset; + +/* Number of iovec structures */ +__u32 nr_segs; + +/* Pointer to iovec structures */ +const struct iovec *iov; +}; + +/* Context that BPF programs receive when attached to a user_event */ +struct user_bpf_context { + +/* Data type being passed (see union below) */ +__u32 data_type; + +/* Length of the data */ +__u32 data_len; + +/* Pointer to data, varies by data type */ +union { +/* Kernel data (data_type == USER_BPF_DATA_KERNEL) */ +void *kdata; + +/* User data (data_type == USER_BPF_DATA_USER) */ +void *udata; + +/* Direct iovec (data_type == USER_BPF_DATA_ITER) */ +struct user_bpf_iter *iter; + }; +}; + +#endif /* _UAPI_LINUX_USER_EVENTS_H */
kernel/trace/Kconfig +14 -0
@@ -724,6 +724,20 @@ config SYNTH_EVENTS If in doubt, say N. +config USER_EVENTS + bool "User trace events" + select TRACING + select DYNAMIC_EVENTS + help + User trace events are user-defined trace events that + can be used like an existing kernel trace event. User trace + events are generated by writing to a tracefs file. User + processes can determine if their tracing events should be + generated by memory mapping a tracefs file and checking for + an associated byte being non-zero. + + If in doubt, say N. + config HIST_TRIGGERS bool "Histogram triggers" depends on ARCH_HAVE_NMI_SAFE_CMPXCHG
kernel/trace/Makefile +1 -0
@@ -82,6 +82,7 @@ obj-$(CONFIG_PROBE_EVENTS) += trace_eprobe.o obj-$(CONFIG_TRACE_EVENT_INJECT) += trace_events_inject.o obj-$(CONFIG_SYNTH_EVENTS) += trace_events_synth.o obj-$(CONFIG_HIST_TRIGGERS) += trace_events_hist.o +obj-$(CONFIG_USER_EVENTS) += trace_events_user.o obj-$(CONFIG_BPF_EVENTS) += bpf_trace.o obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe.o obj-$(CONFIG_TRACEPOINTS) += error_report-traces.o
kernel/trace/trace_events_user.c new file +1179 -0
1,179 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v9 2/12 user_events: Add print_fmt generation support for basic types
In reply to Beau BelgraveAddes print_fmt format generation for basic types that are supported for user processes. Only supports sizes that are the same on 32 and 64 bit.
--- kernel/trace/trace_events_user.c | 115 ++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 2 deletions(-)
kernel/trace/trace_events_user.c +113 -2
@@ -359,6 +359,114 @@ static int user_event_parse_fields(struct user_event *user, char *args)static struct trace_event_fields user_event_fields_array[1 ]; +static const char *user_field_format (const char *type) +{ +if (strcmp (type,"s64" ) ==0 ) +return "%lld" ; +if (strcmp (type,"u64" ) ==0 ) +return "%llu" ; +if (strcmp (type,"s32" ) ==0 ) +return "%d" ; +if (strcmp (type,"u32" ) ==0 ) +return "%u" ; +if (strcmp (type,"int" ) ==0 ) +return "%d" ; +if (strcmp (type,"unsigned int" ) ==0 ) +return "%u" ; +if (strcmp (type,"s16" ) ==0 ) +return "%d" ; +if (strcmp (type,"u16" ) ==0 ) +return "%u" ; +if (strcmp (type,"short" ) ==0 ) +return "%d" ; +if (strcmp (type,"unsigned short" ) ==0 ) +return "%u" ; +if (strcmp (type,"s8" ) ==0 ) +return "%d" ; +if (strcmp (type,"u8" ) ==0 ) +return "%u" ; +if (strcmp (type,"char" ) ==0 ) +return "%d" ; +if (strcmp (type,"unsigned char" ) ==0 ) +return "%u" ; +if (strstr (type,"char[" ) !=0 ) +return "%s" ; + +/* Unknown, likely struct, allowed treat as 64-bit */ +return "%llu" ; +} + +static bool user_field_is_dyn_string (const char *type,const char **str_func) +{ +if (str_has_prefix (type,"__data_loc " )) { + *str_func ="__get_str" ; + goto check; + } + +if (str_has_prefix (type,"__rel_loc " )) { + *str_func ="__get_rel_str" ; + goto check; + } + +return false; +check: +return strstr (type,"char" ) !=0 ; +} + +#define LEN_OR_ZERO (len ? len - pos : 0) +static int user_event_set_print_fmt (struct user_event *user,char *buf,int len) +{ +struct ftrace_event_field *field, *next; +struct list_head *head = &user->fields; +int pos =0 , depth =0 ; +const char *str_func; + + pos +=snprintf (buf + pos,LEN_OR_ZERO ,"\"" ); + +list_for_each_entry_safe_reverse (field, next, head, link) { +if (depth !=0 ) + pos +=snprintf (buf + pos,LEN_OR_ZERO ," " ); + + pos +=snprintf (buf + pos,LEN_OR_ZERO ,"%s=%s" , + field->name,user_field_format (field->type)); + + depth++; + } + + pos +=snprintf (buf + pos,LEN_OR_ZERO ,"\"" ); + +list_for_each_entry_safe_reverse (field, next, head, link) { +if (user_field_is_dyn_string (field->type, &str_func)) + pos +=snprintf (buf + pos,LEN_OR_ZERO , +", %s(%s)" , str_func, field->name); +else + pos +=snprintf (buf + pos,LEN_OR_ZERO , +", REC->%s" , field->name); + } + +return pos +1 ; +} +#undef LEN_OR_ZERO + +static int user_event_create_print_fmt (struct user_event *user) +{ +char *print_fmt; +int len; + + len =user_event_set_print_fmt (user,NULL ,0 ); + + print_fmt =kmalloc (len,GFP_KERNEL ); + +if (!print_fmt) +return -ENOMEM ; + +user_event_set_print_fmt (user, print_fmt, len); + + user->call.print_fmt = print_fmt; + +return 0 ; +} +static enum print_line_t user_event_print_trace (struct trace_iterator *iter,int flags,struct trace_event *event)
@@ -392,6 +500,7 @@ static int destroy_user_event(struct user_event *user)clear_bit (user->index, page_bitmap);hash_del (&user->node); +kfree (user->call.print_fmt);kfree (EVENT_NAME (user));kfree (user);
@@ -664,8 +773,10 @@ static int user_event_parse(char *name, char *args, char *flags,if (ret) goto put_user; -/* Minimal print format */ - user->call.print_fmt ="\"\"" ; + ret =user_event_create_print_fmt (user); + +if (ret) + goto put_user; user->call.data = user; user->call.class = &user->class;
Signature · 1 line
2.17.1
PATCH v9 4/12 user_events: Add basic perf and eBPF support
In reply to Beau BelgraveAdds support to write out user_event data to perf_probe/perf files as well as to any attached eBPF program.
--- kernel/trace/trace_events_user.c | 72 +++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-)
kernel/trace/trace_events_user.c +71 -1
@@ -551,6 +551,50 @@ static void user_event_ftrace(struct user_event *user, void *data, u32 datalen,trace_event_buffer_commit (&event_buffer); } +#ifdef CONFIG_PERF_EVENTS +/* +* Writes the user supplied payload out to perf ring buffer or eBPF program. +*/ +static void user_event_perf (struct user_event *user,void *data,u32 datalen, +void *tpdata) +{ +struct hlist_head *perf_head; + +if (bpf_prog_array_valid (&user->call)) { +struct user_bpf_context context = {0 }; + + context.data_len = datalen; + context.data_type =USER_BPF_DATA_KERNEL ; + context.kdata = data; + +trace_call_bpf (&user->call, &context); + } + + perf_head =this_cpu_ptr (user->call.perf_events); + +if (perf_head && !hlist_empty (perf_head)) { +struct trace_entry *perf_entry; +struct pt_regs *regs; +size_t size =sizeof (*perf_entry) + datalen; +int context; + + perf_entry =perf_trace_buf_alloc (ALIGN (size,8 ), + ®s, &context); + +if (unlikely (!perf_entry)) +return ; + +perf_fetch_caller_regs (regs); + +memcpy (perf_entry +1 , data, datalen); + +perf_trace_buf_submit (perf_entry, size, context, + user->call.event.type,1 , regs, + perf_head,NULL ); + } +} +#endif +/* * Update the register page that is shared between user processes. */
@@ -573,6 +617,10 @@ static void update_reg_page_for(struct user_event *user)if (probe_func == user_event_ftrace) status |=EVENT_STATUS_FTRACE ; +#ifdef CONFIG_PERF_EVENTS + else if (probe_func == user_event_perf) + status |= EVENT_STATUS_PERF; +#endif else status |= EVENT_STATUS_OTHER; }while ((++probe_func_ptr)->func);
@@ -612,8 +660,27 @@ static int user_event_reg(struct trace_event_call *call, data); goto dec; -default : +#ifdef CONFIG_PERF_EVENTS +case TRACE_REG_PERF_REGISTER : + ret =tracepoint_probe_register (call->tp, + call->class->perf_probe, + data); +if (!ret) + goto inc; +break ; + +case TRACE_REG_PERF_UNREGISTER : +tracepoint_probe_unregister (call->tp, + call->class->perf_probe, + data); + goto dec; + +case TRACE_REG_PERF_OPEN : +case TRACE_REG_PERF_CLOSE : +case TRACE_REG_PERF_ADD : +case TRACE_REG_PERF_DEL :break ; +#endif }return ret;
@@ -865,6 +932,9 @@ static int user_event_parse(char *name, char *args, char *flags, user->class.get_fields = user_event_get_fields; user->class.reg = user_event_reg; user->class.probe = user_event_ftrace; +#ifdef CONFIG_PERF_EVENTS + user->class.perf_probe = user_event_perf; +#endif mutex_lock (&event_mutex); ret =user_event_trace_register (user);
Signature · 1 line
2.17.1
PATCH v9 8/12 user_events: Add self-test for dynamic_events integration
In reply to Beau BelgraveTests matching deletes, creation of basic and complex types. Ensures common patterns work correctly when interacting with dynamic_events file.
--- tools/testing/selftests/user_events/Makefile | 2 +- .../testing/selftests/user_events/dyn_test.c | 130 ++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/user_events/dyn_test.c
tools/testing/selftests/user_events/Makefile +1 -1
@@ -2,7 +2,7 @@ CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include LDLIBS += -lrt -lpthread -lm -TEST_GEN_PROGS = ftrace_test +TEST_GEN_PROGS = ftrace_test dyn_test TEST_FILES := settings
tools/testing/selftests/user_events/dyn_test.c new file +130 -0
130 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v9 7/12 user_events: Add self-test for ftrace integration
In reply to Beau BelgraveTests basic functionality of registering/deregistering, status and writing data out via ftrace mechanisms within user_events.
--- tools/testing/selftests/user_events/Makefile | 9 + .../selftests/user_events/ftrace_test.c | 387 ++++++++++++++++++ tools/testing/selftests/user_events/settings | 1 + 3 files changed, 397 insertions(+) create mode 100644 tools/testing/selftests/user_events/Makefile create mode 100644 tools/testing/selftests/user_events/ftrace_test.c create mode 100644 tools/testing/selftests/user_events/settings
tools/testing/selftests/user_events/Makefile new file +9 -0
@@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0 +CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include +LDLIBS += -lrt -lpthread -lm + +TEST_GEN_PROGS = ftrace_test + +TEST_FILES := settings + +include ../lib.mk
tools/testing/selftests/user_events/ftrace_test.c new file +387 -0
387 lines not on this page · raw message
tools/testing/selftests/user_events/settings new file +1 -0
@@ -0,0 +1 @@ +timeout=90
Signature · 1 line
2.17.1
PATCH v9 9/12 user_events: Add self-test for perf_event integration
In reply to Beau BelgraveTests perf can be attached to and written out correctly. Ensures attach updates status bits in user programs.
--- tools/testing/selftests/user_events/Makefile | 2 +- .../testing/selftests/user_events/perf_test.c | 168 ++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/user_events/perf_test.c
tools/testing/selftests/user_events/Makefile +1 -1
@@ -2,7 +2,7 @@ CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include LDLIBS += -lrt -lpthread -lm -TEST_GEN_PROGS = ftrace_test dyn_test +TEST_GEN_PROGS = ftrace_test dyn_test perf_test TEST_FILES := settings
tools/testing/selftests/user_events/perf_test.c new file +168 -0
168 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v9 12/12 user_events: Add documentation file
In reply to Beau BelgraveAdd a documentation file about user_events with example code, etc. explaining how it may be used.
--- Documentation/trace/index.rst | 1 + Documentation/trace/user_events.rst | 216 ++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+) create mode 100644 Documentation/trace/user_events.rst
Documentation/trace/index.rst +1 -0
@@ -30,3 +30,4 @@ Linux Tracing Technologies stm sys-t coresight/index + user_events
Documentation/trace/user_events.rst new file +216 -0
216 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v9 6/12 user_events: Validate user payloads for size and null termination
In reply to Beau BelgraveAdd validation to ensure data is at or greater than the min size for the fields of the event. If a dynamic array is used and is a type of char, ensure null termination of the array exists.
--- kernel/trace/trace_events_user.c | 147 ++++++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 14 deletions(-)
kernel/trace/trace_events_user.c +133 -14
@@ -64,9 +64,11 @@ struct user_event {struct dyn_event devent;struct hlist_node node;struct list_head fields; +struct list_head validators;atomic_t refcnt;int index;int flags; +int min_size; };/*
@@ -81,8 +83,17 @@ struct user_event_refs {struct user_event *events[]; }; +#define VALIDATOR_ENSURE_NULL (1 << 0) +#define VALIDATOR_REL (1 << 1) + +struct user_event_validator { +struct list_head link; +int offset; +int flags; +}; +typedef void (*user_event_func_t ) (struct user_event *user,struct iov_iter *i, -void *tpdata); +void *tpdata,bool *faulted);static int user_event_parse (char *name,char *args,char *flags,struct user_event **newuser);
@@ -215,6 +226,17 @@ static int user_field_size(const char *type)return -EINVAL ; } +static void user_event_destroy_validators (struct user_event *user) +{ +struct user_event_validator *validator, *next; +struct list_head *head = &user->validators; + +list_for_each_entry_safe (validator, next, head, link) { +list_del (&validator->link); +kfree (validator); + } +} +static void user_event_destroy_fields (struct user_event *user) {struct ftrace_event_field *field, *next;
@@ -230,13 +252,43 @@ static int user_event_add_field(struct user_event *user, const char *type, const char *name, int offset, int size, int is_signed, int filter_type) { + struct user_event_validator *validator;struct ftrace_event_field *field; +int validator_flags =0 ; field =kmalloc (sizeof (*field),GFP_KERNEL );if (!field)return -ENOMEM ; +if (str_has_prefix (type,"__data_loc " )) + goto add_validator; + +if (str_has_prefix (type,"__rel_loc " )) { + validator_flags |=VALIDATOR_REL ; + goto add_validator; + } + + goto add_field; + +add_validator: +if (strstr (type,"char" ) !=0 ) + validator_flags |=VALIDATOR_ENSURE_NULL ; + + validator =kmalloc (sizeof (*validator),GFP_KERNEL ); + +if (!validator) { +kfree (field); +return -ENOMEM ; + } + + validator->flags = validator_flags; + validator->offset = offset; + +/* Want sequential access when validating */ +list_add_tail (&validator->link, &user->validators); + +add_field: field->type = type; field->name = name; field->offset = offset;
@@ -246,6 +298,12 @@ static int user_event_add_field(struct user_event *user, const char *type,list_add (&field->link, &user->fields); +/* +* Min size from user writes that are required, this does not include +* the size of trace_entry (common fields). +*/ + user->min_size = (offset + size) -sizeof (struct trace_entry ); +return 0 ; }
@@ -517,6 +575,7 @@ static int destroy_user_event(struct user_event *user)clear_bit (user->index, page_bitmap);hash_del (&user->node); +user_event_destroy_validators (user);kfree (user->call.print_fmt);kfree (EVENT_NAME (user));kfree (user);
@@ -538,15 +597,49 @@ static struct user_event *find_user_event(char *name, u32 *outkey)return NULL ; } +static int user_event_validate (struct user_event *user,void *data,int len) +{ +struct list_head *head = &user->validators; +struct user_event_validator *validator; +void *pos, *end = data + len; +u32 loc, offset, size; + +list_for_each_entry (validator, head, link) { + pos = data + validator->offset; + +/* Already done min_size check, no bounds check here */ + loc = *(u32 *)pos; + offset = loc &0xffff ; + size = loc >>16 ; + +if (likely (validator->flags &VALIDATOR_REL )) + pos += offset +sizeof (loc); +else + pos = data + offset; + + pos += size; + +if (unlikely (pos > end)) +return -EFAULT ; + +if (likely (validator->flags &VALIDATOR_ENSURE_NULL )) +if (unlikely (*(char *)(pos -1 ) !='\0' )) +return -EFAULT ; + } + +return 0 ; +} +/* * Writes the user supplied payload out to a trace file. */ static void user_event_ftrace (struct user_event *user,struct iov_iter *i, -void *tpdata) +void *tpdata,bool *faulted) {struct trace_event_file *file;struct trace_entry *entry;struct trace_event_buffer event_buffer; +size_t size =sizeof (*entry) + i->count; file = (struct trace_event_file *)tpdata;
@@ -556,17 +649,25 @@ static void user_event_ftrace(struct user_event *user, struct iov_iter *i,return ;/* Allocates and fills trace_entry, + 1 of this is data payload */ - entry =trace_event_buffer_reserve (&event_buffer, file, -sizeof (*entry) + i->count); + entry =trace_event_buffer_reserve (&event_buffer, file, size);if (unlikely (!entry))return ;if (unlikely (!copy_nofault (entry +1 , i->count, i))) -__trace_event_discard_commit (event_buffer.buffer, - event_buffer.event); -else -trace_event_buffer_commit (&event_buffer); + goto discard; + +if (!list_empty (&user->validators) && +unlikely (user_event_validate (user, entry, size))) + goto discard; + +trace_event_buffer_commit (&event_buffer); + +return ; +discard: + *faulted = true; +__trace_event_discard_commit (event_buffer.buffer, + event_buffer.event); }#ifdef CONFIG_PERF_EVENTS
@@ -621,7 +722,7 @@ static void user_event_bpf(struct user_event *user, struct iov_iter *i)* Writes the user supplied payload out to perf ring buffer or eBPF program. */ static void user_event_perf (struct user_event *user,struct iov_iter *i, -void *tpdata) +void *tpdata,bool *faulted) {struct hlist_head *perf_head;
@@ -644,14 +745,21 @@ static void user_event_perf(struct user_event *user, struct iov_iter *i,perf_fetch_caller_regs (regs); -if (unlikely (!copy_nofault (perf_entry +1 , i->count, i))) { -perf_swevent_put_recursion_context (context); -return ; - } +if (unlikely (!copy_nofault (perf_entry +1 , i->count, i))) + goto discard; + +if (!list_empty (&user->validators) && +unlikely (user_event_validate (user, perf_entry, size))) + goto discard;perf_trace_buf_submit (perf_entry, size, context, user->call.event.type,1 , regs, perf_head,NULL ); + +return ; +discard: + *faulted = true; + perf_swevent_put_recursion_context(context); } }#endif
@@ -966,6 +1074,7 @@ static int user_event_parse(char *name, char *args, char *flags,INIT_LIST_HEAD (&user->class.fields);INIT_LIST_HEAD (&user->fields); +INIT_LIST_HEAD (&user->validators); user->tracepoint.name = name;
@@ -1014,6 +1123,7 @@ static int user_event_parse(char *name, char *args, char *flags,return 0 ; put_user:user_event_destroy_fields (user); +user_event_destroy_validators (user);kfree (user);return ret; }
@@ -1071,6 +1181,9 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)if (unlikely (user ==NULL ))return -ENOENT ; +if (unlikely (i->count < user->min_size)) +return -EINVAL ; + tp = &user->tracepoint;/*
@@ -1082,10 +1195,13 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)user_event_func_t probe_func;struct iov_iter copy;void *tpdata; +bool faulted;if (unlikely (fault_in_iov_iter_readable (i, i->count)))return -EFAULT ; + faulted = false; +rcu_read_lock_sched (); probe_func_ptr =rcu_dereference_sched (tp->funcs);
@@ -1095,11 +1211,14 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i) copy = *i; probe_func = probe_func_ptr->func; tpdata = probe_func_ptr->data; -probe_func (user, ©, tpdata); +probe_func (user, ©, tpdata, &faulted); }while ((++probe_func_ptr)->func); }rcu_read_unlock_sched (); + +if (unlikely (faulted)) +return -EFAULT ; }return ret;
Signature · 1 line
2.17.1
PATCH v9 10/12 user_events: Add self-test for validator boundaries
In reply to Beau BelgraveTests to ensure validator boundary cases are working correctly within close and far bounds. Ensures __data_loc and __rel_loc strings are null terminated and within range. Ensures min size checks work as expected.
--- .../selftests/user_events/ftrace_test.c | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+)
tools/testing/selftests/user_events/ftrace_test.c +65 -0
@@ -309,6 +309,71 @@ TEST_F(user, write_fault) {ASSERT_EQ (0 ,munmap (anon, l)); } +TEST_F(user, write_validator) { +struct user_reg reg = {0 }; +struct iovec io[3 ]; +int loc, bytes; +char data[8 ]; +int before =0 , after =0 ; + + reg.size =sizeof (reg); + reg.name_args = (__u64 )"__test_event __rel_loc char[] data" ; + +/* Register should work */ +ASSERT_EQ (0 ,ioctl (self->data_fd,DIAG_IOCSREG , ®)); +ASSERT_EQ (0 , reg.write_index); +ASSERT_NE (0 , reg.status_index); + + io[0 ].iov_base = ®.write_index; + io[0 ].iov_len =sizeof (reg.write_index); + io[1 ].iov_base = &loc; + io[1 ].iov_len =sizeof (loc); + io[2 ].iov_base = data; + bytes =snprintf (data,sizeof (data),"Test" ) +1 ; + io[2 ].iov_len = bytes; + +/* Undersized write should fail */ +ASSERT_EQ (-1 ,writev (self->data_fd, (const struct iovec *)io,1 )); +ASSERT_EQ (EINVAL , errno); + +/* Enable event */ + self->enable_fd =open (enable_file,O_RDWR ); +ASSERT_NE (-1 ,write (self->enable_fd,"1" ,sizeof ("1" ))) + +/* Full in-bounds write should work */ + before = trace_bytes(); + loc =DYN_LOC (0 , bytes); +ASSERT_NE (-1 ,writev (self->data_fd, (const struct iovec *)io,3 )); + after =trace_bytes (); +ASSERT_GT (after, before); + +/* Out of bounds write should fault (offset way out) */ + loc =DYN_LOC (1024 , bytes); +ASSERT_EQ (-1 ,writev (self->data_fd, (const struct iovec *)io,3 )); +ASSERT_EQ (EFAULT , errno); + +/* Out of bounds write should fault (offset 1 byte out) */ + loc =DYN_LOC (1 , bytes); +ASSERT_EQ (-1 ,writev (self->data_fd, (const struct iovec *)io,3 )); +ASSERT_EQ (EFAULT , errno); + +/* Out of bounds write should fault (size way out) */ + loc =DYN_LOC (0 , bytes +1024 ); +ASSERT_EQ (-1 ,writev (self->data_fd, (const struct iovec *)io,3 )); +ASSERT_EQ (EFAULT , errno); + +/* Out of bounds write should fault (size 1 byte out) */ + loc =DYN_LOC (0 , bytes +1 ); +ASSERT_EQ (-1 ,writev (self->data_fd, (const struct iovec *)io,3 )); +ASSERT_EQ (EFAULT , errno); + +/* Non-Null should fault */ +memset (data,'A' ,sizeof (data)); + loc =DYN_LOC (0 , bytes); +ASSERT_EQ (-1 ,writev (self->data_fd, (const struct iovec *)io,3 )); +ASSERT_EQ (EFAULT , errno); +} + TEST_F(user, print_fmt) {int ret;
Signature · 1 line
2.17.1
PATCH v9 11/12 user_events: Add sample code for typical usage
In reply to Beau BelgraveAdd sample code for user_events typical usage to show how to register and monitor status, as well as to write out data.
--- samples/user_events/Makefile | 5 ++ samples/user_events/example.c | 91 +++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 samples/user_events/Makefile create mode 100644 samples/user_events/example.c
samples/user_events/Makefile new file +5 -0
@@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +CFLAGS += -Wl,-no-as-needed -Wall -I../../usr/include + +example: example.o +example.o: example.c
samples/user_events/example.c new file +91 -0
91 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v9 5/12 user_events: Optimize writing events by only copying data once
In reply to Beau BelgravePass iterator through to probes to allow copying data directly to the probe buffers instead of taking multiple copies. Enables eBPF user and raw iterator types out to programs for no-copy scenarios.
--- kernel/trace/trace_events_user.c | 115 +++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 30 deletions(-)
kernel/trace/trace_events_user.c +85 -30
@@ -41,6 +41,9 @@#define MAX_FIELD_ARRAY_SIZE 1024#define MAX_FIELD_ARG_NAME 256 +#define MAX_BPF_COPY_SIZE PAGE_SIZE +#define MAX_STACK_BPF_DATA 512 +static char *register_page_data;static DEFINE_MUTEX (reg_mutex);
@@ -78,8 +81,7 @@ struct user_event_refs {struct user_event *events[]; }; -typedef void (*user_event_func_t ) (struct user_event *user, -void *data,u32 datalen, +typedef void (*user_event_func_t ) (struct user_event *user,struct iov_iter *i,void *tpdata); static int user_event_parse(char *name, char *args, char *flags,
@@ -90,6 +92,20 @@ static u32 user_event_key(char *name)return jhash (name,strlen (name),0 ); } +static __always_inline __must_check +size_t copy_nofault(void *addr, size_t bytes, struct iov_iter *i) +{ +size_t ret; + +pagefault_disable (); + + ret =copy_from_iter_nocache (addr, bytes, i); + +pagefault_enable (); + +return ret; +} +static struct list_head *user_event_get_fields (struct trace_event_call *call) {struct user_event *user = (struct user_event *)call->data;
@@ -525,7 +541,7 @@ static struct user_event *find_user_event(char *name, u32 *outkey)/* * Writes the user supplied payload out to a trace file. */ -static void user_event_ftrace (struct user_event *user,void *data,u32 datalen, +static void user_event_ftrace (struct user_event *user,struct iov_iter *i,void *tpdata) {struct trace_event_file *file;
@@ -541,41 +557,83 @@ static void user_event_ftrace(struct user_event *user, void *data, u32 datalen,/* Allocates and fills trace_entry, + 1 of this is data payload */ entry =trace_event_buffer_reserve (&event_buffer, file, -sizeof (*entry) + datalen); +sizeof (*entry) + i->count);if (unlikely (!entry))return ; -memcpy (entry +1 , data, datalen); - -trace_event_buffer_commit (&event_buffer); +if (unlikely (!copy_nofault (entry +1 , i->count, i))) +__trace_event_discard_commit (event_buffer.buffer, + event_buffer.event); +else +trace_event_buffer_commit (&event_buffer); }#ifdef CONFIG_PERF_EVENTS +static void user_event_bpf (struct user_event *user,struct iov_iter *i) +{ +struct user_bpf_context context; +struct user_bpf_iter bpf_i; +char fast_data[MAX_STACK_BPF_DATA ]; +void *temp =NULL ; + +if ((user->flags &FLAG_BPF_ITER ) &&iter_is_iovec (i)) { +/* Raw iterator */ + context.data_type =USER_BPF_DATA_ITER ; + context.data_len = i->count; + context.iter = &bpf_i; + + bpf_i.iov_offset = i->iov_offset; + bpf_i.iov = i->iov; + bpf_i.nr_segs = i->nr_segs; + }else if (i->nr_segs ==1 &&iter_is_iovec (i)) { +/* Single buffer from user */ + context.data_type =USER_BPF_DATA_USER ; + context.data_len = i->count; + context.udata = i->iov->iov_base + i->iov_offset; + }else { +/* Multi buffer from user */ +struct iov_iter copy = *i; +size_t copy_size =min_t (size_t, i->count,MAX_BPF_COPY_SIZE ); + + context.data_type =USER_BPF_DATA_KERNEL ; + context.kdata = fast_data; + +if (unlikely (copy_size >sizeof (fast_data))) { + temp =kmalloc (copy_size,GFP_NOWAIT ); + +if (temp) + context.kdata = temp; +else + copy_size =sizeof (fast_data); + } + + context.data_len =copy_nofault (context.kdata, + copy_size, ©); + } + +trace_call_bpf (&user->call, &context); + +kfree (temp); +} +/* * Writes the user supplied payload out to perf ring buffer or eBPF program. */ -static void user_event_perf (struct user_event *user,void *data,u32 datalen, +static void user_event_perf (struct user_event *user,struct iov_iter *i,void *tpdata) {struct hlist_head *perf_head; -if (bpf_prog_array_valid (&user->call)) { -struct user_bpf_context context = {0 }; - - context.data_len = datalen; - context.data_type =USER_BPF_DATA_KERNEL ; - context.kdata = data; - -trace_call_bpf (&user->call, &context); - } +if (bpf_prog_array_valid (&user->call)) +user_event_bpf (user, i); perf_head =this_cpu_ptr (user->call.perf_events);if (perf_head && !hlist_empty (perf_head)) {struct trace_entry *perf_entry;struct pt_regs *regs; -size_t size =sizeof (*perf_entry) + datalen; +size_t size =sizeof (*perf_entry) + i->count;int context; perf_entry = perf_trace_buf_alloc(ALIGN(size, 8),
@@ -586,7 +644,10 @@ static void user_event_perf(struct user_event *user, void *data, u32 datalen,perf_fetch_caller_regs (regs); -memcpy (perf_entry +1 , data, datalen); +if (unlikely (!copy_nofault (perf_entry +1 , i->count, i))) { +perf_swevent_put_recursion_context (context); +return ; + } perf_trace_buf_submit(perf_entry, size, context, user->call.event.type, 1, regs,
@@ -1019,16 +1080,11 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)if (likely (atomic_read (&tp->key.enabled) >0 )) {struct tracepoint_func *probe_func_ptr;user_event_func_t probe_func; +struct iov_iter copy;void *tpdata; -void *kdata; -u32 datalen; - kdata =kmalloc (i->count,GFP_KERNEL ); - -if (unlikely (!kdata)) -return -ENOMEM ; - - datalen =copy_from_iter (kdata, i->count, i); +if (unlikely (fault_in_iov_iter_readable (i, i->count))) +return -EFAULT ;rcu_read_lock_sched ();
@@ -1036,15 +1092,14 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)if (probe_func_ptr) {do { + copy = *i; probe_func = probe_func_ptr->func; tpdata = probe_func_ptr->data; -probe_func (user, kdata, datalen, tpdata); +probe_func (user, ©, tpdata); }while ((++probe_func_ptr)->func); }rcu_read_unlock_sched (); - -kfree (kdata); }return ret;
Signature · 1 line
2.17.1
kernel test robot
build botRe: [PATCH v9 01/12] user_events: Add minimal support for trace_event into ftrace
In reply to Beau BelgraveThank you for the patch! Perhaps something to improve:
Hi Beau,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 85c62c8c3749eec02ba81217bdcac26867dc262e]
url: https://github.com/0day-ci/linux/commits/Beau-Belgrave/user_events-Enable-user-processes-to-create-and-write-to-trace-events/20220112-014042 base: 85c62c8c3749eec02ba81217bdcac26867dc262e config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20220114/202201142322.DpkhCCS2-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 82c8aca93488730ce8f66101e0f3538f14b551dd)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a5a2d34cbaeb90d281d8c13c753f4aaaa447ec11
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Beau-Belgrave/user_events-Enable-user-processes-to-create-and-write-to-trace-events/20220112-014042 git checkout a5a2d34cbaeb90d281d8c13c753f4aaaa447ec11
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/trace/ sound/soc/sof/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> kernel/trace/trace_events_user.c:1027:14: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return NULL + (*pos == 0);
~~~~ ^1 warning generated.
vim +1027 kernel/trace/trace_events_user.c
1024
1025 static void *user_seq_start(struct seq_file *m, loff_t *pos)
1026 {
> 1027 return NULL + (*pos == 0);
1028 }
1029
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Re: [PATCH v9 01/12] user_events: Add minimal support for trace_event into ftrace
Quoted 32 lines · kernel test robot
Hi Beau,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on
85c62c8c3749eec02ba81217bdcac26867dc262e]url: https://github.com/0day-ci/linux/commits/Beau-Belgrave/user_events-Enable-user-processes-to-create-and-write-to-trace-events/20220112-014042 base: 85c62c8c3749eec02ba81217bdcac26867dc262e config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20220114/202201142322.DpkhCCS2-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 82c8aca93488730ce8f66101e0f3538f14b551dd)reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a5a2d34cbaeb90d281d8c13c753f4aaaa447ec11
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Beau-Belgrave/user_events-Enable-user-processes-to-create-and-write-to-trace-events/20220112-014042 git checkouta5a2d34cbaeb90d281d8c13c753f4aaaa447ec11
# save the config file to linux build tree
mkdirbuild_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bashkernel/trace/sound/soc/sof/If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>All warnings (new ones prefixed by >>):
Quoted 1 line
Quoted 1 line
kernel/trace/trace_events_user.c:1027:14: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]return NULL + (*pos == 0); ~~~~ ^1 warning generated.
Quoted 9 lines
vim +1027
kernel/trace/trace_events_user.c1024
1025 static void *user_seq_start(structseq_file*m,loff_t*pos)
1026 {Quoted 1 line
1027 return NULL + (*pos == 0);
1028 }
1029
Arnd, it looks like [1] never got picked up? That would help solve this, as this function is the same as single_start(). Should it be resent or pinged?
[1]: https://lore.kernel.org/r/20201028151202.3074398-1-arnd(a)kernel.org/
Cheers,
Nathan
Masami Hiramatsu
maintainerRe: [PATCH v9 01/12] user_events: Add minimal support for trace_event into ftrace
In reply to Beau BelgraveHi Beau,
Quoted 14 lines · On Tue, 11 Jan 2022 09:25:51 -0800 Beau Belgrave <[email protected]>…
kernel/trace/trace_events_user.c new file +14
-0+static int user_event_show (struct seq_file *m,struct dyn_event *ev) +{ + struct user_event *user = container_of(ev, struct user_event, devent); +struct ftrace_event_field *field, *next; +struct list_head *head; +int depth =0 ; + +seq_printf (m,"%s%s" ,USER_EVENTS_PREFIX ,EVENT_NAME (user)); + + head =trace_get_fields (&user->call); + +list_for_each_entry_safe_reverse (field, next, head, link) { +if (depth ==0 ) +seq_puts (m," " );
kernel/trace/trace_events_user.c line 570
Quoted 10 lines
kernel/trace/trace_events_user.c new file +10
-0+else +seq_puts (m,"; " ); +seq_printf (m,"%s %s" , field->type, field->name); + depth++; + } + +seq_puts (m,"\n" ); + +return 0 ; +}
Let me confirm just one point. Your syntax supports
[__data_loc|__rel_loc] [unsigned] TYPE[\[LEN\]] NAME
or
struct TYPE NAME SIZE
for the fields, right? In that case, above seq_printf() seems not enough.
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>
Re: [PATCH v9 01/12] user_events: Add minimal support for trace_event into ftrace
Quoted 32 lines · Masami Hiramatsu
Hi Beau,
Quoted 14 lines · On Tue, 11 Jan 2022 09:25:51 -0800 Beau Belgrave <[email protected]>…
kernel/trace/trace_events_user.c new file +14
-0+static int user_event_show (struct seq_file *m,struct dyn_event *ev) +{ + struct user_event *user = container_of(ev, struct user_event, devent); +struct ftrace_event_field *field, *next; +struct list_head *head; +int depth =0 ; + +seq_printf (m,"%s%s" ,USER_EVENTS_PREFIX ,EVENT_NAME (user)); + + head =trace_get_fields (&user->call); + +list_for_each_entry_safe_reverse (field, next, head, link) { +if (depth ==0 ) +seq_puts (m," " );kernel/trace/trace_events_user.c line 570
Quoted 10 lines
kernel/trace/trace_events_user.c new file +10
-0+else +seq_puts (m,"; " ); +seq_printf (m,"%s %s" , field->type, field->name); + depth++; + } + +seq_puts (m,"\n" ); + +return 0 ; +}Let me confirm just one point. Your syntax supports
Quoted 7 lines
[__data_loc|__rel_loc] [unsigned] TYPE[\[LEN\]] NAME
or
struct TYPE NAME SIZE
for the fields, right? In that case, above
seq_printf()seems not enough.
Yep, I see.
The non-struct cases work as expected from my testing:
echo 'u:test unsigned char msg[20]' > dynamic_events cat dynamic_events
u:test unsigned char msg[20]
In the struct case you are right, it's missing the size. Good catch! I'll fix this up!
Was there another case you had in mind that I might have missed beyond the struct case?
I also would like, since I'm re-spinning, to fix a warning the intel bot found related to the same code pulled from single_open, etc.
See https://lore.kernel.org/llvm/YeGk0nIH9x91k01I@archlinux-ax161/
Quoted 6 lines
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>
Thanks,
-Beau
Masami Hiramatsu
maintainerRe: [PATCH v9 01/12] user_events: Add minimal support for trace_event into ftrace
Quoted 48 lines · On Mon, 17 Jan 2022 10:09:14 -0800 Beau Belgrave <[email protected]>…
Quoted 32 lines · Masami Hiramatsu
Hi Beau,
Quoted 14 lines · On Tue, 11 Jan 2022 09:25:51 -0800 Beau Belgrave <[email protected]>…
kernel/trace/trace_events_user.c new file +14
-0+static int user_event_show (struct seq_file *m,struct dyn_event *ev) +{ + struct user_event *user = container_of(ev, struct user_event, devent); +struct ftrace_event_field *field, *next; +struct list_head *head; +int depth =0 ; + +seq_printf (m,"%s%s" ,USER_EVENTS_PREFIX ,EVENT_NAME (user)); + + head =trace_get_fields (&user->call); + +list_for_each_entry_safe_reverse (field, next, head, link) { +if (depth ==0 ) +seq_puts (m," " );kernel/trace/trace_events_user.c line 570
Quoted 10 lines
kernel/trace/trace_events_user.c new file +10
-0+else +seq_puts (m,"; " ); +seq_printf (m,"%s %s" , field->type, field->name); + depth++; + } + +seq_puts (m,"\n" ); + +return 0 ; +}Let me confirm just one point. Your syntax supports
Quoted 7 lines
[__data_loc|__rel_loc] [unsigned] TYPE[\[LEN\]] NAME
or
struct TYPE NAME SIZE
for the fields, right? In that case, above
seq_printf()seems not enough.Yep, I see.
The non-struct cases work as expected from my testing:
echo 'u:test unsigned char msg[20]' >dynamic_eventscatdynamic_events
u:test unsigned char msg[20]
Quoted 5 lines
In the struct case you are right, it's missing the size. Good catch! I'll fix this up!
Was there another case you had in mind that I might have missed beyond the struct case?
No, that's the only one which I found :)
Quoted 5 lines
I also would like, since I'm re-spinning, to fix a warning the intel bot found related to the same code pulled from
single_open, etc.See https://lore.kernel.org/llvm/YeGk0nIH9x91k01I@archlinux-ax161/
OK, I'll then wait for your next version.
Thank you!
Quoted 10 lines
Quoted 7 lines
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>Thanks,
-Beau
Signature · 1 line
Masami Hiramatsu <[email protected]>