Re: [PATCH v4 04/12] tracing: Add dynamic event command creation interface
From: Masami Hiramatsu <mhiramat@kernel.org>
Date: 2020-01-30 02:40:35
Also in:
lkml
Hi Tom On Wed, 29 Jan 2020 12:59:24 -0600 Tom Zanussi [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add an interface used to build up dynamic event creation commands, such as synthetic and kprobe events. Interfaces specific to those particular types of events and others can be built on top of this interface. Command creation is started by first using the dynevent_cmd_init() function to initialize the dynevent_cmd object. Following that, args are appended and optionally checked by the dynevent_arg_add() and dynevent_arg_pair_add() functions, which use objects representing arguments and pairs of arguments, initialized respectively by dynevent_arg_init() and dynevent_arg_pair_init(). Finally, once all args have been successfully added, the command is finalized and actually created using dynevent_create(). The code here for actually printing into the dyn_event->cmd buffer using snprintf() etc was adapted from v4 of Masami's 'tracing/boot: Add synthetic event support' patch. Signed-off-by: Tom Zanussi <zanussi@kernel.org> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- include/linux/trace_events.h | 23 ++++ kernel/trace/trace_dynevent.c | 240 ++++++++++++++++++++++++++++++++++++++++++ kernel/trace/trace_dynevent.h | 33 ++++++ 3 files changed, 296 insertions(+)diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index 25fe743bcbaf..651b03d5e272 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h@@ -354,6 +354,29 @@ extern struct trace_event_file *trace_get_event_file(const char *instance, const char *event); extern void trace_put_event_file(struct trace_event_file *file); +#define MAX_DYNEVENT_CMD_LEN (2048) + +enum dynevent_type { + DYNEVENT_TYPE_NONE, +}; + +struct dynevent_cmd; + +typedef int (*dynevent_create_fn_t)(struct dynevent_cmd *cmd); + +struct dynevent_cmd { + char *buf; + const char *event_name; + int maxlen; + int remaining; + unsigned int n_fields; + enum dynevent_type type; + dynevent_create_fn_t run_command; + void *private_data; +}; + +extern int dynevent_create(struct dynevent_cmd *cmd); + extern int synth_event_delete(const char *name); /*diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c index 89779eb84a07..6ffdbc4fda53 100644 --- a/kernel/trace/trace_dynevent.c +++ b/kernel/trace/trace_dynevent.c@@ -223,3 +223,243 @@ static __init int init_dynamic_event(void) return 0; } fs_initcall(init_dynamic_event); + +/** + * dynevent_arg_add - Add an arg to a dynevent_cmd + * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd + * @arg: The argument to append to the current cmd + * + * Append an argument to a dynevent_cmd. The argument string will be + * appended to the current cmd string, followed by a separator, if + * applicable. Before the argument is added, the check_arg() + * function, if defined, is called. + * + * The cmd string, separator, and check_arg() function should be set + * using the dynevent_arg_init() before any arguments are added using + * this function. + * + * Return: 0 if successful, error otherwise. + */ +int dynevent_arg_add(struct dynevent_cmd *cmd, + struct dynevent_arg *arg) +{ + int ret = 0; + int delta; + char *q; + + if (arg->check_arg) { + ret = arg->check_arg(arg);
Hmm, this seems a bit odd. The check_arg() arg object member check the validity of the object itself. What about moving those checker into dynevent_cmd instead of dynevent_arg? (Or, just pass the appropriate checker as the 3rd parameter of this function)
+ if (ret)
+ return ret;
+ }
+
+ q = cmd->buf + (cmd->maxlen - cmd->remaining);
+
+ delta = snprintf(q, cmd->remaining, " %s%c", arg->str, arg->separator);
+ if (delta >= cmd->remaining) {
+ pr_err("String is too long: %s\n", arg->str);
+ return -E2BIG;
+ }
+ cmd->remaining -= delta;
+
+ return ret;
+}
+
+/**
+ * dynevent_arg_pair_add - Add an arg pair to a dynevent_cmd
+ * @cmd: A pointer to the dynevent_cmd struct representing the new event cmd
+ * @arg_pair: The argument pair to append to the current cmd
+ *
+ * Append an argument pair to a dynevent_cmd. An argument pair
+ * consists of a left-hand-side argument and a right-hand-side
+ * argument separated by an operator, which can be whitespace, all
+ * followed by a separator, if applicable. This can be used to add
+ * arguments of the form 'type variable_name;' or 'x+y'.
+ *
+ * The lhs argument string will be appended to the current cmd string,
+ * followed by an operator, if applicable, followd by the rhs string,
+ * followed finally by a separator, if applicable. Before anything is
+ * added, the check_arg() function, if defined, is called.
+ *
+ * The cmd strings, operator, separator, and check_arg() function
+ * should be set using the dynevent_arg_pair_init() before any arguments
+ * are added using this function.
+ *
+ * Return: 0 if successful, error otherwise.
+ */
+int dynevent_arg_pair_add(struct dynevent_cmd *cmd,
+ struct dynevent_arg_pair *arg_pair)
+{
+ int ret = 0;
+ int delta;
+ char *q;
+
+ if (arg_pair->check_arg) {
+ ret = arg_pair->check_arg(arg_pair);Ditto.
+ if (ret)
+ return ret;
+ }
+
+ q = cmd->buf + (cmd->maxlen - cmd->remaining);
+
+ delta = snprintf(q, cmd->remaining, " %s%c", arg_pair->lhs,
+ arg_pair->operator);
+ if (delta >= cmd->remaining) {
+ pr_err("field string is too long: %s\n", arg_pair->lhs);
+ return -E2BIG;
+ }
+ cmd->remaining -= delta; q += delta;
+
+ delta = snprintf(q, cmd->remaining, "%s%c", arg_pair->rhs,
+ arg_pair->separator);
+ if (delta >= cmd->remaining) {
+ pr_err("field string is too long: %s\n", arg_pair->rhs);
+ return -E2BIG;
+ }
+ cmd->remaining -= delta; q += delta;Here, q += delta is redundant, because q is not used anymore.
+ + return ret; +} +
Thank you, -- Masami Hiramatsu [off-list ref]