Re: [PATCH v7 03/13] user_events: Add print_fmt generation support for basic types

Masami Hiramatsu
2021-12-10T02:51:00Z

On Thu, 9 Dec 2021 14:32:00 -0800
Beau Belgrave <beaub@linux.microsoft.com> wrote:

> Addes print_fmt format generation for basic types that are supported for
> user processes. Only supports sizes that are the same on 32 and 64 bit.
>

This looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
> ---
> kernel/trace/trace_events_user.c | 107 ++++++++++++++++++++++++++++++-
> 1 file changed, 105 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 1d96d1c85147..bd8ac46fddb1 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -357,6 +357,106 @@ static int user_event_parse_fields(struct user_event *user, char *args)
>
> static struct trace_event_fields user_event_fields_array[1];
>
> +static const char *user_field_format(const char *type)
> +{
> + if (strcmp(type, "s64") == 0)
> + return "%lld";
> + if (strcmp(type, "u64") == 0)
> + return "%llu";
> + if (strcmp(type, "s32") == 0)
> + return "%d";
> + if (strcmp(type, "u32") == 0)
> + return "%u";
> + if (strcmp(type, "int") == 0)
> + return "%d";
> + if (strcmp(type, "unsigned int") == 0)
> + return "%u";
> + if (strcmp(type, "s16") == 0)
> + return "%d";
> + if (strcmp(type, "u16") == 0)
> + return "%u";
> + if (strcmp(type, "short") == 0)
> + return "%d";
> + if (strcmp(type, "unsigned short") == 0)
> + return "%u";
> + if (strcmp(type, "s8") == 0)
> + return "%d";
> + if (strcmp(type, "u8") == 0)
> + return "%u";
> + if (strcmp(type, "char") == 0)
> + return "%d";
> + if (strcmp(type, "unsigned char") == 0)
> + return "%u";
> + if (strstr(type, "char[") != 0)
> + return "%s";
> +
> + /* Unknown, likely struct, allowed treat as 64-bit */
> + return "%llu";
> +}
> +
> +static bool user_field_is_dyn_string(const char *type)
> +{
> + if (str_has_prefix(type, "__data_loc ") ||
> + str_has_prefix(type, "__rel_loc "))
> + if (strstr(type, "char[") != 0)
> + return true;
> +
> + return false;
> +}
> +
> +#define LEN_OR_ZERO (len ? len - pos : 0)
> +static int user_event_set_print_fmt(struct user_event *user, char *buf, int len)
> +{
> + struct ftrace_event_field *field, *next;
> + struct list_head *head = &user->fields;
> + int pos = 0, depth = 0;
> +
> + pos += snprintf(buf + pos, LEN_OR_ZERO, "\"");
> +
> + list_for_each_entry_safe_reverse(field, next, head, link) {
> + if (depth != 0)
> + pos += snprintf(buf + pos, LEN_OR_ZERO, " ");
> +
> + pos += snprintf(buf + pos, LEN_OR_ZERO, "%s=%s",
> + field->name, user_field_format(field->type));
> +
> + depth++;
> + }
> +
> + pos += snprintf(buf + pos, LEN_OR_ZERO, "\"");
> +
> + list_for_each_entry_safe_reverse(field, next, head, link) {
> + if (user_field_is_dyn_string(field->type))
> + pos += snprintf(buf + pos, LEN_OR_ZERO,
> + ", __get_str(%s)", field->name);
> + else
> + pos += snprintf(buf + pos, LEN_OR_ZERO,
> + ", REC->%s", field->name);
> + }
> +
> + return pos + 1;
> +}
> +#undef LEN_OR_ZERO
> +
> +static int user_event_create_print_fmt(struct user_event *user)
> +{
> + char *print_fmt;
> + int len;
> +
> + len = user_event_set_print_fmt(user, NULL, 0);
> +
> + print_fmt = kmalloc(len, GFP_KERNEL);
> +
> + if (!print_fmt)
> + return -ENOMEM;
> +
> + user_event_set_print_fmt(user, print_fmt, len);
> +
> + user->call.print_fmt = print_fmt;
> +
> + return 0;
> +}
> +
> static enum print_line_t user_event_print_trace(struct trace_iterator *iter,
> int flags,
> struct trace_event *event)
> @@ -390,6 +490,7 @@ static int destroy_user_event(struct user_event *user)
> clear_bit(user->index, page_bitmap);
> hash_del(&user->node);
>
> + kfree(user->call.print_fmt);
> kfree(EVENT_NAME(user));
> kfree(user);
>
> @@ -669,8 +770,10 @@ static int user_event_parse(char *name, char *args, char *flags,
> if (ret)
> goto put_user;
>
> - /* Minimal print format */
> - user->call.print_fmt = "\"\"";
> + ret = user_event_create_print_fmt(user);
> +
> + if (ret)
> + goto put_user;
>
> user->call.data = user;
> user->call.class = &user->class;
> --
> 2.17.1
>

--
Masami Hiramatsu <mhiramat@kernel.org>