Hi Masami,
On Sun, 2020-10-11 at 00:03 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> On Fri, 9 Oct 2020 10:17:07 -0500
> Tom Zanussi <zanu...@kernel.org> wrote:
>
> > For synthetic event dynamic fields, the type contains "__data_loc",
> > which is basically an internal part of the type which is only meant
> > to
> > be displayed in the format, not in the event description itself,
> > which
> > is confusing to users since they can't use __data_loc on the
> > command-line to define an event field, which printing it would lead
> > them to believe.
> >
> > So filter it out from the description, while leaving it in the
> > type.
> >
>
> OK, I confirmed this removes __data_loc from synth_events interface.
> However, I also found another issue.
>
> /sys/kernel/debug/tracing # echo "myevent char str[]; int v" >>
> synthetic_events
> /sys/kernel/debug/tracing # cat synthetic_events
> myevent char[]; str; int v
>
> It seems that the type "char[]" includes ";" as a type, this results
>
Yeah, this isn't a result of this patchset - it's a different bug which
I'll submit a new fix for. Basically in the array case it doesn't
effectively strip off trailing characters when creating the array type.
Thanks,
Tom
>
> /sys/kernel/debug/tracing # cat events/synthetic/myevent/format
> name: myevent
> ID: 1220
> format:
> field:unsigned short common_type; offset:0; size:2; signe
> d:0;
> field:unsigned char common_flags; offset:2; size:1; signe
> d:0;
> field:unsigned char common_preempt_count; offset:3; size:
> 1; signed:0;
> field:int common_pid; offset:4; size:4; signed:1;
>
> field:__data_loc char[]; str; offset:8; size:8; signe
> d:1;
> field:int v; offset:16; size:4; signed:1;
>
> print fmt: "str=%.*s, v=%d", __get_str(str), REC->v
>
>
> As you can see, the field type has ";" in format file too. This will
> prevent
> parsing event information correctly.
> I also try to remove ";" as below, it seems to work correctly.
>
> /sys/kernel/debug/tracing # echo "myevent char[] str; int v" >>
> synthetic_events
> /sys/kernel/debug/tracing # cat events/synthetic/myevent/format
> name: myevent
> ID: 1221
> format:
> field:unsigned short common_type; offset:0; size:2; signe
> d:0;
> field:unsigned char common_flags; offset:2; size:1; signe
> d:0;
> field:unsigned char common_preempt_count; offset:3; size:
> 1; signed:0;
> field:int common_pid; offset:4; size:4; signed:1;
>
> field:__data_loc char[] str; offset:8; size:8; signe
> d:1;
> field:int v; offset:16; size:4; signed:1;
>
> print fmt: "str=%.*s, v=%d", __get_str(str), REC->v
>
>
> Thank you,
>
> > Reported-by: Masami Hiramatsu <mhira...@kernel.org>
> > Signed-off-by: Tom Zanussi <zanu...@kernel.org>
> > ---
> > kernel/trace/trace_events_synth.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace_events_synth.c
> > b/kernel/trace/trace_events_synth.c
> > index 3b2dcc42b8ee..b19e2f4159ab 100644
> > --- a/kernel/trace/trace_events_synth.c
> > +++ b/kernel/trace/trace_events_synth.c
> > @@ -1867,14 +1867,22 @@ static int __synth_event_show(struct
> > seq_file *m, struct synth_event *event)
> > {
> > struct synth_field *field;
> > unsigned int i;
> > + char *type, *t;
> >
> > seq_printf(m, "%s\t", event->name);
> >
> > for (i = 0; i < event->n_fields; i++) {
> > field = event->fields[i];
> >
> > + type = field->type;
> > + t = strstr(type, "__data_loc");
> > + if (t) { /* __data_loc belongs in format but not event
> > desc */
> > + t += sizeof("__data_loc");
> > + type = t;
> > + }
> > +
> > /* parameter values */
> > - seq_printf(m, "%s %s%s", field->type, field->name,
> > + seq_printf(m, "%s %s%s", type, field->name,
> > i == event->n_fields - 1 ? "" : "; ");
> > }
> >
> > --
> > 2.17.1
> >
>
>
