lkml.org 
[lkml]   [2019]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 0/7] tracing: Add support for in-kernel synthetic event API
From
Date
Hi Masami,

On Thu, 2019-12-19 at 23:45 +0900, Masami Hiramatsu wrote:
> Hello Tom,
>
> On Wed, 18 Dec 2019 09:27:36 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > Hi,
> >
> > I've recently had several requests and suggestions from users to
> > add
> > support for the creation and generation of synthetic events from
> > kernel code such as modules, and not just from the available
> > command
> > line commands.
>
> This reminds me my recent series of patches.
>
> Could you use synth_event_run_command() for this purpose as I did
> in boot-time tracing series?
>
> https://lkml.kernel.org/r/157528179955.22451.16317363776831311868.stg
> it@devnote2
>
> Above example uses a command string as same as command string, but I
> think
> we can introduce some macros to construct the command string for
> easier
> definition.
>
> Or maybe it is possible to pass the const char *args[] array to that
> API,
> instead of single char *cmd. (for dynamic event definiton)
>
> Maybe we should consider more generic APIs for modules to
> create/delete
> dynamic-event including synthetic and probes, and those might reuse
> existing command parsers.
>

I'll have to look at your patches more closely, but I think it should
be possible to generate the command string synth_event_run_command()
needs, or the equivalent const char *args[] array you mentioned, from
the higher-level event definition in my patches.

So the two ways of defining an event in my patches is 1) from a static
array known at build-time defined like this:

static struct synth_field_desc synthtest_fields[] = {
{ .type = "pid_t", .name = "next_pid_field" },
{ .type = "char[16]", .name = "next_comm_field" },
{ .type = "u64", .name = "ts_ns" },
{ .type = "u64", .name = "ts_ms" },
{ .type = "unsigned int", .name = "cpu" },
{ .type = "char[64]", .name = "my_string_field" },
{ .type = "int", .name = "my_int_field" },
};

which is then passed to create_synth_event(&synthtest_fields)

or 2) at run-time by adding fields individually as they become known:

add_synth_field("type", "name")

which requires some sort of start/end("event name").

I think that should all be possible using your patches, and maybe
generalizable to not just synth events by removing _synth_ from
everything? Let me know what you think. If that's correct, I can go
and rewrite the event creation part on top of your patches.

> > This patchset adds support for that. The first three patches add
> > some
> > minor preliminary setup, followed by the two main patches that add
> > the
> > ability to create and generate synthetic events from the
> > kernel. The
> > next patch adds a test module that demonstrates actual use of the
> > API
> > and verifies that it works as intended, followed by Documentation.
>
> Could you also check the locks are correctly acquired? It seems that
> your APIs doesn't lock it.
>

I did notice that I said that trace_types_lock and event_mutex should
be held for trace_array_find() but it should only be trace_types_lock,
and then I missed doing that in get_event_file() in one place. And I
also don't really need the nolock versions, so will simplify and remove
them. I think elsewhere event_mutex is taken where needed. But if
talking about something else, please let me know.

Thanks,

Tom

>
> Thank you,
>
>

\
 
 \ /
  Last update: 2019-12-19 17:25    [W:0.086 / U:3.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site