* [PATCH v2 0/4] tracing: More synthetic event error fixes
@ 2020-10-23 20:33 Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function Tom Zanussi
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Tom Zanussi @ 2020-10-23 20:33 UTC (permalink / raw)
To: rostedt, axelrasmussen; +Cc: mhiramat, linux-kernel
This is v2 of the previous sythetic event error fix patchset.
This version drops the original ([PATCH 1/4] tracing: Make
trace_*_run_command() more flexible) and (tracing: Use new
trace_run_command() options) patches and replaces them with Masami's
patch (tracing/dynevent: Delegate parsing to create function) [1].
The new version adds in all the synthetic event changes needed to
compile and use the new interface.
A new patch was also added (selftests/ftrace: Add synthetic event
field separators) that fixes more invalid synthetic event syntax I
found while testing.
I also added some more new checks to the synthetic event sytax error
testcase.
As before, I didn't see any problems running the entire ftrace
testsuite or the test modules that also use the things that were
touched here.
[1] https://lore.kernel.org/lkml/20201019001504.70dc3ec608277ed22060d2f7@kernel.org/
Thanks,
Tom
v1 text:
Hi,
This patchset addresses the synthetic event error anomalies reported
by Masami in the last patchset [1].
It turns out that most of the problems boil down to clunky separator
parsing; adding a couple new abilities to trace_run_command() and then
adapting the existing users seemed to me the best way to fix these
things, and also gets rid of some code.
Also, to make things easier for error display, I changed these to
preserve the original command string and pass it through the callback
instead of rebuilding it for error display.
I added some new error strings and removed unused ones as well, and
added a bunch of new test cases to the synthetic parser error test
case.
I didn't see any problems running the entire ftrace testsuite or the
test modules that also use the things that were touched here.
Thanks,
Tom
[1] https://lore.kernel.org/lkml/20201014110636.139df7be275d40a23b523b84@kernel.org/
The following changes since commit ce66f6136460a51acfc32de4481fe8fd69dfd50b:
tracing: Remove __init from __trace_early_add_new_event() (2020-10-16 09:43:36 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/synth-fixes-v2
Masami Hiramatsu (1):
tracing/dynevent: Delegate parsing to create function
Tom Zanussi (3):
tracing: Update synth command errors
selftests/ftrace: Add synthetic event field separators
selftests/ftrace: Update synthetic event syntax errors
kernel/trace/trace.c | 23 +-
kernel/trace/trace.h | 3 +-
kernel/trace/trace_dynevent.c | 35 ++-
kernel/trace/trace_dynevent.h | 4 +-
kernel/trace/trace_events_synth.c | 246 +++++++++++-------
kernel/trace/trace_kprobe.c | 33 +--
kernel/trace/trace_probe.c | 17 ++
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 17 +-
.../trigger-inter-event-combined-hist.tc | 4 +-
.../trigger-onmatch-action-hist.tc | 2 +-
.../trigger-onmatch-onmax-action-hist.tc | 2 +-
.../trigger-synthetic_event_syntax_errors.tc | 36 ++-
.../inter-event/trigger-trace-action-hist.tc | 2 +-
14 files changed, 262 insertions(+), 163 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function
2020-10-23 20:33 [PATCH v2 0/4] tracing: More synthetic event error fixes Tom Zanussi
@ 2020-10-23 20:33 ` Tom Zanussi
2020-10-23 23:03 ` kernel test robot
2020-10-24 8:50 ` Masami Hiramatsu
2020-10-23 20:33 ` [PATCH v2 2/4] tracing: Update synth command errors Tom Zanussi
` (2 subsequent siblings)
3 siblings, 2 replies; 8+ messages in thread
From: Tom Zanussi @ 2020-10-23 20:33 UTC (permalink / raw)
To: rostedt, axelrasmussen; +Cc: mhiramat, linux-kernel
From: Masami Hiramatsu <mhiramat@kernel.org>
Delegate command parsing to each create function so that the
command syntax can be customized.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
[ zanussi@kernel.org: added synthetic event modifications ]
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
kernel/trace/trace.c | 23 +---
kernel/trace/trace.h | 3 +-
kernel/trace/trace_dynevent.c | 35 +++---
kernel/trace/trace_dynevent.h | 4 +-
kernel/trace/trace_events_synth.c | 186 ++++++++++++++++--------------
kernel/trace/trace_kprobe.c | 33 +++---
kernel/trace/trace_probe.c | 17 +++
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 17 ++-
9 files changed, 174 insertions(+), 145 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 63c97012ed39..277d97220971 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9367,30 +9367,11 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
}
EXPORT_SYMBOL_GPL(ftrace_dump);
-int trace_run_command(const char *buf, int (*createfn)(int, char **))
-{
- char **argv;
- int argc, ret;
-
- argc = 0;
- ret = 0;
- argv = argv_split(GFP_KERNEL, buf, &argc);
- if (!argv)
- return -ENOMEM;
-
- if (argc)
- ret = createfn(argc, argv);
-
- argv_free(argv);
-
- return ret;
-}
-
#define WRITE_BUFSIZE 4096
ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
size_t count, loff_t *ppos,
- int (*createfn)(int, char **))
+ int (*createfn)(const char *))
{
char *kbuf, *buf, *tmp;
int ret = 0;
@@ -9438,7 +9419,7 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
if (tmp)
*tmp = '\0';
- ret = trace_run_command(buf, createfn);
+ ret = createfn(buf);
if (ret)
goto out;
buf += size;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 34e0c4d5a6e7..02d7c487a30b 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1982,10 +1982,9 @@ extern int tracing_set_cpumask(struct trace_array *tr,
#define MAX_EVENT_NAME_LEN 64
-extern int trace_run_command(const char *buf, int (*createfn)(int, char**));
extern ssize_t trace_parse_run_command(struct file *file,
const char __user *buffer, size_t count, loff_t *ppos,
- int (*createfn)(int, char**));
+ int (*createfn)(const char *));
extern unsigned int err_pos(char *cmd, const char *str);
extern void tracing_log_err(struct trace_array *tr,
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 5fa49cfd2bb6..af83bc5447fe 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -31,23 +31,31 @@ int dyn_event_register(struct dyn_event_operations *ops)
return 0;
}
-int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type)
+int dyn_event_release(const char *raw_command, struct dyn_event_operations *type)
{
struct dyn_event *pos, *n;
char *system = NULL, *event, *p;
- int ret = -ENOENT;
+ int argc, ret = -ENOENT;
+ char **argv;
+
+ argv = argv_split(GFP_KERNEL, raw_command, &argc);
+ if (!argv)
+ return -ENOMEM;
if (argv[0][0] == '-') {
- if (argv[0][1] != ':')
- return -EINVAL;
+ if (argv[0][1] != ':') {
+ ret = -EINVAL;
+ goto out;
+ }
event = &argv[0][2];
} else {
event = strchr(argv[0], ':');
- if (!event)
- return -EINVAL;
+ if (!event) {
+ ret = -EINVAL;
+ goto out;
+ }
event++;
}
- argc--; argv++;
p = strchr(event, '/');
if (p) {
@@ -63,7 +71,7 @@ int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type)
if (type && type != pos->ops)
continue;
if (!pos->ops->match(system, event,
- argc, (const char **)argv, pos))
+ argc - 1, (const char **)argv + 1, pos))
continue;
ret = pos->ops->free(pos);
@@ -71,21 +79,22 @@ int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type)
break;
}
mutex_unlock(&event_mutex);
-
+out:
+ argv_free(argv);
return ret;
}
-static int create_dyn_event(int argc, char **argv)
+static int create_dyn_event(const char *raw_command)
{
struct dyn_event_operations *ops;
int ret = -ENODEV;
- if (argv[0][0] == '-' || argv[0][0] == '!')
- return dyn_event_release(argc, argv, NULL);
+ if (raw_command[0] == '-' || raw_command[0] == '!')
+ return dyn_event_release(raw_command, NULL);
mutex_lock(&dyn_event_ops_mutex);
list_for_each_entry(ops, &dyn_event_ops_list, list) {
- ret = ops->create(argc, (const char **)argv);
+ ret = ops->create(raw_command);
if (!ret || ret != -ECANCELED)
break;
}
diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
index d6857a254ede..4f4e03df4cbb 100644
--- a/kernel/trace/trace_dynevent.h
+++ b/kernel/trace/trace_dynevent.h
@@ -39,7 +39,7 @@ struct dyn_event;
*/
struct dyn_event_operations {
struct list_head list;
- int (*create)(int argc, const char *argv[]);
+ int (*create)(const char *raw_command);
int (*show)(struct seq_file *m, struct dyn_event *ev);
bool (*is_busy)(struct dyn_event *ev);
int (*free)(struct dyn_event *ev);
@@ -97,7 +97,7 @@ void *dyn_event_seq_start(struct seq_file *m, loff_t *pos);
void *dyn_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
void dyn_event_seq_stop(struct seq_file *m, void *v);
int dyn_events_release_all(struct dyn_event_operations *type);
-int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type);
+int dyn_event_release(const char *raw_command, struct dyn_event_operations *type);
/*
* for_each_dyn_event - iterate over the dyn_event list
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index 3212e2c653b3..90c1790ed1d6 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -48,7 +48,7 @@ static int errpos(const char *str)
return err_pos(last_cmd, str);
}
-static void last_cmd_set(char *str)
+static void last_cmd_set(const char *str)
{
if (!str)
return;
@@ -62,7 +62,7 @@ static void synth_err(u8 err_type, u8 err_pos)
err_type, err_pos);
}
-static int create_synth_event(int argc, const char **argv);
+static int create_synth_event(const char *raw_command);
static int synth_event_show(struct seq_file *m, struct dyn_event *ev);
static int synth_event_release(struct dyn_event *ev);
static bool synth_event_is_busy(struct dyn_event *ev);
@@ -579,17 +579,13 @@ static void free_synth_field(struct synth_field *field)
kfree(field);
}
-static struct synth_field *parse_synth_field(int argc, const char **argv,
- int *consumed)
+static struct synth_field *parse_synth_field(int argc, char **argv)
{
struct synth_field *field;
const char *prefix = NULL, *field_type = argv[0], *field_name, *array;
int len, ret = 0;
ssize_t size;
- if (field_type[0] == ';')
- field_type++;
-
if (!strcmp(field_type, "unsigned")) {
if (argc < 3) {
synth_err(SYNTH_ERR_INCOMPLETE_TYPE, errpos(field_type));
@@ -598,10 +594,12 @@ static struct synth_field *parse_synth_field(int argc, const char **argv,
prefix = "unsigned ";
field_type = argv[1];
field_name = argv[2];
- *consumed = 3;
- } else {
+ } else
field_name = argv[1];
- *consumed = 2;
+
+ if (!field_name) {
+ synth_err(SYNTH_ERR_INVALID_FIELD, errpos(field_type));
+ return ERR_PTR(-EINVAL);
}
field = kzalloc(sizeof(*field), GFP_KERNEL);
@@ -612,8 +610,6 @@ static struct synth_field *parse_synth_field(int argc, const char **argv,
array = strchr(field_name, '[');
if (array)
len -= strlen(array);
- else if (field_name[len - 1] == ';')
- len--;
field->name = kmemdup_nul(field_name, len, GFP_KERNEL);
if (!field->name) {
@@ -626,17 +622,11 @@ static struct synth_field *parse_synth_field(int argc, const char **argv,
goto free;
}
- if (field_type[0] == ';')
- field_type++;
len = strlen(field_type) + 1;
- if (array) {
- int l = strlen(array);
+ if (array)
+ len += strlen(array);
- if (l && array[l - 1] == ';')
- l--;
- len += l;
- }
if (prefix)
len += strlen(prefix);
@@ -648,15 +638,11 @@ static struct synth_field *parse_synth_field(int argc, const char **argv,
if (prefix)
strcat(field->type, prefix);
strcat(field->type, field_type);
- if (array) {
+ if (array)
strcat(field->type, array);
- if (field->type[len - 1] == ';')
- field->type[len - 1] = '\0';
- }
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) {
@@ -1155,46 +1141,12 @@ 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)
+static int __create_synth_event(const char *name, const char *raw_fields)
{
+ int i, argc, consumed = 0, n_fields = 0, ret = 0;
struct synth_field *field, *fields[SYNTH_FIELDS_MAX];
+ char **argv, *field_str, *tmp_fields, *saved_fields = NULL;
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:
@@ -1203,13 +1155,14 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
* where 'field' = type field_name
*/
- if (name[0] == '\0' || argc < 1) {
+ mutex_lock(&event_mutex);
+
+ if (name[0] == '\0') {
synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
- mutex_lock(&event_mutex);
-
if (!is_good_name(name)) {
synth_err(SYNTH_ERR_BAD_NAME, errpos(name));
ret = -EINVAL;
@@ -1223,26 +1176,43 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
goto out;
}
- for (i = 0; i < argc - 1; i++) {
- if (strcmp(argv[i], ";") == 0)
- continue;
+ tmp_fields = saved_fields = kstrdup(raw_fields, GFP_KERNEL);
+ if (!tmp_fields) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ while ((field_str = strsep(&tmp_fields, ";")) != NULL) {
if (n_fields == SYNTH_FIELDS_MAX) {
synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0);
ret = -EINVAL;
goto err;
}
- field = parse_synth_field(argc - i, &argv[i], &consumed);
+ argv = argv_split(GFP_KERNEL, field_str, &argc);
+ if (!argv) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ if (!argc)
+ continue;
+
+ field = parse_synth_field(argc, argv);
if (IS_ERR(field)) {
+ argv_free(argv);
ret = PTR_ERR(field);
goto err;
}
+
+ argv_free(argv);
+
fields[n_fields++] = field;
i += consumed - 1;
}
- if (i < argc && strcmp(argv[i], ";") != 0) {
- synth_err(SYNTH_ERR_INVALID_FIELD, errpos(argv[i]));
+ if (n_fields == 0) {
+ synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
ret = -EINVAL;
goto err;
}
@@ -1261,6 +1231,8 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
out:
mutex_unlock(&event_mutex);
+ kfree(saved_fields);
+
return ret;
err:
for (i = 0; i < n_fields; i++)
@@ -1378,18 +1350,35 @@ int synth_event_delete(const char *event_name)
}
EXPORT_SYMBOL_GPL(synth_event_delete);
-static int create_or_delete_synth_event(int argc, char **argv)
+static int create_or_delete_synth_event(const char *raw_command)
{
- const char *name = argv[0];
- int ret;
+ char **argv, *name = NULL, *fields;
+ int argc = 0, ret = 0;
+
+ last_cmd_set(raw_command);
+
+ argv = argv_split(GFP_KERNEL, raw_command, &argc);
+ if (!argv)
+ return -ENOMEM;
+
+ if (!argc)
+ goto free;
+
+ name = argv[0];
/* trace_run_command() ensures argc != 0 */
if (name[0] == '!') {
ret = synth_event_delete(name + 1);
- return ret;
+ goto free;
}
- ret = __create_synth_event(argc - 1, name, (const char **)argv + 1);
+ fields = strstr(raw_command, name);
+ fields += strlen(name);
+
+ ret = __create_synth_event(name, fields);
+free:
+ argv_free(argv);
+
return ret == -ECANCELED ? -EINVAL : ret;
}
@@ -1398,7 +1387,7 @@ static int synth_event_run_command(struct dynevent_cmd *cmd)
struct synth_event *se;
int ret;
- ret = trace_run_command(cmd->seq.buffer, create_or_delete_synth_event);
+ ret = create_or_delete_synth_event(cmd->seq.buffer);
if (ret)
return ret;
@@ -1934,23 +1923,48 @@ int synth_event_trace_end(struct synth_event_trace_state *trace_state)
}
EXPORT_SYMBOL_GPL(synth_event_trace_end);
-static int create_synth_event(int argc, const char **argv)
+static int create_synth_event(const char *raw_command)
{
- const char *name = argv[0];
- int len;
+ char **argv, *name, *fields;
+ int len, argc = 0, ret = 0;
+
+ last_cmd_set(raw_command);
+
+ argv = argv_split(GFP_KERNEL, raw_command, &argc);
+ if (!argv) {
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ if (!argc)
+ goto free;
- if (name[0] != 's' || name[1] != ':')
- return -ECANCELED;
+ name = argv[0];
+
+ if (name[0] != 's' || name[1] != ':') {
+ ret = -ECANCELED;
+ goto free;
+ }
name += 2;
/* This interface accepts group name prefix */
if (strchr(name, '/')) {
len = str_has_prefix(name, SYNTH_SYSTEM "/");
- if (len == 0)
- return -EINVAL;
+ if (len == 0) {
+ ret = -EINVAL;
+ goto free;
+ }
name += len;
}
- return __create_synth_event(argc - 1, name, argv + 1);
+
+ fields = strstr(raw_command, name);
+ fields += strlen(name);
+
+ ret = __create_synth_event(name, fields);
+free:
+ argv_free(argv);
+
+ return ret;
}
static int synth_event_release(struct dyn_event *ev)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b911e9f6d9f5..ddef93e32905 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -34,7 +34,7 @@ static int __init set_kprobe_boot_events(char *str)
}
__setup("kprobe_event=", set_kprobe_boot_events);
-static int trace_kprobe_create(int argc, const char **argv);
+static int trace_kprobe_create(const char *raw_command);
static int trace_kprobe_show(struct seq_file *m, struct dyn_event *ev);
static int trace_kprobe_release(struct dyn_event *ev);
static bool trace_kprobe_is_busy(struct dyn_event *ev);
@@ -710,7 +710,7 @@ static inline void sanitize_event_name(char *name)
*name = '_';
}
-static int trace_kprobe_create(int argc, const char *argv[])
+static int __trace_kprobe_create(int argc, const char *argv[])
{
/*
* Argument syntax:
@@ -907,20 +907,25 @@ static int trace_kprobe_create(int argc, const char *argv[])
goto out;
}
-static int create_or_delete_trace_kprobe(int argc, char **argv)
+static int trace_kprobe_create(const char *raw_command)
+{
+ return trace_probe_create(raw_command, __trace_kprobe_create);
+}
+
+static int create_or_delete_trace_kprobe(const char *raw_command)
{
int ret;
- if (argv[0][0] == '-')
- return dyn_event_release(argc, argv, &trace_kprobe_ops);
+ if (raw_command[0] == '-')
+ return dyn_event_release(raw_command, &trace_kprobe_ops);
- ret = trace_kprobe_create(argc, (const char **)argv);
+ ret = trace_kprobe_create(raw_command);
return ret == -ECANCELED ? -EINVAL : ret;
}
static int trace_kprobe_run_command(struct dynevent_cmd *cmd)
{
- return trace_run_command(cmd->seq.buffer, create_or_delete_trace_kprobe);
+ return create_or_delete_trace_kprobe(cmd->seq.buffer);
}
/**
@@ -1081,7 +1086,7 @@ int kprobe_event_delete(const char *name)
snprintf(buf, MAX_EVENT_NAME_LEN, "-:%s", name);
- return trace_run_command(buf, create_or_delete_trace_kprobe);
+ return create_or_delete_trace_kprobe(buf);
}
EXPORT_SYMBOL_GPL(kprobe_event_delete);
@@ -1884,7 +1889,7 @@ static __init void setup_boot_kprobe_events(void)
if (p)
*p++ = '\0';
- ret = trace_run_command(cmd, create_or_delete_trace_kprobe);
+ ret = create_or_delete_trace_kprobe(cmd);
if (ret)
pr_warn("Failed to add event(%d): %s\n", ret, cmd);
else
@@ -1982,8 +1987,7 @@ static __init int kprobe_trace_self_tests_init(void)
pr_info("Testing kprobe tracing: ");
- ret = trace_run_command("p:testprobe kprobe_trace_selftest_target $stack $stack0 +0($stack)",
- create_or_delete_trace_kprobe);
+ ret = create_or_delete_trace_kprobe("p:testprobe kprobe_trace_selftest_target $stack $stack0 +0($stack)");
if (WARN_ON_ONCE(ret)) {
pr_warn("error on probing function entry.\n");
warn++;
@@ -2004,8 +2008,7 @@ static __init int kprobe_trace_self_tests_init(void)
}
}
- ret = trace_run_command("r:testprobe2 kprobe_trace_selftest_target $retval",
- create_or_delete_trace_kprobe);
+ ret = create_or_delete_trace_kprobe("r:testprobe2 kprobe_trace_selftest_target $retval");
if (WARN_ON_ONCE(ret)) {
pr_warn("error on probing function return.\n");
warn++;
@@ -2078,13 +2081,13 @@ static __init int kprobe_trace_self_tests_init(void)
trace_probe_event_call(&tk->tp), file);
}
- ret = trace_run_command("-:testprobe", create_or_delete_trace_kprobe);
+ ret = create_or_delete_trace_kprobe("-:testprobe");
if (WARN_ON_ONCE(ret)) {
pr_warn("error on deleting a probe.\n");
warn++;
}
- ret = trace_run_command("-:testprobe2", create_or_delete_trace_kprobe);
+ ret = create_or_delete_trace_kprobe("-:testprobe2");
if (WARN_ON_ONCE(ret)) {
pr_warn("error on deleting a probe.\n");
warn++;
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index d2867ccc6aca..ec589a4612df 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1134,3 +1134,20 @@ bool trace_probe_match_command_args(struct trace_probe *tp,
}
return true;
}
+
+int trace_probe_create(const char *raw_command, int (*createfn)(int, const char **))
+{
+ int argc = 0, ret = 0;
+ char **argv;
+
+ argv = argv_split(GFP_KERNEL, raw_command, &argc);
+ if (!argv)
+ return -ENOMEM;
+
+ if (argc)
+ ret = createfn(argc, (const char **)argv);
+
+ argv_free(argv);
+
+ return ret;
+}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 2f703a20c724..7ce4027089ee 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -341,6 +341,7 @@ struct event_file_link *trace_probe_get_file_link(struct trace_probe *tp,
int trace_probe_compare_arg_type(struct trace_probe *a, struct trace_probe *b);
bool trace_probe_match_command_args(struct trace_probe *tp,
int argc, const char **argv);
+int trace_probe_create(const char *raw_command, int (*createfn)(int, const char **));
#define trace_probe_for_each_link(pos, tp) \
list_for_each_entry(pos, &(tp)->event->files, list)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 3cf7128e1ad3..e6b56a65f80f 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -34,7 +34,7 @@ struct uprobe_trace_entry_head {
#define DATAOF_TRACE_ENTRY(entry, is_return) \
((void*)(entry) + SIZEOF_TRACE_ENTRY(is_return))
-static int trace_uprobe_create(int argc, const char **argv);
+static int trace_uprobe_create(const char *raw_command);
static int trace_uprobe_show(struct seq_file *m, struct dyn_event *ev);
static int trace_uprobe_release(struct dyn_event *ev);
static bool trace_uprobe_is_busy(struct dyn_event *ev);
@@ -530,7 +530,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
* Argument syntax:
* - Add uprobe: p|r[:[GRP/]EVENT] PATH:OFFSET[%return][(REF)] [FETCHARGS]
*/
-static int trace_uprobe_create(int argc, const char **argv)
+static int __trace_uprobe_create(int argc, const char **argv)
{
struct trace_uprobe *tu;
const char *event = NULL, *group = UPROBE_EVENT_SYSTEM;
@@ -716,14 +716,19 @@ static int trace_uprobe_create(int argc, const char **argv)
return ret;
}
-static int create_or_delete_trace_uprobe(int argc, char **argv)
+int trace_uprobe_create(const char *raw_command)
+{
+ return trace_probe_create(raw_command, __trace_uprobe_create);
+}
+
+static int create_or_delete_trace_uprobe(const char *raw_command)
{
int ret;
- if (argv[0][0] == '-')
- return dyn_event_release(argc, argv, &trace_uprobe_ops);
+ if (raw_command[0] == '-')
+ return dyn_event_release(raw_command, &trace_uprobe_ops);
- ret = trace_uprobe_create(argc, (const char **)argv);
+ ret = trace_uprobe_create(raw_command);
return ret == -ECANCELED ? -EINVAL : ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] tracing: Update synth command errors
2020-10-23 20:33 [PATCH v2 0/4] tracing: More synthetic event error fixes Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function Tom Zanussi
@ 2020-10-23 20:33 ` Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 3/4] selftests/ftrace: Add synthetic event field separators Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 4/4] selftests/ftrace: Update synthetic event syntax errors Tom Zanussi
3 siblings, 0 replies; 8+ messages in thread
From: Tom Zanussi @ 2020-10-23 20:33 UTC (permalink / raw)
To: rostedt, axelrasmussen; +Cc: mhiramat, linux-kernel
Since array types are handled differently, errors referencing them
also need to be handled differently. Add and use a new
INVALID_ARRAY_SPEC error. Also add INVALID_CMD and INVALID_DYN_CMD to
catch and display the correct form for badly-formed commands, which
can also be used in place of CMD_INCOMPLETE, which is removed, and
remove CMD_TOO_LONG, since it's no longer used.
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
kernel/trace/trace_events_synth.c | 66 +++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index 90c1790ed1d6..1aa808791f56 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -23,13 +23,14 @@
#undef ERRORS
#define ERRORS \
C(BAD_NAME, "Illegal name"), \
- C(CMD_INCOMPLETE, "Incomplete command"), \
+ C(INVALID_CMD, "Command must be of the form: <name> field[;field] ..."),\
+ C(INVALID_DYN_CMD, "Command must be of the form: s or -:[synthetic/]<name> field[;field] ..."),\
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"),
+ C(INVALID_FIELD, "Invalid field"), \
+ C(INVALID_ARRAY_SPEC, "Invalid array specification"),
#undef C
#define C(a, b) SYNTH_ERR_##a
@@ -643,6 +644,10 @@ static struct synth_field *parse_synth_field(int argc, char **argv)
size = synth_field_size(field->type);
if (size < 0) {
+ if (array)
+ synth_err(SYNTH_ERR_INVALID_ARRAY_SPEC, errpos(field_name));
+ else
+ synth_err(SYNTH_ERR_INVALID_TYPE, errpos(field_type));
ret = -EINVAL;
goto free;
} else if (size == 0) {
@@ -1158,7 +1163,7 @@ static int __create_synth_event(const char *name, const char *raw_fields)
mutex_lock(&event_mutex);
if (name[0] == '\0') {
- synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
+ synth_err(SYNTH_ERR_INVALID_CMD, 0);
ret = -EINVAL;
goto out;
}
@@ -1212,7 +1217,7 @@ static int __create_synth_event(const char *name, const char *raw_fields)
}
if (n_fields == 0) {
- synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
+ synth_err(SYNTH_ERR_INVALID_CMD, 0);
ret = -EINVAL;
goto err;
}
@@ -1350,13 +1355,42 @@ int synth_event_delete(const char *event_name)
}
EXPORT_SYMBOL_GPL(synth_event_delete);
+static int check_command(const char *raw_command)
+{
+ char **argv = NULL, *cmd, *saved_cmd, *name_and_field;
+ int argc, ret = 0;
+
+ cmd = saved_cmd = kstrdup(raw_command, GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ name_and_field = strsep(&cmd, ";");
+ if (!name_and_field) {
+ ret = -EINVAL;
+ goto free;
+ }
+
+ argv = argv_split(GFP_KERNEL, name_and_field, &argc);
+ if (!argv) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ if (argc < 3)
+ ret = -EINVAL;
+free:
+ kfree(saved_cmd);
+ if (argv)
+ argv_free(argv);
+
+ return ret;
+}
+
static int create_or_delete_synth_event(const char *raw_command)
{
char **argv, *name = NULL, *fields;
int argc = 0, ret = 0;
- last_cmd_set(raw_command);
-
argv = argv_split(GFP_KERNEL, raw_command, &argc);
if (!argv)
return -ENOMEM;
@@ -1364,9 +1398,16 @@ static int create_or_delete_synth_event(const char *raw_command)
if (!argc)
goto free;
+ last_cmd_set(raw_command);
+
+ ret = check_command(raw_command);
+ if (ret) {
+ synth_err(SYNTH_ERR_INVALID_CMD, 0);
+ goto free;
+ }
+
name = argv[0];
- /* trace_run_command() ensures argc != 0 */
if (name[0] == '!') {
ret = synth_event_delete(name + 1);
goto free;
@@ -1951,12 +1992,21 @@ static int create_synth_event(const char *raw_command)
if (strchr(name, '/')) {
len = str_has_prefix(name, SYNTH_SYSTEM "/");
if (len == 0) {
+ synth_err(SYNTH_ERR_INVALID_DYN_CMD, 0);
ret = -EINVAL;
goto free;
}
name += len;
}
+ len = name - argv[0];
+
+ ret = check_command(raw_command + len);
+ if (ret) {
+ synth_err(SYNTH_ERR_INVALID_CMD, 0);
+ goto free;
+ }
+
fields = strstr(raw_command, name);
fields += strlen(name);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] selftests/ftrace: Add synthetic event field separators
2020-10-23 20:33 [PATCH v2 0/4] tracing: More synthetic event error fixes Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 2/4] tracing: Update synth command errors Tom Zanussi
@ 2020-10-23 20:33 ` Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 4/4] selftests/ftrace: Update synthetic event syntax errors Tom Zanussi
3 siblings, 0 replies; 8+ messages in thread
From: Tom Zanussi @ 2020-10-23 20:33 UTC (permalink / raw)
To: rostedt, axelrasmussen; +Cc: mhiramat, linux-kernel
These tests omit field separators from the synthetic event definitions
so don't follow the syntax '<event_name> field[;field] ...' required
for synthetic events.
Fixes: f06eec4d0f2c (selftests: ftrace: Add inter-event hist triggers testcases)
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
.../trigger/inter-event/trigger-inter-event-combined-hist.tc | 4 ++--
.../test.d/trigger/inter-event/trigger-onmatch-action-hist.tc | 2 +-
.../trigger/inter-event/trigger-onmatch-onmax-action-hist.tc | 2 +-
.../test.d/trigger/inter-event/trigger-trace-action-hist.tc | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-inter-event-combined-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-inter-event-combined-hist.tc
index 9098f1e7433f..29a03ed3377d 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-inter-event-combined-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-inter-event-combined-hist.tc
@@ -10,7 +10,7 @@ fail() { #msg
echo "Test create synthetic event"
-echo 'waking_latency u64 lat pid_t pid' > synthetic_events
+echo 'waking_latency u64 lat; pid_t pid' > synthetic_events
if [ ! -d events/synthetic/waking_latency ]; then
fail "Failed to create waking_latency synthetic event"
fi
@@ -21,7 +21,7 @@ echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="ping"' > events/sched/s
echo 'hist:keys=pid:waking_lat=common_timestamp.usecs-$ts0:onmatch(sched.sched_waking).waking_latency($waking_lat,pid) if comm=="ping"' > events/sched/sched_wakeup/trigger
echo 'hist:keys=pid,lat:sort=pid,lat' > events/synthetic/waking_latency/trigger
-echo 'wakeup_latency u64 lat pid_t pid' >> synthetic_events
+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
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-action-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-action-hist.tc
index 20e39471052e..5015d0d74de8 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-action-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-action-hist.tc
@@ -10,7 +10,7 @@ fail() { #msg
echo "Test create synthetic event"
-echo 'wakeup_latency u64 lat pid_t pid char comm[16]' > synthetic_events
+echo 'wakeup_latency u64 lat; pid_t pid; char comm[16]' > synthetic_events
if [ ! -d events/synthetic/wakeup_latency ]; then
fail "Failed to create wakeup_latency synthetic event"
fi
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-onmax-action-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-onmax-action-hist.tc
index f4b03ab7c287..ac7ba2bbce47 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-onmax-action-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onmatch-onmax-action-hist.tc
@@ -10,7 +10,7 @@ fail() { #msg
echo "Test create synthetic event"
-echo 'wakeup_latency u64 lat pid_t pid char comm[16]' > synthetic_events
+echo 'wakeup_latency u64 lat; pid_t pid; char comm[16]' > synthetic_events
if [ ! -d events/synthetic/wakeup_latency ]; then
fail "Failed to create wakeup_latency synthetic event"
fi
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-trace-action-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-trace-action-hist.tc
index c126d2350a6d..76a213f197a0 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-trace-action-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-trace-action-hist.tc
@@ -10,7 +10,7 @@ fail() { #msg
echo "Test create synthetic event"
-echo 'wakeup_latency u64 lat pid_t pid char comm[16]' > synthetic_events
+echo 'wakeup_latency u64 lat; pid_t pid; char comm[16]' > synthetic_events
if [ ! -d events/synthetic/wakeup_latency ]; then
fail "Failed to create wakeup_latency synthetic event"
fi
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] selftests/ftrace: Update synthetic event syntax errors
2020-10-23 20:33 [PATCH v2 0/4] tracing: More synthetic event error fixes Tom Zanussi
` (2 preceding siblings ...)
2020-10-23 20:33 ` [PATCH v2 3/4] selftests/ftrace: Add synthetic event field separators Tom Zanussi
@ 2020-10-23 20:33 ` Tom Zanussi
3 siblings, 0 replies; 8+ messages in thread
From: Tom Zanussi @ 2020-10-23 20:33 UTC (permalink / raw)
To: rostedt, axelrasmussen; +Cc: mhiramat, linux-kernel
Some of the synthetic event errors and positions have changed in the
code - update those and add several more tests.
Also add a runtime check to ensure that the kernel supports dynamic
strings in synthetic events, which these tests require.
Fixes: 81ff92a93d95 (selftests/ftrace: Add test case for synthetic
event syntax errors)
Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
.../trigger-synthetic_event_syntax_errors.tc | 36 ++++++++++++++-----
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
index ada594fe16cb..9b2f47f7eb07 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
@@ -1,19 +1,39 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
# description: event trigger - test synthetic_events syntax parser errors
-# requires: synthetic_events error_log
+# requires: synthetic_events error_log "char name[]' >> synthetic_events":README
check_error() { # command-with-error-pos-by-^
ftrace_errlog_check 'synthetic_events' "$1" 'synthetic_events'
}
+check_dyn_error() { # command-with-error-pos-by-^
+ ftrace_errlog_check 'synthetic_events' "$1" 'dynamic_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
+check_error 'myevent ^unsigned arg' # INCOMPLETE_TYPE
+
+check_error 'myevent char ^str]; int v' # BAD_NAME
+check_error '^mye-vent char str[]' # BAD_NAME
+check_error 'myevent char ^st-r[]' # BAD_NAME
+
+check_error 'myevent char str;^[]' # INVALID_FIELD
+check_error 'myevent char str; ^int' # INVALID_FIELD
+
+check_error 'myevent char ^str[; int v' # INVALID_ARRAY_SPEC
+check_error 'myevent char ^str[kdjdk]' # INVALID_ARRAY_SPEC
+check_error 'myevent char ^str[257]' # INVALID_ARRAY_SPEC
+check_error ' myevent char ^str[' # INVALID_ARRAY_SPEC
+
+check_error '^mye;vent char str[]' # INVALID_CMD
+check_error '^myevent ; char str[]' # INVALID_CMD
+check_error '^myevent; char str[]' # INVALID_CMD
+check_error '^myevent ;char str[]' # INVALID_CMD
+check_error '^; char str[]' # INVALID_CMD
+check_error '^;myevent char str[]' # INVALID_CMD
+check_error '^myevent' # INVALID_CMD
+
+check_dyn_error '^s:junk/myevent char str[' # INVALID_DYN_CMD
exit 0
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function
2020-10-23 20:33 ` [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function Tom Zanussi
@ 2020-10-23 23:03 ` kernel test robot
2020-10-24 8:50 ` Masami Hiramatsu
1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2020-10-23 23:03 UTC (permalink / raw)
To: Tom Zanussi, rostedt, axelrasmussen
Cc: kbuild-all, clang-built-linux, mhiramat, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 16308 bytes --]
Hi Tom,
I love your patch! Perhaps something to improve:
[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master next-20201023]
[cannot apply to tip/perf/core linux/master v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Tom-Zanussi/tracing-More-synthetic-event-error-fixes/20201024-043714
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: s390-randconfig-r031-20201022 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 147b9497e79a98a8614b2b5eb4ba653b44f6b6f0)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/10ea63ba8aed09b6b543d606de36b21d0d2b7b88
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tom-Zanussi/tracing-More-synthetic-event-error-fixes/20201024-043714
git checkout 10ea63ba8aed09b6b543d606de36b21d0d2b7b88
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> kernel/trace/trace_events_synth.c:1211:3: warning: variable 'i' is uninitialized when used here [-Wuninitialized]
i += consumed - 1;
^
kernel/trace/trace_events_synth.c:1146:7: note: initialize the variable 'i' to silence this warning
int i, argc, consumed = 0, n_fields = 0, ret = 0;
^
= 0
21 warnings generated.
vim +/i +1211 kernel/trace/trace_events_synth.c
726721a51838e39 Tom Zanussi 2020-05-28 1143
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1144 static int __create_synth_event(const char *name, const char *raw_fields)
726721a51838e39 Tom Zanussi 2020-05-28 1145 {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1146 int i, argc, consumed = 0, n_fields = 0, ret = 0;
726721a51838e39 Tom Zanussi 2020-05-28 1147 struct synth_field *field, *fields[SYNTH_FIELDS_MAX];
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1148 char **argv, *field_str, *tmp_fields, *saved_fields = NULL;
726721a51838e39 Tom Zanussi 2020-05-28 1149 struct synth_event *event = NULL;
d4d704637d935ef Tom Zanussi 2020-10-13 1150
726721a51838e39 Tom Zanussi 2020-05-28 1151 /*
726721a51838e39 Tom Zanussi 2020-05-28 1152 * Argument syntax:
726721a51838e39 Tom Zanussi 2020-05-28 1153 * - Add synthetic event: <event_name> field[;field] ...
726721a51838e39 Tom Zanussi 2020-05-28 1154 * - Remove synthetic event: !<event_name> field[;field] ...
726721a51838e39 Tom Zanussi 2020-05-28 1155 * where 'field' = type field_name
726721a51838e39 Tom Zanussi 2020-05-28 1156 */
726721a51838e39 Tom Zanussi 2020-05-28 1157
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1158 mutex_lock(&event_mutex);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1159
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1160 if (name[0] == '\0') {
d4d704637d935ef Tom Zanussi 2020-10-13 1161 synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1162 ret = -EINVAL;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1163 goto out;
d4d704637d935ef Tom Zanussi 2020-10-13 1164 }
726721a51838e39 Tom Zanussi 2020-05-28 1165
9bbb33291f8e448 Tom Zanussi 2020-10-13 1166 if (!is_good_name(name)) {
d4d704637d935ef Tom Zanussi 2020-10-13 1167 synth_err(SYNTH_ERR_BAD_NAME, errpos(name));
9bbb33291f8e448 Tom Zanussi 2020-10-13 1168 ret = -EINVAL;
9bbb33291f8e448 Tom Zanussi 2020-10-13 1169 goto out;
9bbb33291f8e448 Tom Zanussi 2020-10-13 1170 }
9bbb33291f8e448 Tom Zanussi 2020-10-13 1171
726721a51838e39 Tom Zanussi 2020-05-28 1172 event = find_synth_event(name);
726721a51838e39 Tom Zanussi 2020-05-28 1173 if (event) {
d4d704637d935ef Tom Zanussi 2020-10-13 1174 synth_err(SYNTH_ERR_EVENT_EXISTS, errpos(name));
726721a51838e39 Tom Zanussi 2020-05-28 1175 ret = -EEXIST;
726721a51838e39 Tom Zanussi 2020-05-28 1176 goto out;
726721a51838e39 Tom Zanussi 2020-05-28 1177 }
726721a51838e39 Tom Zanussi 2020-05-28 1178
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1179 tmp_fields = saved_fields = kstrdup(raw_fields, GFP_KERNEL);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1180 if (!tmp_fields) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1181 ret = -ENOMEM;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1182 goto out;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1183 }
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1184
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1185 while ((field_str = strsep(&tmp_fields, ";")) != NULL) {
726721a51838e39 Tom Zanussi 2020-05-28 1186 if (n_fields == SYNTH_FIELDS_MAX) {
d4d704637d935ef Tom Zanussi 2020-10-13 1187 synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0);
726721a51838e39 Tom Zanussi 2020-05-28 1188 ret = -EINVAL;
726721a51838e39 Tom Zanussi 2020-05-28 1189 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1190 }
726721a51838e39 Tom Zanussi 2020-05-28 1191
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1192 argv = argv_split(GFP_KERNEL, field_str, &argc);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1193 if (!argv) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1194 ret = -ENOMEM;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1195 goto err;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1196 }
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1197
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1198 if (!argc)
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1199 continue;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1200
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1201 field = parse_synth_field(argc, argv);
726721a51838e39 Tom Zanussi 2020-05-28 1202 if (IS_ERR(field)) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1203 argv_free(argv);
726721a51838e39 Tom Zanussi 2020-05-28 1204 ret = PTR_ERR(field);
726721a51838e39 Tom Zanussi 2020-05-28 1205 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1206 }
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1207
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1208 argv_free(argv);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1209
726721a51838e39 Tom Zanussi 2020-05-28 1210 fields[n_fields++] = field;
726721a51838e39 Tom Zanussi 2020-05-28 @1211 i += consumed - 1;
726721a51838e39 Tom Zanussi 2020-05-28 1212 }
726721a51838e39 Tom Zanussi 2020-05-28 1213
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1214 if (n_fields == 0) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1215 synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
726721a51838e39 Tom Zanussi 2020-05-28 1216 ret = -EINVAL;
726721a51838e39 Tom Zanussi 2020-05-28 1217 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1218 }
726721a51838e39 Tom Zanussi 2020-05-28 1219
726721a51838e39 Tom Zanussi 2020-05-28 1220 event = alloc_synth_event(name, n_fields, fields);
726721a51838e39 Tom Zanussi 2020-05-28 1221 if (IS_ERR(event)) {
726721a51838e39 Tom Zanussi 2020-05-28 1222 ret = PTR_ERR(event);
726721a51838e39 Tom Zanussi 2020-05-28 1223 event = NULL;
726721a51838e39 Tom Zanussi 2020-05-28 1224 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1225 }
726721a51838e39 Tom Zanussi 2020-05-28 1226 ret = register_synth_event(event);
726721a51838e39 Tom Zanussi 2020-05-28 1227 if (!ret)
726721a51838e39 Tom Zanussi 2020-05-28 1228 dyn_event_add(&event->devent);
726721a51838e39 Tom Zanussi 2020-05-28 1229 else
726721a51838e39 Tom Zanussi 2020-05-28 1230 free_synth_event(event);
726721a51838e39 Tom Zanussi 2020-05-28 1231 out:
726721a51838e39 Tom Zanussi 2020-05-28 1232 mutex_unlock(&event_mutex);
726721a51838e39 Tom Zanussi 2020-05-28 1233
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1234 kfree(saved_fields);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1235
726721a51838e39 Tom Zanussi 2020-05-28 1236 return ret;
726721a51838e39 Tom Zanussi 2020-05-28 1237 err:
726721a51838e39 Tom Zanussi 2020-05-28 1238 for (i = 0; i < n_fields; i++)
726721a51838e39 Tom Zanussi 2020-05-28 1239 free_synth_field(fields[i]);
726721a51838e39 Tom Zanussi 2020-05-28 1240
726721a51838e39 Tom Zanussi 2020-05-28 1241 goto out;
726721a51838e39 Tom Zanussi 2020-05-28 1242 }
726721a51838e39 Tom Zanussi 2020-05-28 1243
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21211 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function
@ 2020-10-23 23:03 ` kernel test robot
0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2020-10-23 23:03 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 16556 bytes --]
Hi Tom,
I love your patch! Perhaps something to improve:
[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master next-20201023]
[cannot apply to tip/perf/core linux/master v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Tom-Zanussi/tracing-More-synthetic-event-error-fixes/20201024-043714
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: s390-randconfig-r031-20201022 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 147b9497e79a98a8614b2b5eb4ba653b44f6b6f0)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/10ea63ba8aed09b6b543d606de36b21d0d2b7b88
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tom-Zanussi/tracing-More-synthetic-event-error-fixes/20201024-043714
git checkout 10ea63ba8aed09b6b543d606de36b21d0d2b7b88
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from kernel/trace/trace_events_synth.c:21:
In file included from kernel/trace/trace_synth.h:5:
In file included from kernel/trace/trace_dynevent.h:14:
In file included from kernel/trace/trace.h:9:
In file included from include/linux/clocksource.h:21:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> kernel/trace/trace_events_synth.c:1211:3: warning: variable 'i' is uninitialized when used here [-Wuninitialized]
i += consumed - 1;
^
kernel/trace/trace_events_synth.c:1146:7: note: initialize the variable 'i' to silence this warning
int i, argc, consumed = 0, n_fields = 0, ret = 0;
^
= 0
21 warnings generated.
vim +/i +1211 kernel/trace/trace_events_synth.c
726721a51838e39 Tom Zanussi 2020-05-28 1143
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1144 static int __create_synth_event(const char *name, const char *raw_fields)
726721a51838e39 Tom Zanussi 2020-05-28 1145 {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1146 int i, argc, consumed = 0, n_fields = 0, ret = 0;
726721a51838e39 Tom Zanussi 2020-05-28 1147 struct synth_field *field, *fields[SYNTH_FIELDS_MAX];
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1148 char **argv, *field_str, *tmp_fields, *saved_fields = NULL;
726721a51838e39 Tom Zanussi 2020-05-28 1149 struct synth_event *event = NULL;
d4d704637d935ef Tom Zanussi 2020-10-13 1150
726721a51838e39 Tom Zanussi 2020-05-28 1151 /*
726721a51838e39 Tom Zanussi 2020-05-28 1152 * Argument syntax:
726721a51838e39 Tom Zanussi 2020-05-28 1153 * - Add synthetic event: <event_name> field[;field] ...
726721a51838e39 Tom Zanussi 2020-05-28 1154 * - Remove synthetic event: !<event_name> field[;field] ...
726721a51838e39 Tom Zanussi 2020-05-28 1155 * where 'field' = type field_name
726721a51838e39 Tom Zanussi 2020-05-28 1156 */
726721a51838e39 Tom Zanussi 2020-05-28 1157
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1158 mutex_lock(&event_mutex);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1159
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1160 if (name[0] == '\0') {
d4d704637d935ef Tom Zanussi 2020-10-13 1161 synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1162 ret = -EINVAL;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1163 goto out;
d4d704637d935ef Tom Zanussi 2020-10-13 1164 }
726721a51838e39 Tom Zanussi 2020-05-28 1165
9bbb33291f8e448 Tom Zanussi 2020-10-13 1166 if (!is_good_name(name)) {
d4d704637d935ef Tom Zanussi 2020-10-13 1167 synth_err(SYNTH_ERR_BAD_NAME, errpos(name));
9bbb33291f8e448 Tom Zanussi 2020-10-13 1168 ret = -EINVAL;
9bbb33291f8e448 Tom Zanussi 2020-10-13 1169 goto out;
9bbb33291f8e448 Tom Zanussi 2020-10-13 1170 }
9bbb33291f8e448 Tom Zanussi 2020-10-13 1171
726721a51838e39 Tom Zanussi 2020-05-28 1172 event = find_synth_event(name);
726721a51838e39 Tom Zanussi 2020-05-28 1173 if (event) {
d4d704637d935ef Tom Zanussi 2020-10-13 1174 synth_err(SYNTH_ERR_EVENT_EXISTS, errpos(name));
726721a51838e39 Tom Zanussi 2020-05-28 1175 ret = -EEXIST;
726721a51838e39 Tom Zanussi 2020-05-28 1176 goto out;
726721a51838e39 Tom Zanussi 2020-05-28 1177 }
726721a51838e39 Tom Zanussi 2020-05-28 1178
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1179 tmp_fields = saved_fields = kstrdup(raw_fields, GFP_KERNEL);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1180 if (!tmp_fields) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1181 ret = -ENOMEM;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1182 goto out;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1183 }
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1184
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1185 while ((field_str = strsep(&tmp_fields, ";")) != NULL) {
726721a51838e39 Tom Zanussi 2020-05-28 1186 if (n_fields == SYNTH_FIELDS_MAX) {
d4d704637d935ef Tom Zanussi 2020-10-13 1187 synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0);
726721a51838e39 Tom Zanussi 2020-05-28 1188 ret = -EINVAL;
726721a51838e39 Tom Zanussi 2020-05-28 1189 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1190 }
726721a51838e39 Tom Zanussi 2020-05-28 1191
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1192 argv = argv_split(GFP_KERNEL, field_str, &argc);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1193 if (!argv) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1194 ret = -ENOMEM;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1195 goto err;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1196 }
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1197
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1198 if (!argc)
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1199 continue;
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1200
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1201 field = parse_synth_field(argc, argv);
726721a51838e39 Tom Zanussi 2020-05-28 1202 if (IS_ERR(field)) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1203 argv_free(argv);
726721a51838e39 Tom Zanussi 2020-05-28 1204 ret = PTR_ERR(field);
726721a51838e39 Tom Zanussi 2020-05-28 1205 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1206 }
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1207
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1208 argv_free(argv);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1209
726721a51838e39 Tom Zanussi 2020-05-28 1210 fields[n_fields++] = field;
726721a51838e39 Tom Zanussi 2020-05-28 @1211 i += consumed - 1;
726721a51838e39 Tom Zanussi 2020-05-28 1212 }
726721a51838e39 Tom Zanussi 2020-05-28 1213
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1214 if (n_fields == 0) {
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1215 synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
726721a51838e39 Tom Zanussi 2020-05-28 1216 ret = -EINVAL;
726721a51838e39 Tom Zanussi 2020-05-28 1217 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1218 }
726721a51838e39 Tom Zanussi 2020-05-28 1219
726721a51838e39 Tom Zanussi 2020-05-28 1220 event = alloc_synth_event(name, n_fields, fields);
726721a51838e39 Tom Zanussi 2020-05-28 1221 if (IS_ERR(event)) {
726721a51838e39 Tom Zanussi 2020-05-28 1222 ret = PTR_ERR(event);
726721a51838e39 Tom Zanussi 2020-05-28 1223 event = NULL;
726721a51838e39 Tom Zanussi 2020-05-28 1224 goto err;
726721a51838e39 Tom Zanussi 2020-05-28 1225 }
726721a51838e39 Tom Zanussi 2020-05-28 1226 ret = register_synth_event(event);
726721a51838e39 Tom Zanussi 2020-05-28 1227 if (!ret)
726721a51838e39 Tom Zanussi 2020-05-28 1228 dyn_event_add(&event->devent);
726721a51838e39 Tom Zanussi 2020-05-28 1229 else
726721a51838e39 Tom Zanussi 2020-05-28 1230 free_synth_event(event);
726721a51838e39 Tom Zanussi 2020-05-28 1231 out:
726721a51838e39 Tom Zanussi 2020-05-28 1232 mutex_unlock(&event_mutex);
726721a51838e39 Tom Zanussi 2020-05-28 1233
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1234 kfree(saved_fields);
10ea63ba8aed09b Masami Hiramatsu 2020-10-23 1235
726721a51838e39 Tom Zanussi 2020-05-28 1236 return ret;
726721a51838e39 Tom Zanussi 2020-05-28 1237 err:
726721a51838e39 Tom Zanussi 2020-05-28 1238 for (i = 0; i < n_fields; i++)
726721a51838e39 Tom Zanussi 2020-05-28 1239 free_synth_field(fields[i]);
726721a51838e39 Tom Zanussi 2020-05-28 1240
726721a51838e39 Tom Zanussi 2020-05-28 1241 goto out;
726721a51838e39 Tom Zanussi 2020-05-28 1242 }
726721a51838e39 Tom Zanussi 2020-05-28 1243
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 21211 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function
2020-10-23 20:33 ` [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function Tom Zanussi
2020-10-23 23:03 ` kernel test robot
@ 2020-10-24 8:50 ` Masami Hiramatsu
1 sibling, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2020-10-24 8:50 UTC (permalink / raw)
To: Tom Zanussi; +Cc: rostedt, axelrasmussen, mhiramat, linux-kernel
Hi Tom,
Thanks for the update!
On Fri, 23 Oct 2020 15:33:52 -0500
Tom Zanussi <zanussi@kernel.org> wrote:
> From: Masami Hiramatsu <mhiramat@kernel.org>
>
> Delegate command parsing to each create function so that the
> command syntax can be customized.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> [ zanussi@kernel.org: added synthetic event modifications ]
Since you've customized the synth_event parser, could you update
the patch description? (Or split this into 2 patches)
> Signed-off-by: Tom Zanussi <zanussi@kernel.org>
> ---
> kernel/trace/trace.c | 23 +---
> kernel/trace/trace.h | 3 +-
> kernel/trace/trace_dynevent.c | 35 +++---
> kernel/trace/trace_dynevent.h | 4 +-
> kernel/trace/trace_events_synth.c | 186 ++++++++++++++++--------------
> kernel/trace/trace_kprobe.c | 33 +++---
> kernel/trace/trace_probe.c | 17 +++
> kernel/trace/trace_probe.h | 1 +
> kernel/trace/trace_uprobe.c | 17 ++-
> 9 files changed, 174 insertions(+), 145 deletions(-)
[..]
> @@ -1223,26 +1176,43 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
> goto out;
> }
>
> - for (i = 0; i < argc - 1; i++) {
> - if (strcmp(argv[i], ";") == 0)
> - continue;
> + tmp_fields = saved_fields = kstrdup(raw_fields, GFP_KERNEL);
> + if (!tmp_fields) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + while ((field_str = strsep(&tmp_fields, ";")) != NULL) {
> if (n_fields == SYNTH_FIELDS_MAX) {
> synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0);
> ret = -EINVAL;
> goto err;
> }
>
> - field = parse_synth_field(argc - i, &argv[i], &consumed);
> + argv = argv_split(GFP_KERNEL, field_str, &argc);
> + if (!argv) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + if (!argc)
> + continue;
> +
> + field = parse_synth_field(argc, argv);
> if (IS_ERR(field)) {
> + argv_free(argv);
> ret = PTR_ERR(field);
> goto err;
> }
> +
> + argv_free(argv);
> +
> fields[n_fields++] = field;
> i += consumed - 1;
You may not need this line (and "consumed") anymore?
> }
>
> - if (i < argc && strcmp(argv[i], ";") != 0) {
> - synth_err(SYNTH_ERR_INVALID_FIELD, errpos(argv[i]));
> + if (n_fields == 0) {
> + synth_err(SYNTH_ERR_CMD_INCOMPLETE, 0);
> ret = -EINVAL;
> goto err;
> }
> @@ -1261,6 +1231,8 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
> out:
> mutex_unlock(&event_mutex);
>
> + kfree(saved_fields);
> +
> return ret;
> err:
> for (i = 0; i < n_fields; i++)
> @@ -1378,18 +1350,35 @@ int synth_event_delete(const char *event_name)
> }
> EXPORT_SYMBOL_GPL(synth_event_delete);
>
> -static int create_or_delete_synth_event(int argc, char **argv)
> +static int create_or_delete_synth_event(const char *raw_command)
> {
> - const char *name = argv[0];
> - int ret;
> + char **argv, *name = NULL, *fields;
> + int argc = 0, ret = 0;
> +
> + last_cmd_set(raw_command);
> +
> + argv = argv_split(GFP_KERNEL, raw_command, &argc);
> + if (!argv)
> + return -ENOMEM;
If you are sure the first argument is the name, you don't need
to use argv_split, but just strpbrk(raw_command, " \t"), something
like this.
raw_command = skip_spaces(raw_command);
p = strpbrk(raw_command, " \t");
if (!p)
return -EINVAL;
name = kmemdup_nul(raw_command, p - raw_command, GFP_KERNEL);
field = skip_spaces(p);
...
ret = __create_synth_event(name, fields);
free:
kfree(name);
(BTW, we should have find_spaces() instead of slow strpbrk().)
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-24 8:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 20:33 [PATCH v2 0/4] tracing: More synthetic event error fixes Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 1/4] tracing/dynevent: Delegate parsing to create function Tom Zanussi
2020-10-23 23:03 ` kernel test robot
2020-10-23 23:03 ` kernel test robot
2020-10-24 8:50 ` Masami Hiramatsu
2020-10-23 20:33 ` [PATCH v2 2/4] tracing: Update synth command errors Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 3/4] selftests/ftrace: Add synthetic event field separators Tom Zanussi
2020-10-23 20:33 ` [PATCH v2 4/4] selftests/ftrace: Update synthetic event syntax errors Tom Zanussi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.