tracing: Last minute updates before sending to Linus
Subsystems (4)
Steven Rostedt
maintainercover tracing: Last minute updates before sending to Linus
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: d3c07fac565261101337b9535df072361297ea2d
Axel Rasmussen (1):
tracing: support "bool" type in synthetic trace events
Gaurav Kohli (1):
tracing: Fix race in trace_open and buffer resize call
Masami Hiramatsu (1):
tracing/boot: Add ftrace.instance.*.alloc_snapshot option
Qiujun Huang (1):
tracing: Fix some typos in comments
Steven Rostedt (VMware) (1):
tracing: Check return value of __create_val_fields() before using its result
Tom Zanussi (7):
tracing: Don't show dynamic string internals in synthetic event description
tracing: Move is_good_name() from trace_probe.h to trace.h
tracing: Check that the synthetic event and field names are legal
tracing: Add synthetic event error logging
selftests/ftrace: Change synthetic event name for inter-event-combined test
tracing: Handle synthetic event array field type checking correctly selftests/ftrace: Add test case for synthetic event syntax errors
----
kernel/trace/ring_buffer.c | 10 ++ kernel/trace/trace.c | 4 +- kernel/trace/trace.h | 21 +++- kernel/trace/trace_boot.c | 6 + kernel/trace/trace_events_hist.c | 2 +- kernel/trace/trace_events_synth.c | 127 ++++++++++++++++++++- kernel/trace/trace_probe.h | 13 --- .../trigger-inter-event-combined-hist.tc | 8 +- .../trigger-synthetic_event_syntax_errors.tc | 19 +++ 9 files changed, 180 insertions(+), 30 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
Steven Rostedt
maintainerPATCH 1/12 tracing: Check return value of __create_val_fields() before using its result
From: "Steven Rostedt (VMware)" <[email protected]>
After having a typo for writing a histogram trigger.
Wrote:
echo 'hist:key=pid:ts=common_timestamp.usec' > events/sched/sched_waking/trigger
Instead of:
echo 'hist:key=pid:ts=common_timestamp.usecs' > events/sched/sched_waking/trigger
and the following crash happened:
BUG: kernel NULL pointer dereference, address: 0000000000000008 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP PTI CPU: 4 PID: 1641 Comm: sh Not tainted 5.9.0-rc5-test+ #549 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016 RIP: 0010:event_hist_trigger_func+0x70b/0x1ee0 Code: 24 08 89 d5 49 89 cc e9 8c 00 00 00 4c 89 f2 41 b9 00 10 00 00 4c 89 e1 44 89 ee 4c 89 ff e8 dc d3 ff ff 45 89 ea 4b 8b 14 d7 <f6> 42 08 04 74 17 41 8b 8f c0 00 00 00 8d 71 01 41 89 b7 c0 00 00 RSP: 0018:ffff959213d53db0 EFLAGS: 00010202 RAX: ffffffffffffffea RBX: 0000000000000000 RCX: 0000000000084c04 RDX: 0000000000000000 RSI: df7326aefebd174c RDI: 0000000000031080 RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000000000001 R10: 0000000000000001 R11: 0000000000000046 R12: ffff959211dcf690 R13: 0000000000000001 R14: ffff95925a36e370 R15: ffff959251c89800 FS: 00007fb9ea934740(0000) GS:ffff95925ab00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000008 CR3: 00000000c976c005 CR4: 00000000001706e0 Call Trace: ? trigger_process_regex+0x78/0x110 trigger_process_regex+0xc5/0x110 event_trigger_write+0x71/0xd0 vfs_write+0xca/0x210 ksys_write+0x70/0xf0 do_syscall_64+0x33/0x40 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7fb9eaa29487 Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
This was caused by accessing the hlist_data fields after the call to __create_val_fields() without checking if the creation succeed.
--- kernel/trace/trace_events_hist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace_events_hist.c +1 -1
@@ -3687,7 +3687,7 @@ static int create_var_field(struct hist_trigger_data *hist_data, ret =__create_val_field (hist_data, val_idx, file, var_name, expr_str, flags); -if (hist_data->fields[val_idx]->flags &HIST_FIELD_FL_STRING ) +if (!ret && hist_data->fields[val_idx]->flags &HIST_FIELD_FL_STRING ) hist_data->fields[val_idx]->var_str_idx = hist_data->n_var_str++;return ret;
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 2/12 tracing: Fix race in trace_open and buffer resize call
In reply to Steven RostedtFrom: Gaurav Kohli <[email protected]>
Below race can come, if trace_open and resize of
cpu buffer is running parallely on different cpus
CPUX CPUY
ring_buffer_resize atomic_read(&buffer->resize_disabled)
tracing_opentracing_reset_online_cpus ring_buffer_reset_cpurb_reset_cpu
rb_update_pages remove/insert pages
resetting pointer
This race can cause data abort or some times infinte loop in rb_remove_pages and rb_insert_pages while checking pages for sanity.
Take buffer lock to fix this.
--- kernel/trace/ring_buffer.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
kernel/trace/ring_buffer.c +10 -0
@@ -4866,6 +4866,9 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer, int cpu)if (!cpumask_test_cpu (cpu, buffer->cpumask))return ; +/* prevent another thread from changing buffer sizes */ +mutex_lock (&buffer->mutex); +atomic_inc (&cpu_buffer->resize_disabled);atomic_inc (&cpu_buffer->record_disabled);
@@ -4876,6 +4879,8 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer, int cpu)atomic_dec (&cpu_buffer->record_disabled);atomic_dec (&cpu_buffer->resize_disabled); + +mutex_unlock (&buffer->mutex); }EXPORT_SYMBOL_GPL (ring_buffer_reset_cpu);
@@ -4889,6 +4894,9 @@ void ring_buffer_reset_online_cpus(struct trace_buffer *buffer)struct ring_buffer_per_cpu *cpu_buffer;int cpu; +/* prevent another thread from changing buffer sizes */ +mutex_lock (&buffer->mutex); +for_each_online_buffer_cpu (buffer, cpu) { cpu_buffer = buffer->buffers[cpu];
@@ -4907,6 +4915,8 @@ void ring_buffer_reset_online_cpus(struct trace_buffer *buffer)atomic_dec (&cpu_buffer->record_disabled);atomic_dec (&cpu_buffer->resize_disabled); } + +mutex_unlock (&buffer->mutex); }/**
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 3/12 tracing/boot: Add ftrace.instance.*.alloc_snapshot option
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Add ftrace.instance.*.alloc_snapshot option.
This option has been described in Documentation/trace/boottime-trace.rst but not implemented yet.
ftrace.[instance.INSTANCE.]alloc_snapshot
Allocate snapshot buffer.
The difference from kernel.alloc_snapshot is that the kernel.alloc_snapshot will allocate the buffer only for the main instance, but this can allocate buffer for any new instances.
--- kernel/trace/trace_boot.c | 6 ++++++ 1 file changed, 6 insertions(+)
kernel/trace/trace_boot.c +6 -0
@@ -284,6 +284,12 @@ trace_boot_enable_tracer(struct trace_array *tr, struct xbc_node *node)if (tracing_set_tracer (tr, p) <0 )pr_err ("Failed to set given tracer: %s\n" , p); } + +/* Since tracer can free snapshot buffer, allocate snapshot here.*/ +if (xbc_node_find_value (node,"alloc_snapshot" ,NULL )) { +if (tracing_alloc_snapshot_instance (tr) <0 ) +pr_err ("Failed to allocate snapshot buffer\n" ); + } } static void __init
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 4/12 tracing: Fix some typos in comments
In reply to Steven RostedtFrom: Qiujun Huang <[email protected]>
s/wihin/within/
s/retrieven/retrieved/
s/suppport/support/
s/wil/will/
s/accidently/accidentally/
s/if the if the/if the/
--- kernel/trace/trace.c | 4 ++-- kernel/trace/trace.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-)
kernel/trace/trace.c +2 -2
@@ -9465,7 +9465,7 @@ __init static int tracer_alloc_buffers(void) }/* -* Make sure we don't accidently add more trace options +* Make sure we don't accidentally add more trace options * than we have bits for. */ BUILD_BUG_ON (TRACE_ITER_LAST_BIT >TRACE_FLAGS_MAX_SIZE );
@@ -9494,7 +9494,7 @@ __init static int tracer_alloc_buffers(void)/* * The prepare callbacks allocates some memory for the ring buffer. We -* don't free the buffer if the if the CPU goes down. If we were to free +* don't free the buffer if the CPU goes down. If we were to free * the buffer, then the user would lose any trace that was in the * buffer. The memory will be removed once the "instance" is removed. */
kernel/trace/trace.h +4 -4
@@ -246,7 +246,7 @@ typedef bool (*cond_update_fn_t)(struct trace_array *tr, void *cond_data);* tracing_snapshot_cond(tr, cond_data), the cond_data passed in is * passed in turn to the cond_snapshot.update() function. That data * can be compared by the update() implementation with the cond_data -* contained wihin the struct cond_snapshot instance associated with +* contained within the struct cond_snapshot instance associated with * the trace_array. Because the tr->max_lock is held throughout the * update() call, the update() function can directly retrieve the * cond_snapshot and cond_data associated with the per-instance
@@ -271,7 +271,7 @@ typedef bool (*cond_update_fn_t)(struct trace_array *tr, void *cond_data);* take the snapshot, by returning 'true' if so, 'false' if no * snapshot should be taken. Because the max_lock is held for * the duration of update(), the implementation is safe to -* directly retrieven and save any implementation data it needs +* directly retrieved and save any implementation data it needs * to in association with the snapshot. */ struct cond_snapshot {
@@ -573,7 +573,7 @@ struct tracer {* The function callback, which can use the FTRACE bits to * check for recursion. * -* Now if the arch does not suppport a feature, and it calls +* Now if the arch does not support a feature, and it calls * the global list function which calls the ftrace callback * all three of these steps will do a recursion protection. * There's no reason to do one if the previous caller already
@@ -1479,7 +1479,7 @@ __trace_event_discard_commit(struct trace_buffer *buffer,/* * Helper function for event_trigger_unlock_commit{_regs}(). * If there are event triggers attached to this event that requires -* filtering against its fields, then they wil be called as the +* filtering against its fields, then they will be called as the * entry already holds the field information of the current event. * * It also checks if the event should be discarded or not.
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 5/12 tracing: Dont show dynamic string internals in synthetic event description
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
For synthetic event dynamic fields, the type contains "__data_loc", which is basically an internal part of the type which is only meant to be displayed in the format, not in the event description itself, which is confusing to users since they can't use __data_loc on the command-line to define an event field, which printing it would lead them to believe.
So filter it out from the description, while leaving it in the type.
--- kernel/trace/trace_events_synth.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
kernel/trace/trace_events_synth.c +9 -1
@@ -1867,14 +1867,22 @@ static int __synth_event_show(struct seq_file *m, struct synth_event *event) {struct synth_field *field;unsigned int i; +char *type, *t;seq_printf (m,"%s\t" , event->name);for (i =0 ; i < event->n_fields; i++) { field = event->fields[i]; + type = field->type; + t =strstr (type,"__data_loc" ); +if (t) {/* __data_loc belongs in format but not event desc */ + t +=sizeof ("__data_loc" ); + type = t; + } +/* parameter values */ -seq_printf (m,"%s %s%s" , field->type, field->name, +seq_printf (m,"%s %s%s" , type, field->name, i == event->n_fields -1 ?"" :"; " ); }
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 6/12 tracing: Move is_good_name() from trace_probe.h to trace.h
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
is_good_name() is useful for other trace infrastructure, such as synthetic events, so make it available via trace.h.
--- kernel/trace/trace.h | 13 +++++++++++++ kernel/trace/trace_probe.h | 13 ------------- 2 files changed, 13 insertions(+), 13 deletions(-)
kernel/trace/trace.h +13 -0
@@ -19,6 +19,7 @@#include <linux/glob.h> #include <linux/irq_work.h> #include <linux/workqueue.h> +#include <linux/ctype.h> #ifdef CONFIG_FTRACE_SYSCALLS #include <asm/unistd.h> /* For NR_SYSCALLS */
@@ -2090,4 +2091,16 @@ static __always_inline void trace_iterator_reset(struct trace_iterator *iter) iter->pos = -1; } +/* Check the name is good for event/group/fields */ +static inline bool is_good_name (const char *name) +{ +if (!isalpha (*name) && *name !='_' ) +return false; +while (*++name !='\0' ) { +if (!isalpha (*name) && !isdigit (*name) && *name !='_' ) +return false; + } +return true; +} +#endif /* _LINUX_KERNEL_TRACE_H */
kernel/trace/trace_probe.h +0 -13
@@ -16,7 +16,6 @@#include <linux/tracefs.h> #include <linux/types.h> #include <linux/string.h> -#include <linux/ctype.h> #include <linux/ptrace.h> #include <linux/perf_event.h> #include <linux/kprobes.h>
@@ -348,18 +347,6 @@ bool trace_probe_match_command_args(struct trace_probe *tp,#define trace_probe_for_each_link_rcu (pos, tp) \ list_for_each_entry_rcu(pos, &(tp)->event->files, list) -/* Check the name is good for event/group/fields */ -static inline bool is_good_name (const char *name) -{ -if (!isalpha (*name) && *name !='_' ) -return false; -while (*++name !='\0' ) { -if (!isalpha (*name) && !isdigit (*name) && *name !='_' ) -return false; - } -return true; -} -#define TPARG_FL_RETURN BIT(0)#define TPARG_FL_KERNEL BIT(1)#define TPARG_FL_FENTRY BIT(2)
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 7/12 tracing: Check that the synthetic event and field names are legal
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
Call the is_good_name() function used by probe events to make sure synthetic event and field names don't contain illegal characters and cause unexpected parsing of synthetic event commands.
--- kernel/trace/trace_events_synth.c | 9 +++++++++ 1 file changed, 9 insertions(+)
kernel/trace/trace_events_synth.c +9 -0
@@ -572,6 +572,10 @@ static struct synth_field *parse_synth_field(int argc, const char **argv, ret = -ENOMEM ; goto free; } +if (!is_good_name (field->name)) { + ret = -EINVAL ; + goto free; + }if (field_type[0 ] ==';' ) field_type++;
@@ -1112,6 +1116,11 @@ static int __create_synth_event(int argc, const char *name, const char **argv)mutex_lock (&event_mutex); +if (!is_good_name (name)) { + ret = -EINVAL ; + goto out; + } + event =find_synth_event (name);if (event) { ret = -EEXIST ;
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 8/12 tracing: Add synthetic event error logging
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
Add support for synthetic event error logging, which entails adding a logging function for it, a way to save the synthetic event command, and a set of specific synthetic event parse error strings and handling.
--- kernel/trace/trace_events_synth.c | 92 ++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-)
kernel/trace/trace_events_synth.c +90 -2
@@ -20,6 +20,48 @@#include "trace_synth.h" +#undef ERRORS +#define ERRORS \ + C(BAD_NAME, "Illegal name"), \ + C(CMD_INCOMPLETE, "Incomplete command"), \ + C(EVENT_EXISTS, "Event already exists"), \ + C(TOO_MANY_FIELDS, "Too many fields"), \ + C(INCOMPLETE_TYPE, "Incomplete type"), \ + C(INVALID_TYPE, "Invalid type"), \ + C(INVALID_FIELD, "Invalid field"), \ + C(CMD_TOO_LONG, "Command too long"), + +#undef C +#define C (a, b) SYNTH_ERR_##a + +enum {ERRORS }; + +#undef C +#define C (a, b) b + +static const char *err_text[] = {ERRORS }; + +static char last_cmd[MAX_FILTER_STR_VAL ]; + +static int errpos (const char *str) +{ +return err_pos (last_cmd, str); +} + +static void last_cmd_set (char *str) +{ +if (!str) +return ; + +strncpy (last_cmd, str,MAX_FILTER_STR_VAL -1 ); +} + +static void synth_err (u8 err_type,u8 err_pos) +{ +tracing_log_err (NULL ,"synthetic_events" , last_cmd, err_text, + err_type, err_pos); +} +static int create_synth_event (int argc,const char **argv);static int synth_event_show (struct seq_file *m,struct dyn_event *ev);static int synth_event_release (struct dyn_event *ev);
@@ -545,8 +587,10 @@ static struct synth_field *parse_synth_field(int argc, const char **argv, field_type++;if (!strcmp (field_type,"unsigned" )) { -if (argc <3 ) +if (argc <3 ) { +synth_err (SYNTH_ERR_INCOMPLETE_TYPE ,errpos (field_type));return ERR_PTR (-EINVAL ); + } prefix ="unsigned " ; field_type = argv[1 ]; field_name = argv[2 ];
@@ -573,6 +617,7 @@ static struct synth_field *parse_synth_field(int argc, const char **argv, goto free; }if (!is_good_name (field->name)) { +synth_err (SYNTH_ERR_BAD_NAME ,errpos (field_name)); ret = -EINVAL ; goto free; }
@@ -601,6 +646,7 @@ static struct synth_field *parse_synth_field(int argc, const char **argv, size =synth_field_size (field->type);if (size <0 ) { +synth_err (SYNTH_ERR_INVALID_TYPE ,errpos (field_type)); ret = -EINVAL ; goto free; }else if (size ==0 ) {
@@ -621,6 +667,7 @@ static struct synth_field *parse_synth_field(int argc, const char **argv, field->is_dynamic = true; size =sizeof (u64); }else { +synth_err (SYNTH_ERR_INVALID_TYPE ,errpos (field_type)); ret = -EINVAL ; goto free; }
@@ -1098,12 +1145,47 @@ int synth_event_gen_cmd_array_start(struct dynevent_cmd *cmd, const char *name, }EXPORT_SYMBOL_GPL (synth_event_gen_cmd_array_start); +static int save_cmdstr (int argc,const char *name,const char **argv) +{ +struct seq_buf s; +char *buf; +int i; + + buf =kzalloc (MAX_DYNEVENT_CMD_LEN ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + +seq_buf_init (&s, buf,MAX_DYNEVENT_CMD_LEN ); + +seq_buf_puts (&s, name); + +for (i =0 ; i < argc; i++) { +seq_buf_putc (&s,' ' ); +seq_buf_puts (&s, argv[i]); + } + +if (!seq_buf_buffer_left (&s)) { +synth_err (SYNTH_ERR_CMD_TOO_LONG ,0 ); +kfree (buf); +return -EINVAL ; + } + buf[s.len] =0 ; +last_cmd_set (buf); + +kfree (buf); +return 0 ; +} +static int __create_synth_event (int argc,const char *name,const char **argv) {struct synth_field *field, *fields[SYNTH_FIELDS_MAX ];struct synth_event *event =NULL ;int i, consumed =0 , n_fields =0 , ret =0 ; + ret =save_cmdstr (argc, name, argv); +if (ret) +return ret; +/* * Argument syntax: * - Add synthetic event: <event_name> field[;field] ...
@@ -1111,18 +1193,22 @@ static int __create_synth_event(int argc, const char *name, const char **argv)* where 'field' = type field_name */ -if (name[0 ] =='\0' || argc <1 ) +if (name[0 ] =='\0' || argc <1 ) { +synth_err (SYNTH_ERR_CMD_INCOMPLETE ,0 );return -EINVAL ; + }mutex_lock (&event_mutex);if (!is_good_name (name)) { +synth_err (SYNTH_ERR_BAD_NAME ,errpos (name)); ret = -EINVAL ; goto out; } event =find_synth_event (name);if (event) { +synth_err (SYNTH_ERR_EVENT_EXISTS ,errpos (name)); ret = -EEXIST ; goto out; }
@@ -1131,6 +1217,7 @@ static int __create_synth_event(int argc, const char *name, const char **argv)if (strcmp (argv[i],";" ) ==0 )continue ;if (n_fields ==SYNTH_FIELDS_MAX ) { +synth_err (SYNTH_ERR_TOO_MANY_FIELDS ,0 ); ret = -EINVAL ; goto err; }
@@ -1145,6 +1232,7 @@ static int __create_synth_event(int argc, const char *name, const char **argv) }if (i < argc &&strcmp (argv[i],";" ) !=0 ) { +synth_err (SYNTH_ERR_INVALID_FIELD ,errpos (argv[i])); ret = -EINVAL ; goto err; }
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 9/12 selftests/ftrace: Change synthetic event name for inter-event-combined test
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
This test uses waking+wakeup_latency as an event name, which doesn't make sense since it includes an operator. Illegal names are now detected by the synthetic event command parsing, which causes this test to fail. Change the name to 'waking_plus_wakeup_latency' to prevent this.
--- .../inter-event/trigger-inter-event-combined-hist.tc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-inter-event-combined-hist.tc +4 -4
@@ -25,12 +25,12 @@ echo 'wakeup_latency u64 lat pid_t pid' >> synthetic_events echo 'hist:keys=pid:ts1=common_timestamp.usecs if comm=="ping"' >> events/sched/sched_wakeup/trigger echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts1:onmatch(sched.sched_wakeup).wakeup_latency($wakeup_lat,next_pid) if next_comm=="ping"' > events/sched/sched_switch/trigger -echo 'waking+wakeup_latency u64 lat; pid_t pid' >> synthetic_events -echo 'hist:keys=pid,lat:sort=pid,lat:ww_lat=$waking_lat+$wakeup_lat:onmatch(synthetic.wakeup_latency).waking+wakeup_latency($ww_lat,pid)' >> events/synthetic/wakeup_latency/trigger -echo 'hist:keys=pid,lat:sort=pid,lat' >> events/synthetic/waking+wakeup_latency/trigger +echo 'waking_plus_wakeup_latency u64 lat; pid_t pid' >> synthetic_events +echo 'hist:keys=pid,lat:sort=pid,lat:ww_lat=$waking_lat+$wakeup_lat:onmatch(synthetic.wakeup_latency).waking_plus_wakeup_latency($ww_lat,pid)' >> events/synthetic/wakeup_latency/trigger +echo 'hist:keys=pid,lat:sort=pid,lat' >> events/synthetic/waking_plus_wakeup_latency/trigger ping $LOCALHOST -c 3 -if ! grep -q "pid:" events/synthetic/waking+wakeup_latency/hist; then +if ! grep -q "pid:" events/synthetic/waking_plus_wakeup_latency/hist; then fail "Failed to create combined histogram" fi
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 10/12 tracing: Handle synthetic event array field type checking correctly
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
Since synthetic event array types are derived from the field name, there may be a semicolon at the end of the type which should be stripped off.
If there are more characters following that, normal type string checking will result in an invalid type.
Without this patch, you can end up with an invalid field type string that gets displayed in both the synthetic event description and the event format:
Before:
# echo 'myevent char str[16]; int v' >> synthetic_events
# cat synthetic_events
myevent char[16]; str; int v name: myevent
ID: 1936
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:char str[16];; offset:8; size:16; signed:1; field:int v; offset:40; size:4; signed:1;
print fmt: "str=%s, v=%d", REC->str, REC->v
After:
# echo 'myevent char str[16]; int v' >> synthetic_events
# cat synthetic_events
myevent char[16] str; int v# cat events/synthetic/myevent/format name: myevent ID: 1936 format: field:unsigned short common_type; offset:0; size:2; signed:0; field:unsigned char common_flags; offset:2; size:1; signed:0; field:unsigned char common_preempt_count; offset:3; size:1; signed:0; field:int common_pid; offset:4; size:4; signed:1;
field:char str[16]; offset:8; size:16; signed:1; field:int v; offset:40; size:4; signed:1;
print fmt: "str=%s, v=%d", REC->str, REC->v
--- kernel/trace/trace_events_synth.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
kernel/trace/trace_events_synth.c +9 -3
@@ -174,7 +174,7 @@ static int synth_field_string_size(char *type) start +=sizeof ("char[" ) -1 ; end =strchr (type,']' ); -if (!end || end < start) +if (!end || end < start || type +strlen (type) > end +1 )return -EINVAL ; len = end - start;
@@ -625,8 +625,14 @@ static struct synth_field *parse_synth_field(int argc, const char **argv,if (field_type[0 ] ==';' ) field_type++; len =strlen (field_type) +1 ; -if (array) - len +=strlen (array); + +if (array) { +int l =strlen (array); + +if (l && array[l -1 ] ==';' ) + l--; + len += l; + }if (prefix) len +=strlen (prefix);
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 11/12 selftests/ftrace: Add test case for synthetic event syntax errors
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
Add a selftest that verifies that the syntax error messages and caret positions are correct for most of the possible synthetic event syntax error cases.
--- .../trigger-synthetic_event_syntax_errors.tc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc new file +19 -0
@@ -0,0 +1,19 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: event trigger - test synthetic_events syntax parser errors +# requires: synthetic_events error_log + +check_error() { # command-with-error-pos-by-^ + ftrace_errlog_check 'synthetic_events' "$1" 'synthetic_events' +} + +check_error 'myevent ^chr arg' # INVALID_TYPE +check_error 'myevent ^char str[];; int v' # INVALID_TYPE +check_error 'myevent char ^str]; int v' # INVALID_NAME +check_error 'myevent char ^str;[]' # INVALID_NAME +check_error 'myevent ^char str[; int v' # INVALID_TYPE +check_error '^mye;vent char str[]' # BAD_NAME +check_error 'myevent char str[]; ^int' # INVALID_FIELD +check_error '^myevent' # INCOMPLETE_CMD + +exit 0
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH 12/12 tracing: support "bool" type in synthetic trace events
In reply to Steven RostedtFrom: Axel Rasmussen <[email protected]>
It's common [1] to define tracepoint fields as "bool" when they contain a true / false value. Currently, defining a synthetic event with a "bool" field yields EINVAL. It's possible to work around this by using e.g. u8 (assuming sizeof(bool) is 1, and bool is unsigned; if either of these properties don't match, you get EINVAL [2]).
Supporting "bool" explicitly makes hooking this up easier and more portable for userspace.
[1]: grep -r "bool" include/trace/events/
[2]: check_synth_field() in kernel/trace/trace_events_hist.c
--- kernel/trace/trace_events_synth.c | 4 ++++ 1 file changed, 4 insertions(+)
kernel/trace/trace_events_synth.c +4 -0
@@ -229,6 +229,8 @@ static int synth_field_size(char *type) size =sizeof (long ); else if (strcmp(type, "unsigned long") == 0) size =sizeof (unsigned long ); + else if (strcmp(type, "bool") == 0) + size =sizeof (bool ); else if (strcmp(type, "pid_t") == 0) size =sizeof (pid_t); else if (strcmp(type, "gfp_t") == 0)
@@ -271,6 +273,8 @@ static const char *synth_field_fmt(char *type) fmt ="%ld" ; else if (strcmp(type, "unsigned long") == 0) fmt ="%lu" ; + else if (strcmp(type, "bool") == 0) + fmt ="%d" ; else if (strcmp(type, "pid_t") == 0) fmt ="%d" ; else if (strcmp(type, "gfp_t") == 0)
Signature · 1 line
2.28.0