tracing: common error_log for ftrace
Subsystems (4)
v5 cover tracing: common error_log for ftrace
From: Tom Zanussi <[email protected]>
Hi,
This is v5 of the frace error_log patchset. This version updates the patches according to suggestions from Masami and Namhyung and moves some of the selftest code from the kprobe/uprobe testcases to a common function that can be reused by multiple testcases, including the new error_log testcase.
Changes from v4:
- Merged in the >& -> 2> redirection patch into '[PATCH v5 06/11] selftests/ftrace: Add error_log testcase for probe errors'
- Added new patch, '[PATCH v5 07/11] selftests/ftrace: Move kprobe/uprobe check_error() to test.d/functions' that adds a new ftrace_errlog_check() to test.d/functions, which is based on check_error() from '[PATCH v5 06/11] selftests/ftrace: Add error_log testcase for probe errors'
- Modified the kprobe/uprobe test cases to use the common ftrace_errlog_check()
- Modified the error_log test cases to display the error position using the common ftrace_errlog_check()
- Updated subject for '[PATCH v5 08/11] selftests/ftrace: Remove trigger-extended-error-support testcase
- Streamlined the error_log open and write file operations as suggested by Masami
- Fixed the long lines in '[PATCH v4 04/11] tracing: Use tracing error_log with trace event filters' as suggested by Namhyung
Changes from v3:
- Added Masami's [PATCH 05/11] tracing: Use tracing error_log with probe events
- Added Masami's [PATCH 06/11] selftests/ftrace: Add error_log testcase for probe
- Added [PATCH 11/11] to fix [PATCH 06/11]
- Removed [RFC PATCH v3 4/5] tracing: Use tracing error_log with kprobe events
- Added [PATCH 08/11] selftests/ftrace: Add tracing/error_log testcase
- Removed trigger-extended-error-support testcase
- Changed [n] numbering to [timestamp] numbering as suggested by Masami
- Updated Documentation and README
Changes from v2:
- Added [n] numbering as suggested by Masami
Text from original post:
Last April, I posted an RFC patchset [1] implementing a common error_log interface as suggested by Masami [2]. We were supposed to discuss it at Plumbers but that never happened, and Steve recently asked about patches for a follow-on discussion [3], so here they are.
I incorporated comments from the previous discussion, the most important of which are:
- Incorporated Steve's suggestion of using static strings as in the existing trace event filter code, along with err_info indexing into the string arrays and a position for the error caret.
- Converted all the hist trigger errors and the existing trace event filter parse errors to use the new interface.
- Converted a few kprobe_event errors to the new interface as examples, but these will require more work - I didn't spend much time figuring out how to get the full kprobe command into the error info, for instance.
- Got rid of the custom single-page ring buffer and used standard lists instead.
For now, this is implemented on top of the latest 'hist trigger snapshot and onchange additions' patchset [4].
Below is an example session of a few failed commands and the corresponding error_log contents:
# echo > /sys/kernel/debug/tracing/error_log
# echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger -su: echo: write error: Invalid argument
# cat /sys/kernel/debug/tracing/error_log
[ 217.431858] hist:sched:sched_wakeup: error: Variable already defined
Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"
^# echo 'hist:key=comm:p=prio:onchange($q).snapshot()' > /sys/kernel/debug/tracing/events/sched/sched_waking/trigger -su: echo: write error: Invalid argument
# cat /sys/kernel/debug/tracing/error_log
[ 217.431858] hist:sched:sched_wakeup: error: Variable already defined
Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"
^
[ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable
Command: key=comm:p=prio:onchange($q).snapshot()
^# echo 'hist:keys=pid' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger # echo 'hist:keys=pid' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger -su: echo: write error: File exists # echo 'comm="cyclictest"' > /sys/kernel/debug/tracing/events/sched/sched_wakeup/filter -su: echo: write error: Invalid argument
# cat /sys/kernel/debug/tracing/error_log
[ 217.431858] hist:sched:sched_wakeup: error: Variable already defined
Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"
^
[ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable
Command: key=comm:p=prio:onchange($q).snapshot()
^
[ 715.626153] hist:sched:sched_wakeup: error: Hist trigger already exists
Command: keys=pid
^
[ 730.480310] event filter parse error: error: Invalid operator
Command: comm="cyclictest"
^# echo "((sig >= 10 && sig < 15) || dsig == 17) && comm != bash" > /sys/kernel/debug/tracing/events/signal/signal_generate/filter -su: echo: write error: Invalid argument
# cat /sys/kernel/debug/tracing/error_log
[ 217.431858] hist:sched:sched_wakeup: error: Variable already defined
Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"
^
[ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable
Command: key=comm:p=prio:onchange($q).snapshot()
^
[ 715.626153] hist:sched:sched_wakeup: error: Hist trigger already exists
Command: keys=pid
^
[ 730.480310] event filter parse error: error: Invalid operator
Command: comm="cyclictest"
^
[ 800.548673] event filter parse error: error: Field not found
Command: ((sig >= 10 && sig < 15) || dsig == 17) && comm != bash
^Thanks,
Tom
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/
[4] https://lore.kernel.org/lkml/[email protected]/
The following changes since commit 01c4e8042fc949018b70ded769f959bc3d80e8b6:
tracing: initialize variable in create_dyn_event() (2019-03-22 15:19:34 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/error_log_v5
Masami Hiramatsu (2):
tracing: Use tracing error_log with probe events selftests/ftrace: Add error_log testcase for probe errors
Tom Zanussi (9):
tracing: Add tracing error log
tracing: Save the last hist command's associated event name
tracing: Use tracing error_log with hist triggers
tracing: Use tracing error_log with trace event filters selftests/ftrace: Move kprobe/uprobe check_error() to test.d/functions selftests/ftrace: Remove trigger-extended-error-support testcase selftests/ftrace: Add tracing/error_log testcase
tracing: Add tracing/error_log Documentation
tracing: Add error_log to README
Documentation/trace/ftrace.rst | 31 +++ Documentation/trace/histogram.rst | 16 +- kernel/trace/trace.c | 219 ++++++++++++++++ kernel/trace/trace.h | 4 + kernel/trace/trace_events_filter.c | 11 +- kernel/trace/trace_events_hist.c | 221 +++++++++-------- kernel/trace/trace_kprobe.c | 77 +++--- kernel/trace/trace_probe.c | 274 +++++++++++++++------ kernel/trace/trace_probe.h | 77 +++++- kernel/trace/trace_uprobe.c | 44 +++- .../ftrace/test.d/ftrace/tracing-error-log.tc | 19 ++ tools/testing/selftests/ftrace/test.d/functions | 12 + .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 85 +++++++ .../ftrace/test.d/kprobe/uprobe_syntax_errors.tc | 23 ++ .../inter-event/trigger-extended-error-support.tc | 28 --- 15 files changed, 874 insertions(+), 267 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.tc delete mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-extended-error-support.tc
Signature · 1 line
2.14.1
PATCH v5 1/11 tracing: Add tracing error log
From: Tom Zanussi <[email protected]>
Introduce a new ftrace file, tracing/error_log, for ftrace commands to log errors. This is useful for allowing more complex commands such as hist trigger and kprobe_event commands to point out specifically where something may have gone wrong without forcing them to resort to more ad hoc methods such as tacking error messages onto existing output files.
To log a tracing error, call the event_log_err() function, passing it a location string describing where it came from e.g. kprobe_events or system:event, the command that caused the error, an array of static error strings describing errors and an index within that array which describes the specific error, along with the position to place the error caret.
Reading the log displays the last (currently) 8 errors logged in the following format:
[timestamp] <loc>: error: <static error text>
Command: <command that caused the error>
^
Memory for the error log isn't allocated unless there has been a trace event error, and the error log can be cleared and have its memory freed by writing the empty string in truncation mode to it:
# echo > tracing/error_log.
--- kernel/trace/trace.c | 218 +++++++++++++++++++++++++++++++++++++++++++++++++++ kernel/trace/trace.h | 4 + 2 files changed, 222 insertions(+)
kernel/trace/trace.c +218 -0
@@ -6876,6 +6876,221 @@ static const struct file_operations snapshot_raw_fops = {#endif /* CONFIG_TRACER_SNAPSHOT */ +#define TRACING_LOG_ERRS_MAX 8 +#define TRACING_LOG_LOC_MAX 128 + +#define CMD_PREFIX " Command: " + +struct err_info { +const char **errs;/* ptr to loc-specific array of err strings */ +u8 type;/* index into errs -> specific err string */ +u8 pos;/* MAX_FILTER_STR_VAL = 256 */ +u64 ts; +}; + +struct tracing_log_err { +struct list_head list; +struct err_info info; +char loc[TRACING_LOG_LOC_MAX ];/* err location */ +char cmd[MAX_FILTER_STR_VAL ];/* what caused err */ +}; + +static LIST_HEAD (tracing_err_log); +static DEFINE_MUTEX (tracing_err_log_lock); + +static unsigned int n_tracing_err_log_entries; + +struct tracing_log_err *get_tracing_log_err (void ) +{ +struct tracing_log_err *err; + +if (n_tracing_err_log_entries <TRACING_LOG_ERRS_MAX ) { + err =kzalloc (sizeof (*err),GFP_KERNEL ); +if (!err) + err =ERR_PTR (-ENOMEM ); + n_tracing_err_log_entries++; + +return err; + } + + err = list_first_entry(&tracing_err_log, struct tracing_log_err, list); +list_del (&err->list); + +return err; +} + +/** +* err_pos - find the position of a string within a command for error careting +* @cmd: The tracing command that caused the error +* @str: The string to position the caret at within @cmd +* +* Finds the position of the first occurence of @str within @cmd. The +* return value can be passed to tracing_log_err() for caret placement +* within @cmd. +* +* Returns the index within @cmd of the first occurence of @str or 0 +* if @str was not found. +*/ +unsigned int err_pos (char *cmd,const char *str) +{ +char *found; + +if (WARN_ON (!strlen (cmd))) +return 0 ; + + found =strstr (cmd, str); +if (found) +return found - cmd; + +return 0 ; +} + +/** +* tracing_log_err - write an error to the tracing error log +* @loc: A string describing where the error occurred +* @cmd: The tracing command that caused the error +* @errs: The array of loc-specific static error strings +* @type: The index into errs[], which produces the specific static err string +* @pos: The position the caret should be placed in the cmd +* +* Writes an error into tracing/error_log of the form: +* +* <loc>: error: <text> +* Command: <cmd> +* ^ +* +* tracing/error_log is a small log file containing the last +* TRACING_LOG_ERRS_MAX errors (8). Memory for errors isn't allocated +* unless there has been a tracing error, and the error log can be +* cleared and have its memory freed by writing the empty string in +* truncation mode to it i.e. echo > tracing/error_log. +* +* NOTE: the @errs array along with the @type param are used to +* produce a static error string - this string is not copied and saved +* when the error is logged - only a pointer to it is saved. See +* existing callers for examples of how static strings are typically +* defined for use with tracing_log_err(). +*/ +void tracing_log_err (const char *loc,const char *cmd, +const char **errs,u8 type,u8 pos) +{ +struct tracing_log_err *err; + +mutex_lock (&tracing_err_log_lock); + err =get_tracing_log_err (); +if (PTR_ERR (err) == -ENOMEM ) { +mutex_unlock (&tracing_err_log_lock); +return ; + } + +snprintf (err->loc,TRACING_LOG_LOC_MAX ,"%s: error: " , loc); +snprintf (err->cmd,MAX_FILTER_STR_VAL ,"\n" CMD_PREFIX "%s\n" , cmd); + + err->info.errs = errs; + err->info.type = type; + err->info.pos = pos; + err->info.ts =local_clock (); + +list_add_tail (&err->list, &tracing_err_log); +mutex_unlock (&tracing_err_log_lock); +} + +static void clear_tracing_err_log (void ) +{ +struct tracing_log_err *err, *next; + +mutex_lock (&tracing_err_log_lock); +list_for_each_entry_safe (err, next, &tracing_err_log, list) { +list_del (&err->list); +kfree (err); + } + + n_tracing_err_log_entries =0 ; +mutex_unlock (&tracing_err_log_lock); +} + +static void *tracing_err_log_seq_start (struct seq_file *m,loff_t *pos) +{ +mutex_lock (&tracing_err_log_lock); + +return seq_list_start (&tracing_err_log, *pos); +} + +static void *tracing_err_log_seq_next (struct seq_file *m,void *v,loff_t *pos) +{ +return seq_list_next (v, &tracing_err_log, pos); +} + +static void tracing_err_log_seq_stop (struct seq_file *m,void *v) +{ +mutex_unlock (&tracing_err_log_lock); +} + +static void tracing_err_log_show_pos (struct seq_file *m,u8 pos) +{ +u8 i; + +for (i =0 ; i <sizeof (CMD_PREFIX ) -1 ; i++) +seq_putc (m,' ' ); +for (i =0 ; i < pos; i++) +seq_putc (m,' ' ); +seq_puts (m,"^\n" ); +} + +static int tracing_err_log_seq_show (struct seq_file *m,void *v) +{ +struct tracing_log_err *err = v; + +if (err) { +const char *err_text = err->info.errs[err->info.type]; +u64 sec = err->info.ts; +u32 nsec; + + nsec =do_div (sec,NSEC_PER_SEC ); +seq_printf (m,"[%5llu.%06u] %s%s" , sec, nsec /1000 , + err->loc, err_text); +seq_printf (m,"%s" , err->cmd); +tracing_err_log_show_pos (m, err->info.pos); + } + +return 0 ; +} + +static const struct seq_operations tracing_err_log_seq_ops = { + .start = tracing_err_log_seq_start, + .next = tracing_err_log_seq_next, + .stop = tracing_err_log_seq_stop, + .show = tracing_err_log_seq_show +}; + +static int tracing_err_log_open (struct inode *inode,struct file *file) +{ +int ret =0 ; + +/* If this file was opened for write, then erase contents */ +if ((file->f_mode &FMODE_WRITE ) && (file->f_flags &O_TRUNC )) +clear_tracing_err_log (); + +if (file->f_mode &FMODE_READ ) + ret =seq_open (file, &tracing_err_log_seq_ops); + +return ret; +} + +static ssize_t tracing_err_log_write (struct file *file, +const char __user *buffer, +size_t count,loff_t *ppos) +{ +return count; +} + +static const struct file_operations tracing_err_log_fops = { + .open = tracing_err_log_open, + .write = tracing_err_log_write, + .read = seq_read, + .llseek = seq_lseek, +}; +static int tracing_buffers_open (struct inode *inode,struct file *filp) {struct trace_array *tr = inode->i_private;
@@ -8258,6 +8473,9 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer) tr, &snapshot_fops);#endif +trace_create_file ("error_log" ,0644 , d_tracer, + tr, &tracing_err_log_fops); +for_each_tracing_cpu (cpu)tracing_init_tracefs_percpu (tr, cpu);
kernel/trace/trace.h +4 -0
@@ -1884,6 +1884,10 @@ extern ssize_t trace_parse_run_command(struct file *file, const char __user *buffer, size_t count, loff_t *ppos, int (*createfn)(int, char**)); +extern unsigned int err_pos (char *cmd,const char *str); +extern void tracing_log_err (const char *loc,const char *cmd, +const char **errs,u8 type,u8 pos); +/* * Normal trace_printk() and friends allocates special buffers * to do the manipulation, as well as saves the print formats
Signature · 1 line
2.14.1
PATCH v5 2/11 tracing: Save the last hist command's associated event name
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
In preparation for making use of the new trace error log, save the subsystem and event name associated with the last hist command - it will be passed as the location param in the event_log_err() calls.
--- kernel/trace/trace_events_hist.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-)
kernel/trace/trace_events_hist.c +27 -6
@@ -535,15 +535,34 @@ static struct track_data *track_data_alloc(unsigned int key_len,return data; } -static char last_hist_cmd[MAX_FILTER_STR_VAL ]; +static char last_cmd[MAX_FILTER_STR_VAL ]; +static char last_cmd_loc[MAX_FILTER_STR_VAL ]; +static char hist_err_str[MAX_FILTER_STR_VAL ]; -static void last_cmd_set (char *str) +static void last_cmd_set (struct trace_event_file *file,char *str) { +const char *system =NULL , *name =NULL ; +struct trace_event_call *call; +if (!str)return ; -strncpy (last_hist_cmd, str,MAX_FILTER_STR_VAL -1 ); +strncpy (last_cmd, str,MAX_FILTER_STR_VAL -1 ); + +if (file) { + call = file->event_call; + + system = call->class->system; +if (system) { + name =trace_event_name (call); +if (!name) + system =NULL ; + } + } + +if (system) +snprintf (last_cmd_loc,MAX_FILTER_STR_VAL ,"hist:%s:%s" , system, name); }static void hist_err (char *str,char *var)
@@ -583,6 +602,8 @@ static void hist_err_event(char *str, char *system, char *event, char *var)static void hist_err_clear (void ) { hist_err_str[0 ] ='\0' ; + last_cmd[0 ] ='\0' ; + last_cmd_loc[0 ] ='\0' ; }static bool have_hist_err (void )
@@ -5438,8 +5459,8 @@ static int hist_show(struct seq_file *m, void *v) }if (have_hist_err ()) { -seq_printf (m,"\nERROR: %s\n" , hist_err_str); -seq_printf (m," Last command: %s\n" , last_hist_cmd); +seq_printf (m,"\n%s: error: \n" , hist_err_str); +seq_printf (m," Last command: %s\n" , last_cmd); } out_unlock:
@@ -6043,8 +6064,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,lockdep_assert_held (&event_mutex);if (glob &&strlen (glob)) { -last_cmd_set (param);hist_err_clear (); +last_cmd_set (file, param); }if (!param)
Signature · 1 line
2.14.1
PATCH v5 3/11 tracing: Use tracing error_log with hist triggers
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
Replace hist_err() and hist_err_event() with tracing_log_err() from the new tracing error_log mechanism.
Also add a couple related helper functions and remove most of the old hist_err()-related code.
With this change, users no longer read the hist files for hist trigger error information, but instead look at tracing/error_log for the same information.
--- kernel/trace/trace_events_hist.c | 206 ++++++++++++++++++++------------------- 1 file changed, 104 insertions(+), 102 deletions(-)
kernel/trace/trace_events_hist.c +104 -102
@@ -22,6 +22,57 @@#define STR_VAR_LEN_MAX 32/* must be multiple of sizeof(u64) */ +#define ERRORS \ + C(NONE, "No error"), \ + C(DUPLICATE_VAR, "Variable already defined"), \ + C(VAR_NOT_UNIQUE, "Variable name not unique, need to use fully qualified name (subsys.event.var) for variable"), \ + C(TOO_MANY_VARS, "Too many variables defined"), \ + C(MALFORMED_ASSIGNMENT, "Malformed assignment"), \ + C(NAMED_MISMATCH, "Named hist trigger doesn't match existing named trigger (includes variables)"), \ + C(TRIGGER_EEXIST, "Hist trigger already exists"), \ + C(TRIGGER_ENOENT_CLEAR, "Can't clear or continue a nonexistent hist trigger"), \ + C(SET_CLOCK_FAIL, "Couldn't set trace_clock"), \ + C(BAD_FIELD_MODIFIER, "Invalid field modifier"), \ + C(TOO_MANY_SUBEXPR, "Too many subexpressions (3 max)"), \ + C(TIMESTAMP_MISMATCH, "Timestamp units in expression don't match"), \ + C(TOO_MANY_FIELD_VARS, "Too many field variables defined"), \ + C(EVENT_FILE_NOT_FOUND, "Event file not found"), \ + C(HIST_NOT_FOUND, "Matching event histogram not found"), \ + C(HIST_CREATE_FAIL, "Couldn't create histogram for field"), \ + C(SYNTH_VAR_NOT_FOUND, "Couldn't find synthetic variable"), \ + C(SYNTH_EVENT_NOT_FOUND,"Couldn't find synthetic event"), \ + C(SYNTH_TYPE_MISMATCH, "Param type doesn't match synthetic event field type"), \ + C(SYNTH_COUNT_MISMATCH, "Param count doesn't match synthetic event field count"), \ + C(FIELD_VAR_PARSE_FAIL, "Couldn't parse field variable"), \ + C(VAR_CREATE_FIND_FAIL, "Couldn't create or find variable"), \ + C(ONX_NOT_VAR, "For onmax(x) or onchange(x), x must be a variable"), \ + C(ONX_VAR_NOT_FOUND, "Couldn't find onmax or onchange variable"), \ + C(ONX_VAR_CREATE_FAIL, "Couldn't create onmax or onchange variable"), \ + C(FIELD_VAR_CREATE_FAIL,"Couldn't create field variable"), \ + C(TOO_MANY_PARAMS, "Too many action params"), \ + C(PARAM_NOT_FOUND, "Couldn't find param"), \ + C(INVALID_PARAM, "Invalid action param"), \ + C(ACTION_NOT_FOUND, "No action found"), \ + C(NO_SAVE_PARAMS, "No params found for save()"), \ + C(TOO_MANY_SAVE_ACTIONS,"Can't have more than one save() action per hist"), \ + C(ACTION_MISMATCH, "Handler doesn't support action"), \ + C(NO_CLOSING_PAREN, "No closing paren found"), \ + C(SUBSYS_NOT_FOUND, "Missing subsystem"), \ + C(INVALID_SUBSYS_EVENT, "Invalid subsystem or event name"), \ + C(INVALID_REF_KEY, "Using variable references as keys not supported"), \ + C(VAR_NOT_FOUND, "Couldn't find variable"), \ + C(FIELD_NOT_FOUND, "Couldn't find field"), + +#undef C +#define C (a, b) HIST_ERR_##a + +enum {ERRORS }; + +#undef C +#define C (a, b) b + +static const char *err_text[] = {ERRORS }; +struct hist_field ; typedef u64 (*hist_field_fn_t) (struct hist_field *field,
@@ -538,7 +589,10 @@ static struct track_data *track_data_alloc(unsigned int key_len,static char last_cmd[MAX_FILTER_STR_VAL ];static char last_cmd_loc[MAX_FILTER_STR_VAL ]; -static char hist_err_str[MAX_FILTER_STR_VAL ]; +static int errpos (char *str) +{ +return err_pos (last_cmd, str); +}static void last_cmd_set (struct trace_event_file *file,char *str) {
@@ -565,55 +619,17 @@ static void last_cmd_set(struct trace_event_file *file, char *str)snprintf (last_cmd_loc,MAX_FILTER_STR_VAL ,"hist:%s:%s" , system, name); } -static void hist_err (char *str,char *var) +static void hist_err (u8 err_type,u8 err_pos) { -int maxlen =MAX_FILTER_STR_VAL -1 ; - -if (!str) -return ; - -if (strlen (hist_err_str)) -return ; - -if (!var) - var ="" ; - -if (strlen (hist_err_str) +strlen (str) +strlen (var) > maxlen) -return ; - -strcat (hist_err_str, str); -strcat (hist_err_str, var); -} - -static void hist_err_event (char *str,char *system,char *event,char *var) -{ -char err[MAX_FILTER_STR_VAL ]; - -if (system && var) -snprintf (err,MAX_FILTER_STR_VAL ,"%s.%s.%s" , system, event, var); -else if (system) -snprintf (err,MAX_FILTER_STR_VAL ,"%s.%s" , system, event); -else -strscpy (err, var,MAX_FILTER_STR_VAL ); - -hist_err (str, err); +tracing_log_err (last_cmd_loc, last_cmd, err_text, err_type, err_pos); }static void hist_err_clear (void ) { - hist_err_str[0 ] ='\0' ; last_cmd[0 ] ='\0' ; last_cmd_loc[0 ] ='\0' ; } -static bool have_hist_err (void ) -{ -if (strlen (hist_err_str)) -return true; - -return false; -} -struct synth_trace_event {struct trace_entry ent;u64 fields[];
@@ -1740,7 +1756,7 @@ static struct trace_event_file *find_var_file(struct trace_array *tr,if (find_var_field (var_hist_data, var_name)) {if (found) { -hist_err_event ("Variable name not unique, need to use fully qualified name (subsys.event.var) for variable: " , system, event_name, var_name); +hist_err (HIST_ERR_VAR_NOT_UNIQUE ,errpos (var_name));return NULL ; }
@@ -1791,7 +1807,7 @@ find_match_var(struct hist_trigger_data *hist_data, char *var_name) hist_field =find_file_var (file, var_name);if (hist_field) {if (found) { -hist_err_event ("Variable name not unique, need to use fully qualified name (subsys.event.var) for variable: " , system, event_name, var_name); +hist_err (HIST_ERR_VAR_NOT_UNIQUE ,errpos (var_name));return ERR_PTR (-EINVAL ); }
@@ -2023,7 +2039,6 @@ static int parse_action(char *str, struct hist_trigger_attrs *attrs) attrs->n_actions++; ret =0 ; } -return ret; }
@@ -2083,7 +2098,7 @@ static int parse_assignment(char *str, struct hist_trigger_attrs *attrs)char *assignment;if (attrs->n_assignments ==TRACING_MAP_VARS_MAX ) { -hist_err ("Too many variables defined: " , str); +hist_err (HIST_ERR_TOO_MANY_VARS ,errpos (str)); ret = -EINVAL ; goto out; }
@@ -2681,8 +2696,7 @@ static struct hist_field *parse_var_ref(struct hist_trigger_data *hist_data, system, event_name);if (!ref_field) -hist_err_event ("Couldn't find variable: $" , - system, event_name, var_name); +hist_err (HIST_ERR_VAR_NOT_FOUND ,errpos (var_name));return ref_field; }
@@ -2716,7 +2730,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,else if (strcmp (modifier,"usecs" ) ==0 ) *flags |=HIST_FIELD_FL_TIMESTAMP_USECS ;else { -hist_err ("Invalid field modifier: " , modifier); +hist_err (HIST_ERR_BAD_FIELD_MODIFIER ,errpos (modifier)); field =ERR_PTR (-EINVAL ); goto out; }
@@ -2732,7 +2746,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,else { field =trace_find_event_field (file->event_call, field_name);if (!field || !field->size) { -hist_err ("Couldn't find field: " , field_name); +hist_err (HIST_ERR_FIELD_NOT_FOUND ,errpos (field_name)); field =ERR_PTR (-EINVAL ); goto out; }
@@ -2843,7 +2857,7 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,/* we support only -(xxx) i.e. explicit parens required */ if (level >3 ) { -hist_err ("Too many subexpressions (3 max): " , str); +hist_err (HIST_ERR_TOO_MANY_SUBEXPR ,errpos (str)); ret = -EINVAL ; goto free; }
@@ -2926,7 +2940,7 @@ static int check_expr_operands(struct hist_field *operand1,if ((operand1_flags &HIST_FIELD_FL_TIMESTAMP_USECS ) != (operand2_flags &HIST_FIELD_FL_TIMESTAMP_USECS )) { -hist_err ("Timestamp units in expression don't match" ,NULL ); +hist_err (HIST_ERR_TIMESTAMP_MISMATCH ,0 );return -EINVAL ; }
@@ -2944,7 +2958,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,char *sep, *operand1_str;if (level >3 ) { -hist_err ("Too many subexpressions (3 max): " , str); +hist_err (HIST_ERR_TOO_MANY_SUBEXPR ,errpos (str));return ERR_PTR (-EINVAL ); }
@@ -3182,16 +3196,14 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,int ret;if (target_hist_data->n_field_var_hists >=SYNTH_FIELDS_MAX ) { -hist_err_event ("trace action: Too many field variables defined: " , - subsys_name, event_name, field_name); +hist_err (HIST_ERR_TOO_MANY_FIELD_VARS ,errpos (field_name));return ERR_PTR (-EINVAL ); } file =event_file (tr, subsys_name, event_name);if (IS_ERR (file)) { -hist_err_event ("trace action: Event file not found: " , - subsys_name, event_name, field_name); +hist_err (HIST_ERR_EVENT_FILE_NOT_FOUND ,errpos (field_name)); ret =PTR_ERR (file);return ERR_PTR (ret); }
@@ -3204,8 +3216,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,*/ hist_data =find_compatible_hist (target_hist_data, file);if (!hist_data) { -hist_err_event ("trace action: Matching event histogram not found: " , - subsys_name, event_name, field_name); +hist_err (HIST_ERR_HIST_NOT_FOUND ,errpos (field_name));return ERR_PTR (-EINVAL ); }
@@ -3266,8 +3277,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,kfree (cmd);kfree (var_hist->cmd);kfree (var_hist); -hist_err_event ("trace action: Couldn't create histogram for field: " , - subsys_name, event_name, field_name); +hist_err (HIST_ERR_HIST_CREATE_FAIL ,errpos (field_name));return ERR_PTR (ret); }
@@ -3279,8 +3289,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,if (IS_ERR_OR_NULL (event_var)) {kfree (var_hist->cmd);kfree (var_hist); -hist_err_event ("trace action: Couldn't find synthetic variable: " , - subsys_name, event_name, field_name); +hist_err (HIST_ERR_SYNTH_VAR_NOT_FOUND ,errpos (field_name));return ERR_PTR (-EINVAL ); }
@@ -3417,21 +3426,21 @@ static struct field_var *create_field_var(struct hist_trigger_data *hist_data,int ret =0 ;if (hist_data->n_field_vars >=SYNTH_FIELDS_MAX ) { -hist_err ("Too many field variables defined: " , field_name); +hist_err (HIST_ERR_TOO_MANY_FIELD_VARS ,errpos (field_name)); ret = -EINVAL ; goto err; } val =parse_atom (hist_data, file, field_name, &flags,NULL );if (IS_ERR (val)) { -hist_err ("Couldn't parse field variable: " , field_name); +hist_err (HIST_ERR_FIELD_VAR_PARSE_FAIL ,errpos (field_name)); ret =PTR_ERR (val); goto err; } var =create_var (hist_data, file, field_name, val->size, val->type);if (IS_ERR (var)) { -hist_err ("Couldn't create or find variable: " , field_name); +hist_err (HIST_ERR_VAR_CREATE_FIND_FAIL ,errpos (field_name));kfree (val); ret =PTR_ERR (var); goto err;
@@ -3763,14 +3772,14 @@ static int track_data_create(struct hist_trigger_data *hist_data, track_data_var_str = data->track_data.var_str;if (track_data_var_str[0 ] !='$' ) { -hist_err ("For onmax(x) or onchange(x), x must be a variable: " , track_data_var_str); +hist_err (HIST_ERR_ONX_NOT_VAR ,errpos (track_data_var_str));return -EINVAL ; } track_data_var_str++; var_field =find_target_event_var (hist_data,NULL ,NULL , track_data_var_str);if (!var_field) { -hist_err ("Couldn't find onmax or onchange variable: " , track_data_var_str); +hist_err (HIST_ERR_ONX_VAR_NOT_FOUND ,errpos (track_data_var_str));return -EINVAL ; }
@@ -3783,7 +3792,7 @@ static int track_data_create(struct hist_trigger_data *hist_data,if (data->handler ==HANDLER_ONMAX ) track_var =create_var (hist_data, file,"__max" ,sizeof (u64),"u64" );if (IS_ERR (track_var)) { -hist_err ("Couldn't create onmax variable: " ,"__max" ); +hist_err (HIST_ERR_ONX_VAR_CREATE_FAIL ,0 ); ret =PTR_ERR (track_var); goto out; }
@@ -3791,7 +3800,7 @@ static int track_data_create(struct hist_trigger_data *hist_data,if (data->handler ==HANDLER_ONCHANGE ) track_var =create_var (hist_data, file,"__change" ,sizeof (u64),"u64" );if (IS_ERR (track_var)) { -hist_err ("Couldn't create onchange variable: " ,"__change" ); +hist_err (HIST_ERR_ONX_VAR_CREATE_FAIL ,0 ); ret =PTR_ERR (track_var); goto out; }
@@ -3810,20 +3819,20 @@ static int parse_action_params(char *params, struct action_data *data)while (params) {if (data->n_params >=SYNTH_FIELDS_MAX ) { -hist_err ("Too many action params" ,"" ); +hist_err (HIST_ERR_TOO_MANY_PARAMS ,0 ); goto out; } param =strsep (¶ms,"," );if (!param) { -hist_err ("No action param found" ,"" ); +hist_err (HIST_ERR_PARAM_NOT_FOUND ,0 ); ret = -EINVAL ; goto out; } param =strstrip (param);if (strlen (param) <2 ) { -hist_err ("Invalid action param: " , param); +hist_err (HIST_ERR_INVALID_PARAM ,errpos (param)); ret = -EINVAL ; goto out; }
@@ -3855,14 +3864,14 @@ static int action_parse(char *str, struct action_data *data,strsep (&str,"." );if (!str) { -hist_err ("action parsing: No action found" ,"" ); +hist_err (HIST_ERR_ACTION_NOT_FOUND ,0 ); ret = -EINVAL ; goto out; } action_name =strsep (&str,"(" );if (!action_name || !str) { -hist_err ("action parsing: No action found" ,"" ); +hist_err (HIST_ERR_ACTION_NOT_FOUND ,0 ); ret = -EINVAL ; goto out; }
@@ -3871,7 +3880,7 @@ static int action_parse(char *str, struct action_data *data,char *params =strsep (&str,")" );if (!params) { -hist_err ("action parsing: No params found for %s" ,"save" ); +hist_err (HIST_ERR_NO_SAVE_PARAMS ,0 ); ret = -EINVAL ; goto out; }
@@ -3885,7 +3894,7 @@ static int action_parse(char *str, struct action_data *data,else if (handler ==HANDLER_ONCHANGE ) data->track_data.check_val = check_track_val_changed;else { -hist_err ("action parsing: Handler doesn't support action: " , action_name); +hist_err (HIST_ERR_ACTION_MISMATCH ,errpos (action_name)); ret = -EINVAL ; goto out; }
@@ -3897,7 +3906,7 @@ static int action_parse(char *str, struct action_data *data,char *params =strsep (&str,")" );if (!str) { -hist_err ("action parsing: No closing paren found: %s" , params); +hist_err (HIST_ERR_NO_CLOSING_PAREN ,errpos (params)); ret = -EINVAL ; goto out; }
@@ -3907,7 +3916,7 @@ static int action_parse(char *str, struct action_data *data,else if (handler ==HANDLER_ONCHANGE ) data->track_data.check_val = check_track_val_changed;else { -hist_err ("action parsing: Handler doesn't support action: " , action_name); +hist_err (HIST_ERR_ACTION_MISMATCH ,errpos (action_name)); ret = -EINVAL ; goto out; }
@@ -4060,7 +4069,7 @@ trace_action_find_var(struct hist_trigger_data *hist_data, }if (!hist_field) -hist_err_event ("trace action: Couldn't find param: $" , system, event, var); +hist_err (HIST_ERR_PARAM_NOT_FOUND ,errpos (var));return hist_field; }
@@ -4135,7 +4144,7 @@ static int trace_action_create(struct hist_trigger_data *hist_data, event =find_synth_event (synth_event_name);if (!event) { -hist_err ("trace action: Couldn't find synthetic event: " , synth_event_name); +hist_err (HIST_ERR_SYNTH_EVENT_NOT_FOUND ,errpos (synth_event_name));return -EINVAL ; }
@@ -4196,15 +4205,14 @@ static int trace_action_create(struct hist_trigger_data *hist_data,continue ; } -hist_err_event ("trace action: Param type doesn't match synthetic event field type: " , - system, event_name, param); +hist_err (HIST_ERR_SYNTH_TYPE_MISMATCH ,errpos (param));kfree (p); ret = -EINVAL ; goto err; }if (field_pos != event->n_fields) { -hist_err ("trace action: Param count doesn't match synthetic event field count: " , event->name); +hist_err (HIST_ERR_SYNTH_COUNT_MISMATCH ,errpos (event->name)); ret = -EINVAL ; goto err; }
@@ -4250,7 +4258,7 @@ static int action_create(struct hist_trigger_data *hist_data,if (data->action ==ACTION_SAVE ) {if (hist_data->n_save_vars) { ret = -EEXIST ; -hist_err ("save action: Can't have more than one save() action per hist" ,"" ); +hist_err (HIST_ERR_TOO_MANY_SAVE_ACTIONS ,0 ); goto out; }
@@ -4263,7 +4271,7 @@ static int action_create(struct hist_trigger_data *hist_data, field_var =create_target_field_var (hist_data,NULL ,NULL , param);if (IS_ERR (field_var)) { -hist_err ("save action: Couldn't create field variable: " , param); +hist_err (HIST_ERR_FIELD_VAR_CREATE_FAIL ,errpos (param)); ret =PTR_ERR (field_var);kfree (param); goto out;
@@ -4297,19 +4305,18 @@ static struct action_data *onmatch_parse(struct trace_array *tr, char *str) match_event =strsep (&str,")" );if (!match_event || !str) { -hist_err ("onmatch: Missing closing paren: " , match_event); +hist_err (HIST_ERR_NO_CLOSING_PAREN ,errpos (match_event)); goto free; } match_event_system =strsep (&match_event,"." );if (!match_event) { -hist_err ("onmatch: Missing subsystem for match event: " , match_event_system); +hist_err (HIST_ERR_SUBSYS_NOT_FOUND ,errpos (match_event_system)); goto free; }if (IS_ERR (event_file (tr, match_event_system, match_event))) { -hist_err_event ("onmatch: Invalid subsystem or event name: " , - match_event_system, match_event,NULL ); +hist_err (HIST_ERR_INVALID_SUBSYS_EVENT ,errpos (match_event)); goto free; }
@@ -4400,7 +4407,7 @@ static int create_var_field(struct hist_trigger_data *hist_data,return -EINVAL ;if (find_var (hist_data, file, var_name) && !hist_data->remove) { -hist_err ("Variable already defined: " , var_name); +hist_err (HIST_ERR_DUPLICATE_VAR ,errpos (var_name));return -EINVAL ; }
@@ -4481,7 +4488,7 @@ static int create_key_field(struct hist_trigger_data *hist_data, }if (hist_field->flags &HIST_FIELD_FL_VAR_REF ) { -hist_err ("Using variable references as keys not supported: " , field_str); +hist_err (HIST_ERR_INVALID_REF_KEY ,errpos (field_str));destroy_hist_field (hist_field,0 ); ret = -EINVAL ; goto out;
@@ -4595,13 +4602,13 @@ static int parse_var_defs(struct hist_trigger_data *hist_data) var_name =strsep (&field_str,"=" );if (!var_name || !field_str) { -hist_err ("Malformed assignment: " , var_name); +hist_err (HIST_ERR_MALFORMED_ASSIGNMENT ,errpos (var_name)); ret = -EINVAL ; goto free; }if (n_vars ==TRACING_MAP_VARS_MAX ) { -hist_err ("Too many variables defined: " , var_name); +hist_err (HIST_ERR_TOO_MANY_VARS ,errpos (var_name)); ret = -EINVAL ; goto free; }
@@ -5458,11 +5465,6 @@ static int hist_show(struct seq_file *m, void *v)hist_trigger_show (m, data, n++); } -if (have_hist_err ()) { -seq_printf (m,"\n%s: error: \n" , hist_err_str); -seq_printf (m," Last command: %s\n" , last_cmd); - } - out_unlock:mutex_unlock (&event_mutex);
@@ -5834,7 +5836,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,if (named_data) {if (!hist_trigger_match (data, named_data, named_data, true)) { -hist_err ("Named hist trigger doesn't match existing named trigger (includes variables): " , hist_data->attrs->name); +hist_err (HIST_ERR_NAMED_MISMATCH ,errpos (hist_data->attrs->name)); ret = -EINVAL ; goto out; }
@@ -5855,7 +5857,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,else if (hist_data->attrs->clear)hist_clear (test);else { -hist_err ("Hist trigger already exists" ,NULL ); +hist_err (HIST_ERR_TRIGGER_EEXIST ,0 ); ret = -EEXIST ; } goto out;
@@ -5863,7 +5865,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops, } new:if (hist_data->attrs->cont || hist_data->attrs->clear) { -hist_err ("Can't clear or continue a nonexistent hist trigger" ,NULL ); +hist_err (HIST_ERR_TRIGGER_ENOENT_CLEAR ,0 ); ret = -ENOENT ; goto out; }
@@ -5888,7 +5890,7 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops, ret =tracing_set_clock (file->tr, hist_data->attrs->clock);if (ret) { -hist_err ("Couldn't set trace_clock: " , clock); +hist_err (HIST_ERR_SET_CLOCK_FAIL ,errpos (clock)); goto out; }
Signature · 1 line
2.14.1
PATCH v5 4/11 tracing: Use tracing error_log with trace event filters
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
Use tracing_log_err() from the new tracing error_log mechanism to send filter parse errors to tracing/error_log.
With this change, users will be able to see filter errors by looking at tracing/error_log.
The same errors will also be available in the filter file, as expected.
--- kernel/trace/trace_events_filter.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
kernel/trace/trace_events_filter.c +9 -2
@@ -66,7 +66,8 @@ static const char * ops[] = { OPS };C (INVALID_FILTER ,"Meaningless filter expression" ), \C (IP_FIELD_ONLY ,"Only 'ip' field is supported for function trace" ), \C (INVALID_VALUE ,"Invalid value (did you forget quotes)?" ), \ -C (NO_FILTER ,"No filter found" ), +C (ERRNO ,"Error" ), \ + C(NO_FILTER, "No filter found")#undef C#define C (a, b) FILT_ERR_##a
@@ -76,7 +77,7 @@ enum { ERRORS };#undef C#define C (a, b) b -static char *err_text[] = {ERRORS }; +static const char *err_text[] = {ERRORS };/* Called after a '!' character but "!=" and "!~" are not "not"s */ static bool is_not (const char *str)
@@ -947,8 +948,14 @@ static void append_filter_err(struct filter_parse_error *pe,if (pe->lasterr >0 ) {trace_seq_printf (s,"\n%*s" , pos,"^" );trace_seq_printf (s,"\nparse_error: %s\n" , err_text[pe->lasterr]); +tracing_log_err ("event filter parse error" , + filter->filter_string, err_text, + pe->lasterr, pe->lasterr_pos); }else {trace_seq_printf (s,"\nError: (%d)\n" , pe->lasterr); +tracing_log_err ("event filter parse error" , + filter->filter_string, err_text, +FILT_ERR_ERRNO ,0 ); }trace_seq_putc (s,0 ); buf =kmemdup_nul (s->buffer, s->seq.len,GFP_KERNEL );
Signature · 1 line
2.14.1
PATCH v5 5/11 tracing: Use tracing error_log with probe events
In reply to Tom ZanussiFrom: Masami Hiramatsu <[email protected]>
Use tracing error_log with probe events for logging error more precisely. This also makes all parse error returns -EINVAL (except for -ENOMEM), because user can see better error message in error_log file now.
--- kernel/trace/trace_kprobe.c | 77 ++++++++----- kernel/trace/trace_probe.c | 274 +++++++++++++++++++++++++++++++------------- kernel/trace/trace_probe.h | 77 ++++++++++++- kernel/trace/trace_uprobe.c | 44 ++++--- 4 files changed, 348 insertions(+), 124 deletions(-)
kernel/trace/trace_kprobe.c +46 -31
@@ -441,13 +441,8 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)else ret =register_kprobe (&tk->rp.kp); -if (ret ==0 ) { +if (ret ==0 ) tk->tp.flags |=TP_FLAG_REGISTERED ; - }else if (ret == -EILSEQ ) { -pr_warn ("Probing address(0x%p) is not an instruction boundary.\n" , - tk->rp.kp.addr); - ret = -EINVAL ; - }return ret; }
@@ -591,7 +586,7 @@ static int trace_kprobe_create(int argc, const char *argv[])* Type of args: * FETCHARG:TYPE : use TYPE instead of unsigned long. */ -struct trace_kprobe *tk; +struct trace_kprobe *tk =NULL ;int i, len, ret =0 ;bool is_return = false;char *symbol =NULL , *tmp =NULL ;
@@ -615,44 +610,50 @@ static int trace_kprobe_create(int argc, const char *argv[])if (argc <2 )return -ECANCELED ; +trace_probe_log_init ("trace_kprobe" , argc, argv); + event =strchr (&argv[0 ][1 ],':' );if (event) event++;if (isdigit (argv[0 ][1 ])) {if (!is_return) { -pr_info ("Maxactive is not for kprobe" ); -return -EINVAL ; +trace_probe_log_err (1 ,MAXACT_NO_KPROBE ); + goto parse_error; }if (event) len = event - &argv[0 ][1 ] -1 ;else len =strlen (&argv[0 ][1 ]); -if (len >MAX_EVENT_NAME_LEN -1 ) -return -E2BIG ; +if (len >MAX_EVENT_NAME_LEN -1 ) { +trace_probe_log_err (1 ,BAD_MAXACT ); + goto parse_error; + }memcpy (buf, &argv[0 ][1 ], len); buf[len] ='\0' ; ret =kstrtouint (buf,0 , &maxactive);if (ret || !maxactive) { -pr_info ("Invalid maxactive number\n" ); -return ret; +trace_probe_log_err (1 ,BAD_MAXACT ); + goto parse_error; }/* kretprobes instances are iterated over via a list. The * maximum should stay reasonable. */ if (maxactive >KRETPROBE_MAXACTIVE_MAX ) { -pr_info ("Maxactive is too big (%d > %d).\n" , - maxactive,KRETPROBE_MAXACTIVE_MAX ); -return -E2BIG ; +trace_probe_log_err (1 ,MAXACT_TOO_BIG ); + goto parse_error; } }/* try to parse an address. if that fails, try to read the * input as a symbol. */ if (kstrtoul (argv[1 ],0 , (unsigned long *)&addr)) { +trace_probe_log_set_index (1 );/* Check whether uprobe event specified */ -if (strchr (argv[1 ],'/' ) &&strchr (argv[1 ],':' )) -return -ECANCELED ; +if (strchr (argv[1 ],'/' ) &&strchr (argv[1 ],':' )) { + ret = -ECANCELED ; + goto error; + }/* a symbol specified */ symbol =kstrdup (argv[1 ],GFP_KERNEL );if (!symbol)
@@ -660,23 +661,23 @@ static int trace_kprobe_create(int argc, const char *argv[])/* TODO: support .init module functions */ ret =traceprobe_split_symbol_offset (symbol, &offset);if (ret || offset <0 || offset >UINT_MAX ) { -pr_info ("Failed to parse either an address or a symbol.\n" ); - goto out; +trace_probe_log_err (0 ,BAD_PROBE_ADDR ); + goto parse_error; }if (kprobe_on_func_entry (NULL , symbol, offset)) flags |=TPARG_FL_FENTRY ;if (offset && is_return && !(flags &TPARG_FL_FENTRY )) { -pr_info ("Given offset is not valid for return probe.\n" ); - ret = -EINVAL ; - goto out; +trace_probe_log_err (0 ,BAD_RETPROBE ); + goto parse_error; } } - argc -=2 ; argv +=2 ; +trace_probe_log_set_index (0 );if (event) { - ret =traceprobe_parse_event_name (&event, &group, buf); + ret =traceprobe_parse_event_name (&event, &group, buf, + event - argv[0 ]);if (ret) - goto out; + goto parse_error; }else {/* Make a new event name */ if (symbol)
@@ -691,13 +692,14 @@ static int trace_kprobe_create(int argc, const char *argv[])/* setup a probe */ tk =alloc_trace_kprobe (group, event, addr, symbol, offset, maxactive, - argc, is_return); + argc -2 , is_return);if (IS_ERR (tk)) { ret =PTR_ERR (tk); -/* This must return -ENOMEM otherwise there is a bug */ +/* This must return -ENOMEM, else there is a bug */ WARN_ON_ONCE (ret != -ENOMEM ); - goto out; + goto out;/* We know tk is not allocated */ } + argc -=2 ; argv +=2 ;/* parse arguments */ for (i =0 ; i < argc && i <MAX_TRACE_ARGS ; i++) {
@@ -707,19 +709,32 @@ static int trace_kprobe_create(int argc, const char *argv[]) goto error; } +trace_probe_log_set_index (i +2 ); ret =traceprobe_parse_probe_arg (&tk->tp, i, tmp, flags);kfree (tmp);if (ret) - goto error; + goto error;/* This can be -ENOMEM */ } ret =register_trace_kprobe (tk); -if (ret) +if (ret) { +trace_probe_log_set_index (1 ); +if (ret == -EILSEQ ) +trace_probe_log_err (0 ,BAD_INSN_BNDRY ); +else if (ret == -ENOENT ) +trace_probe_log_err (0 ,BAD_PROBE_ADDR ); +else if (ret != -ENOMEM ) +trace_probe_log_err (0 ,FAIL_REG_PROBE ); goto error; + } + out: +trace_probe_log_clear ();kfree (symbol);return ret; +parse_error: + ret = -EINVAL ; error:free_trace_kprobe (tk); goto out;
kernel/trace/trace_probe.c +196 -78
522 lines not on this page · raw message
kernel/trace/trace_probe.h +75 -2
86 lines not on this page · raw message
kernel/trace/trace_uprobe.c +31 -13
107 lines not on this page · raw message
Signature · 1 line
2.14.1
PATCH v5 6/11 selftests/ftrace: Add error_log testcase for probe errors
In reply to Tom ZanussiFrom: Masami Hiramatsu <[email protected]>
Add error_log testcase for error logs on probe events. This tests most of error cases and checks the error position is correct.
--- .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 93 ++++++++++++++++++++++ .../ftrace/test.d/kprobe/uprobe_syntax_errors.tc | 31 ++++++++ 2 files changed, 124 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.tc
tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc new file +93 -0
@@ -0,0 +1,93 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: Kprobe event parser error log check + +[ -f kprobe_events ] || exit_unsupported # this is configurable + +[ -f error_log ] || exit_unsupported + +check_error() { # command-with-error-pos-by-^ +pos=$(echo -n "${1%^*}" | wc -c) # error position +command=$(echo "$1" | tr -d ^) +echo "Test command: $command" +echo > error_log +(! echo "$command" > kprobe_events ) 2> /dev/null +grep "trace_kprobe: error:" -A 3 error_log +N=$(tail -n 1 error_log | wc -c) +# " Command: " and "^\n" => 13 +test $(expr 13 + $pos) -eq $N +} + +if grep -q 'r\[maxactive\]' README; then +check_error 'p^100 vfs_read' # MAXACT_NO_KPROBE +check_error 'r^1a111 vfs_read' # BAD_MAXACT +check_error 'r^100000 vfs_read' # MAXACT_TOO_BIG +fi + +check_error 'p ^non_exist_func' # BAD_PROBE_ADDR (enoent) +check_error 'p ^hoge-fuga' # BAD_PROBE_ADDR (bad syntax) +check_error 'p ^hoge+1000-1000' # BAD_PROBE_ADDR (bad syntax) +check_error 'r ^vfs_read+10' # BAD_RETPROBE +check_error 'p:^/bar vfs_read' # NO_GROUP_NAME +check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG + +check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME +check_error 'p:foo/^ vfs_read' # NO_EVENT_NAME +check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG +check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME + +check_error 'p vfs_read ^$retval' # RETVAL_ON_PROBE +check_error 'p vfs_read ^$stack10000' # BAD_STACK_NUM + +if grep -q '$arg<N>' README; then +check_error 'p vfs_read ^$arg10000' # BAD_ARG_NUM +fi + +check_error 'p vfs_read ^$none_var' # BAD_VAR + +check_error 'p vfs_read ^%none_reg' # BAD_REG_NAME +check_error 'p vfs_read ^@12345678abcde' # BAD_MEM_ADDR +check_error 'p vfs_read ^@+10' # FILE_ON_KPROBE + +check_error 'p vfs_read ^+0@0)' # DEREF_NEED_BRACE +check_error 'p vfs_read ^+0ab1(@0)' # BAD_DEREF_OFFS +check_error 'p vfs_read +0(+0(@0^)' # DEREF_OPEN_BRACE + +if grep -A1 "fetcharg:" README | grep -q '\$comm' ; then +check_error 'p vfs_read +0(^$comm)' # COMM_CANT_DEREF +fi + +check_error 'p vfs_read ^&1' # BAD_FETCH_ARG + + +# We've introduced this limitation with array support +if grep -q ' <type>\\\[<array-size>\\\]' README; then +check_error 'p vfs_read +0(^+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(+0(@0))))))))))))))' # TOO_MANY_OPS? +check_error 'p vfs_read +0(@11):u8[10^' # ARRAY_NO_CLOSE +check_error 'p vfs_read +0(@11):u8[10]^a' # BAD_ARRAY_SUFFIX +check_error 'p vfs_read +0(@11):u8[^10a]' # BAD_ARRAY_NUM +check_error 'p vfs_read +0(@11):u8[^256]' # ARRAY_TOO_BIG +fi + +check_error 'p vfs_read @11:^unknown_type' # BAD_TYPE +check_error 'p vfs_read $stack0:^string' # BAD_STRING +check_error 'p vfs_read @11:^b10@a/16' # BAD_BITFIELD + +check_error 'p vfs_read ^arg123456789012345678901234567890=@11' # ARG_NAME_TOO_LOG +check_error 'p vfs_read ^=@11' # NO_ARG_NAME +check_error 'p vfs_read ^var.1=@11' # BAD_ARG_NAME +check_error 'p vfs_read var1=@11 ^var1=@12' # USED_ARG_NAME +check_error 'p vfs_read ^+1234567(+1234567(+1234567(+1234567(+1234567(+1234567(@1234))))))' # ARG_TOO_LONG +check_error 'p vfs_read arg1=^' # NO_ARG_BODY + +# instruction boundary check is valid on x86 (at this moment) +case $(uname -m) in + x86_64|i[3456]86) + echo 'p vfs_read' > kprobe_events + if grep -q FTRACE ../kprobes/list ; then + check_error 'p ^vfs_read+3' # BAD_INSN_BNDRY (only if function-tracer is enabled) + fi + ;; +esac + +exit 0
tools/testing/selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.tc new file +31 -0
@@ -0,0 +1,31 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: Uprobe event parser error log check + +[ -f uprobe_events ] || exit_unsupported # this is configurable + +[ -f error_log ] || exit_unsupported + +check_error() { # command-with-error-pos-by-^ +pos=$(echo -n "${1%^*}" | wc -c) # error position +command=$(echo "$1" | tr -d ^) +echo "Test command: $command" +echo > error_log +(! echo "$command" > uprobe_events ) 2> /dev/null +grep "trace_uprobe: error:" -A 3 error_log +N=$(tail -n 1 error_log | wc -c) +# " Command: " and "^\n" => 13 +test $(expr 13 + $pos) -eq $N +} + +check_error 'p ^/non_exist_file:100' # FILE_NOT_FOUND +check_error 'p ^/sys:100' # NO_REGULAR_FILE +check_error 'p /bin/sh:^10a' # BAD_UPROBE_OFFS +check_error 'p /bin/sh:10(^1a)' # BAD_REFCNT +check_error 'p /bin/sh:10(10^' # REFCNT_OPEN_BRACE +check_error 'p /bin/sh:10(10)^a' # BAD_REFCNT_SUFFIX + +check_error 'p /bin/sh:10 ^@+ab' # BAD_FILE_OFFS +check_error 'p /bin/sh:10 ^@symbol' # SYM_ON_UPROBE + +exit 0
Signature · 1 line
2.14.1
PATCH v5 7/11 selftests/ftrace: Move kprobe/uprobe check_error() to test.d/functions
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
The k/uprobe_sytax_errors test case defines a check_error() function used to run a command and check the position of the caret in the output.
This would be useful for other ftrace facilities too, so move it to test.d/functions for use by anyone. In the process, rename it to ftrace_errlog_check() and parametrize it for general use.
--- tools/testing/selftests/ftrace/test.d/functions | 12 ++++++++++++ .../selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 10 +--------- .../selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.tc | 10 +--------- 3 files changed, 14 insertions(+), 18 deletions(-)
tools/testing/selftests/ftrace/test.d/functions +12 -0
@@ -109,3 +109,15 @@ LOCALHOST=127.0.0.1 yield() { ping $LOCALHOST -c 1 || sleep .001 || usleep 1 || sleep 1 } + +ftrace_errlog_check() { # err-prefix command-with-error-pos-by-^ command-file + pos=$(echo -n "${2%^*}" | wc -c) # error position + command=$(echo "$2" | tr -d ^) + echo "Test command: $command" + echo > error_log + (! echo "$command" > "$3" ) 2> /dev/null + grep "$1: error:" -A 3 error_log + N=$(tail -n 1 error_log | wc -c) + # " Command: " and "^\n" => 13 + test $(expr 13 + $pos) -eq $N +}
tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc +1 -9
@@ -7,15 +7,7 @@ [ -f error_log ] || exit_unsupported check_error() { # command-with-error-pos-by-^ -pos=$(echo -n "${1%^*}" | wc -c) # error position -command=$(echo "$1" | tr -d ^) -echo "Test command: $command" -echo > error_log -(! echo "$command" > kprobe_events ) 2> /dev/null -grep "trace_kprobe: error:" -A 3 error_log -N=$(tail -n 1 error_log | wc -c) -# " Command: " and "^\n" => 13 -test $(expr 13 + $pos) -eq $N + ftrace_errlog_check 'trace_kprobe' "$1" 'kprobe_events' } if grep -q 'r\[maxactive\]' README; then
tools/testing/selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.tc +1 -9
@@ -7,15 +7,7 @@ [ -f error_log ] || exit_unsupported check_error() { # command-with-error-pos-by-^ -pos=$(echo -n "${1%^*}" | wc -c) # error position -command=$(echo "$1" | tr -d ^) -echo "Test command: $command" -echo > error_log -(! echo "$command" > uprobe_events ) 2> /dev/null -grep "trace_uprobe: error:" -A 3 error_log -N=$(tail -n 1 error_log | wc -c) -# " Command: " and "^\n" => 13 -test $(expr 13 + $pos) -eq $N + ftrace_errlog_check 'trace_uprobe' "$1" 'uprobe_events' } check_error 'p ^/non_exist_file:100' # FILE_NOT_FOUND
Signature · 1 line
2.14.1
PATCH v5 8/11 selftests/ftrace: Remove trigger-extended-error-support testcase
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
Error handling has been moved to the common tracing/error_log, so this test is no longer valid.
--- .../inter-event/trigger-extended-error-support.tc | 28 ---------------------- 1 file changed, 28 deletions(-) delete mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-extended-error-support.tc
tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-extended-error-support.tc deleted +0 -28
@@ -1,28 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0 -# description: event trigger - test extended error support - - -fail() { #msg - echo $1 - exit_fail -} - -if [ ! -f set_event ]; then - echo "event tracing is not supported" - exit_unsupported -fi - -if [ ! -f synthetic_events ]; then - echo "synthetic event is not supported" - exit_unsupported -fi - -echo "Test extended error support" -echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="ping"' > events/sched/sched_wakeup/trigger -! echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="ping"' >> events/sched/sched_wakeup/trigger 2> /dev/null -if ! grep -q "ERROR:" events/sched/sched_wakeup/hist; then - fail "Failed to generate extended error in histogram" -fi - -exit 0
Signature · 1 line
2.14.1
PATCH v5 9/11 selftests/ftrace: Add tracing/error_log testcase
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
Add a testcase verifying basic tracing/error_log functionality.
--- .../ftrace/test.d/ftrace/tracing-error-log.tc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc
tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc new file +19 -0
@@ -0,0 +1,19 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: ftrace - test tracing error log support + +fail() { #msg + echo $1 + exit_fail +} + +# event tracing is currently the only ftrace tracer that uses the +# tracing error_log, hence this check +if [ ! -f set_event ]; then + echo "event tracing is not supported" + exit_unsupported +fi + +ftrace_errlog_check 'event filter parse error' '((sig >= 10 && sig < 15) || dsig ^== 17) && comm != bash' 'events/signal/signal_generate/filter' + +exit 0
Signature · 1 line
2.14.1
PATCH v5 10/11 tracing: Add tracing/error_log Documentation
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
Move most of the hist trigger extended error documentation to ftrace.rst and expand on it to fully document tracing/error_log.
--- Documentation/trace/ftrace.rst | 31 +++++++++++++++++++++++++++++++ Documentation/trace/histogram.rst | 16 ++-------------- 2 files changed, 33 insertions(+), 14 deletions(-)
Documentation/trace/ftrace.rst +31 -0
@@ -765,6 +765,37 @@ Here is the list of current tracers that may be configured. tracers from tracing simply echo "nop" into current_tracer. +Error conditions +---------------- + + For most ftrace commands, failure modes are obvious and communicated + using standard return codes. + + For other more involved commands, extended error information may be + available via the tracing/error_log file. For the commands that + support it, reading the tracing/error_log file after an error will + display more detailed information about what went wrong, if + information is available. The tracing/error_log file is a circular + error log displaying a small number (currently, 8) of ftrace errors + for the last (8) failed commands. + + The extended error information and usage takes the form shown in + this example:: + + # echo xxx > /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger + echo: write error: Invalid argument + + # cat /sys/kernel/debug/tracing/error_log + [ 5348.887237] location: error: Couldn't yyy: zzz + Command: xxx + ^ + [ 7517.023364] location: error: Bad rrr: sss + Command: ppp qqq + ^ + + To clear the error log, echo the empty string into it:: + + # echo > /sys/kernel/debug/tracing/error_log Examples of using the tracer ----------------------------
Documentation/trace/histogram.rst +2 -14
@@ -199,20 +199,8 @@ Extended error information For some error conditions encountered when invoking a hist trigger command, extended error information is available via the - corresponding event's 'hist' file. Reading the hist file after an - error will display more detailed information about what went wrong, - if information is available. This extended error information will - be available until the next hist trigger command for that event. - - If available for a given error condition, the extended error - information and usage takes the following form:: - - # echo xxx > /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger - echo: write error: Invalid argument - - # cat /sys/kernel/debug/tracing/events/sched/sched_wakeup/hist - ERROR: Couldn't yyy: zzz - Last command: xxx + tracing/error_log file. See Error Conditions in + :file:`Documentation/trace/ftrace.rst` for details. 6.2 'hist' trigger examples ---------------------------
Signature · 1 line
2.14.1
PATCH v5 11/11 tracing: Add error_log to README
In reply to Tom ZanussiFrom: Tom Zanussi <[email protected]>
Add brief blurb about error_log to the 'Important files' section.
--- kernel/trace/trace.c | 1 + 1 file changed, 1 insertion(+)
kernel/trace/trace.c +1 -0
@@ -4700,6 +4700,7 @@ static const char readme_msg[] =" trace_pipe\t\t- A consuming read to see the contents of the buffer\n" " current_tracer\t- function and latency tracers\n" " available_tracers\t- list of configured tracers for current_tracer\n" +" error_log\t- error log for failed commands (that support it)\n" " buffer_size_kb\t- view and modify size of per cpu buffer\n" " buffer_total_size_kb - view total size of all cpu buffers\n\n" " trace_clock\t\t-change the clock used to order events\n"
Signature · 1 line
2.14.1
Hi Tom,
Quoted 9 lines · On Sun, 31 Mar 2019 18:48:14 -0500 Tom Zanussi <[email protected]> wrote:
From: Tom Zanussi <[email protected]>
Hi,
This is v5 of the frace
error_logpatchset. This version updates the patches according to suggestions from Masami and Namhyung and moves some of the selftest code from the kprobe/uprobe testcases to a common function that can be reused by multiple testcases, including the newerror_logtestcase.
Thanks for updating!
This series looks good to me.
BTW, could you also add a testcase which tests all (or most of) error cases of hist_err() ?
That will be good to find regressions in future changes.
Thanks,
Quoted 188 lines
Changes from v4:
- Merged in the >& -> 2> redirection patch into '[PATCH v5 06/11] selftests/ftrace: Add
error_logtestcase for probe errors'
- Added new patch, '[PATCH v5 07/11] selftests/ftrace: Move kprobe/uprobecheck_error()to test.d/functions' that adds a newftrace_errlog_check()to test.d/functions, which is based oncheck_error()from '[PATCH v5 06/11] selftests/ftrace: Adderror_logtestcase for probe errors'
- Modified the kprobe/uprobe test cases to use the commonftrace_errlog_check()
- Modified theerror_logtest cases to display the error position using the commonftrace_errlog_check()
- Updated subject for '[PATCH v5 08/11] selftests/ftrace: Remove trigger-extended-error-support testcase
- Streamlined theerror_logopen and write file operations as suggested by Masami
- Fixed the long lines in '[PATCH v4 04/11] tracing: Use tracingerror_logwith trace event filters' as suggested by NamhyungChanges from v3:
- Added Masami's [PATCH 05/11] tracing: Use tracing
error_logwith probe events
- Added Masami's [PATCH 06/11] selftests/ftrace: Adderror_logtestcase for probe
- Added [PATCH 11/11] to fix [PATCH 06/11]
- Removed [RFC PATCH v3 4/5] tracing: Use tracingerror_logwith kprobe events
- Added [PATCH 08/11] selftests/ftrace: Add tracing/error_log testcase
- Removed trigger-extended-error-support testcase
- Changed [n] numbering to [timestamp] numbering as suggested by Masami
- Updated Documentation and READMEChanges from v2:
- Added [n] numbering as suggested by Masami
Text from original post:
Last April, I posted an RFC patchset [1] implementing a common
error_loginterface as suggested by Masami [2]. We were supposed to discuss it at Plumbers but that never happened, and Steve recently asked about patches for a follow-on discussion [3], so here they are.I incorporated comments from the previous discussion, the most important of which are:
- Incorporated Steve's suggestion of using static strings as in the existing trace event filter code, along with
err_infoindexing into the string arrays and a position for the error caret.- Converted all the hist trigger errors and the existing trace event filter parse errors to use the new interface.
- Converted a few
kprobe_eventerrors to the new interface as examples, but these will require more work - I didn't spend much time figuring out how to get the full kprobe command into the error info, for instance.- Got rid of the custom single-page ring buffer and used standard lists instead.
For now, this is implemented on top of the latest 'hist trigger snapshot and onchange additions' patchset [4].
Below is an example session of a few failed commands and the corresponding
error_logcontents:# echo > /sys/kernel/debug/tracing/error_log# echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger -su: echo: write error: Invalid argument# cat
/sys/kernel/debug/tracing/error_log[ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^# echo 'hist:key=comm:p=prio:onchange($q).snapshot()' > /sys/kernel/debug/tracing/events/sched/sched_waking/trigger -su: echo: write error: Invalid argument# cat /sys/kernel/debug/tracing/error_log [ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^ [ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable Command: key=comm:p=prio:onchange($q).snapshot() ^# echo 'hist:keys=pid' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger # echo 'hist:keys=pid' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger -su: echo: write error: File exists # echo 'comm="cyclictest"' > /sys/kernel/debug/tracing/events/sched/sched_wakeup/filter -su: echo: write error: Invalid argument# cat /sys/kernel/debug/tracing/error_log [ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^ [ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable Command: key=comm:p=prio:onchange($q).snapshot() ^ [ 715.626153] hist:sched:sched_wakeup: error: Hist trigger already exists Command: keys=pid ^ [ 730.480310] event filter parse error: error: Invalid operator Command: comm="cyclictest" ^# echo "((sig >= 10 && sig < 15) || dsig == 17) && comm != bash" > /sys/kernel/debug/tracing/events/signal/signal_generate/filter -su: echo: write error: Invalid argument# cat /sys/kernel/debug/tracing/error_log [ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^ [ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable Command: key=comm:p=prio:onchange($q).snapshot() ^ [ 715.626153] hist:sched:sched_wakeup: error: Hist trigger already exists Command: keys=pid ^ [ 730.480310] event filter parse error: error: Invalid operator Command: comm="cyclictest" ^ [ 800.548673] event filter parse error: error: Field not found Command: ((sig >= 10 && sig < 15) || dsig == 17) && comm != bash ^Thanks,
Tom
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/
[4] https://lore.kernel.org/lkml/[email protected]/The following changes since commit
01c4e8042fc949018b70ded769f959bc3d80e8b6:tracing: initialize variable in
create_dyn_event()(2019-03-22 15:19:34 -0400)are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.gitftrace/error_log_v5Masami Hiramatsu (2):
tracing: Use tracingerror_logwith probe events selftests/ftrace: Adderror_logtestcase for probe errorsTom Zanussi (9):
tracing: Add tracing error log
tracing: Save the last hist command's associated event name
tracing: Use tracingerror_logwith hist triggers
tracing: Use tracingerror_logwith trace event filters selftests/ftrace: Move kprobe/uprobecheck_error()to test.d/functions selftests/ftrace: Remove trigger-extended-error-support testcase selftests/ftrace: Add tracing/error_log testcase
tracing: Add tracing/error_log Documentation
tracing: Adderror_logto READMEDocumentation/trace/ftrace.rst | 31 +++ Documentation/trace/histogram.rst | 16 +- kernel/trace/trace.c | 219 ++++++++++++++++ kernel/trace/trace.h | 4 + kernel/trace/trace_events_filter.c | 11 +- kernel/trace/trace_events_hist.c | 221 +++++++++-------- kernel/trace/trace_kprobe.c | 77 +++--- kernel/trace/trace_probe.c | 274 +++++++++++++++------ kernel/trace/trace_probe.h | 77 +++++- kernel/trace/trace_uprobe.c | 44 +++- .../ftrace/test.d/ftrace/tracing-error-log.tc | 19 ++ tools/testing/selftests/ftrace/test.d/functions | 12 + .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 85 +++++++ .../ftrace/test.d/kprobe/uprobe_syntax_errors.tc | 23 ++ .../inter-event/trigger-extended-error-support.tc | 28 --- 15 files changed, 874 insertions(+), 267 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.tc delete mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-extended-error-support.tcSignature · 1 line
2.14.1
Signature · 1 line
Masami Hiramatsu <[email protected]>
Hi Masami,
Quoted 18 lines · Masami Hiramatsu
Hi Tom,
Quoted 4 lines · On Sun, 31 Mar 2019 18:48:14 -0500 Tom Zanussi <[email protected]> wrote:
From: Tom Zanussi <[email protected]>
Hi,
Quoted 8 lines
This is v5 of the frace
error_logpatchset. This version updates the
patches according to suggestions from Masami and Namhyung and moves some of the selftest code from the kprobe/uprobe testcases to a common
function that can be reused by multiple testcases, including the newerror_logtestcase.
Quoted 6 lines
Thanks for updating!
This series looks good to me.
Acked-by Masami Hiramatsu <[email protected]>
Thanks!
Quoted 4 lines
BTW, could you also add a testcase which tests all (or most of) error cases of
hist_err()?
That will be good to find regressions in future changes.
I was going to, but there are a lot of error cases and a lot of them aren't simple one-liners and require some more involved setup to trigger. I'll submit a follow-on patch to add them as soon as I get the chance.
Thanks,
Tom
Quoted 245 lines
Thanks,
Quoted 241 lines
Changes from v4:
- Merged in the >& -> 2> redirection patch into '[PATCH v5 06/11] selftests/ftrace: Add
error_logtestcase for probe errors'
- Added new patch, '[PATCH v5 07/11] selftests/ftrace: Move kprobe/uprobecheck_error()to test.d/functions' that adds a
new
ftrace_errlog_check()to test.d/functions, which is based oncheck_error()from '[PATCH v5 06/11] selftests/ftrace: Adderror_logtestcase for probe errors'
- Modified the kprobe/uprobe test cases to use the commonftrace_errlog_check()
- Modified theerror_logtest cases to display the error position using the commonftrace_errlog_check()
- Updated subject for '[PATCH v5 08/11] selftests/ftrace: Remove trigger-extended-error-support testcase
- Streamlined theerror_logopen and write file operations as suggested by Masami
- Fixed the long lines in '[PATCH v4 04/11] tracing: Use tracingerror_logwith trace event filters' as suggested by NamhyungChanges from v3:
- Added Masami's [PATCH 05/11] tracing: Use tracing
error_log
with probe events
- Added Masami's [PATCH 06/11] selftests/ftrace: Adderror_log
testcase for probe
- Added [PATCH 11/11] to fix [PATCH 06/11]
- Removed [RFC PATCH v3 4/5] tracing: Use tracingerror_logwith
kprobe events
- Added [PATCH 08/11] selftests/ftrace: Add tracing/error_log
testcase
- Removed trigger-extended-error-support testcase
- Changed [n] numbering to [timestamp] numbering as suggested by
Masami
- Updated Documentation and READMEChanges from v2:
- Added [n] numbering as suggested by Masami
Text from original post:
Last April, I posted an RFC patchset [1] implementing a common
error_loginterface as suggested by Masami [2]. We were supposed to
discuss it at Plumbers but that never happened, and Steve recently asked about patches for a follow-on discussion [3], so here they are.I incorporated comments from the previous discussion, the most important of which are:
- Incorporated Steve's suggestion of using static strings as in
the
existing trace event filter code, along witherr_infoindexing
into
the string arrays and a position for the error caret.- Converted all the hist trigger errors and the existing trace
event
filter parse errors to use the new interface.- Converted a few
kprobe_eventerrors to the new interface as examples, but these will require more work - I didn't spend much time figuring out how to get the full kprobe command into the
error
info, for instance.- Got rid of the custom single-page ring buffer and used standard lists instead.
For now, this is implemented on top of the latest 'hist trigger snapshot and onchange additions' patchset [4].
Below is an example session of a few failed commands and the corresponding
error_logcontents:# echo > /sys/kernel/debug/tracing/error_log# echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger -su: echo: write error: Invalid argument# cat
/sys/kernel/debug/tracing/error_log[ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^# echo 'hist:key=comm:p=prio:onchange($q).snapshot()' > /sys/kernel/debug/tracing/events/sched/sched_waking/trigger -su: echo: write error: Invalid argument# cat /sys/kernel/debug/tracing/error_log [ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^ [ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable Command: key=comm:p=prio:onchange($q).snapshot() ^# echo 'hist:keys=pid' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger # echo 'hist:keys=pid' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger -su: echo: write error: File exists # echo 'comm="cyclictest"' > /sys/kernel/debug/tracing/events/sched/sched_wakeup/filter -su: echo: write error: Invalid argument# cat /sys/kernel/debug/tracing/error_log [ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^ [ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable Command: key=comm:p=prio:onchange($q).snapshot() ^ [ 715.626153] hist:sched:sched_wakeup: error: Hist trigger already exists Command: keys=pid ^ [ 730.480310] event filter parse error: error: Invalid operator Command: comm="cyclictest" ^# echo "((sig >= 10 && sig < 15) || dsig == 17) && comm != bash"Quoted 1 line
/sys/kernel/debug/tracing/events/signal/signal_generate/filter-su: echo: write error: Invalid argument
# cat /sys/kernel/debug/tracing/error_log [ 217.431858] hist:sched:sched_wakeup: error: Variable already defined Command: keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest" ^ [ 311.554978] hist:sched:sched_waking: error: Couldn't find onmax or onchange variable Command: key=comm:p=prio:onchange($q).snapshot() ^ [ 715.626153] hist:sched:sched_wakeup: error: Hist trigger already exists Command: keys=pid ^ [ 730.480310] event filter parse error: error: Invalid operator Command: comm="cyclictest" ^ [ 800.548673] event filter parse error: error: Field not found Command: ((sig >= 10 && sig < 15) || dsig == 17) && comm != bash ^Thanks,
Tom
[1] https://lore.kernel.org/lkml/cover.1523545519.git.tom.zanussi@l
inux.intel.com/
[2] https://lore.kernel.org/lkml/20180406105309.b50ea1a21d2cbd9b0e3
[email protected]/
[3] https://lore.kernel.org/lkml/e885d1fd02e76f121d6cc2bb28e58b523e
[email protected]/
[4] https://lore.kernel.org/lkml/cover.1549403369.git.tom.zanussi@l
inux.intel.com/The following changes since commit
01c4e8042fc949018b70ded769f959bc3d80e8b6:tracing: initialize variable in
create_dyn_event()(2019-03-22
15:19:34 -0400)are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-
trace.git ftrace/error_log_v5Masami Hiramatsu (2):
tracing: Use tracingerror_logwith probe events selftests/ftrace: Adderror_logtestcase for probe errorsTom Zanussi (9):
tracing: Add tracing error log
tracing: Save the last hist command's associated event name
tracing: Use tracingerror_logwith hist triggers
tracing: Use tracingerror_logwith trace event filters selftests/ftrace: Move kprobe/uprobecheck_error()to
test.d/functions
selftests/ftrace: Remove trigger-extended-error-support testcase selftests/ftrace: Add tracing/error_log testcase
tracing: Add tracing/error_log Documentation
tracing: Adderror_logto READMEDocumentation/trace/ftrace.rst | 31 +++ Documentation/trace/histogram.rst | 16 +- kernel/trace/trace.c | 219 ++++++++++++++++ kernel/trace/trace.h | 4 + kernel/trace/trace_events_filter.c | 11 +- kernel/trace/trace_events_hist.c | 221 +++++++++-------- kernel/trace/trace_kprobe.c | 77 +++--- kernel/trace/trace_probe.c | 274 +++++++++++++++------ kernel/trace/trace_probe.h | 77 +++++- kernel/trace/trace_uprobe.c | 44 +++- .../ftrace/test.d/ftrace/tracing-error-log.tc | 19 ++ tools/testing/selftests/ftrace/test.d/functions | 12 + .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 85 +++++++ .../ftrace/test.d/kprobe/uprobe_syntax_errors.tc | 23 ++ .../inter-event/trigger-extended-error-support.tc | 28 --- 15 files changed, 874 insertions(+), 267 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.t c create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/uprobe_syntax_errors.t c delete mode 100644
tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-extended-error-support.tcSignature · 1 line
2.14.1
Masami Hiramatsu
maintainerQuoted 10 lines · On Mon, 01 Apr 2019 08:31:08 -0500 Tom Zanussi <[email protected]> wrote:
Quoted 5 lines
BTW, could you also add a testcase which tests all (or most of) error cases of
hist_err()?
That will be good to find regressions in future changes.I was going to, but there are a lot of error cases and a lot of them aren't simple one-liners and require some more involved setup to trigger. I'll submit a follow-on patch to add them as soon as I get the chance.
Agreed, I look forward your testcase :)
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>