Messages in this thread | | | Subject | Re: [PATCH v4 4/7] tracing: Add support for dynamic strings to synthetic events | From | Tom Zanussi <> | Date | Thu, 08 Oct 2020 15:33:41 -0500 |
| |
Hi Masami,
On Thu, 2020-10-08 at 18:22 +0900, Masami Hiramatsu wrote: > Hi Tom, > > On Sun, 4 Oct 2020 17:14:06 -0500 > Tom Zanussi <zanussi@kernel.org> wrote: > > > Currently, sythetic events only support static string fields such > > as: > > > > # echo 'test_latency u64 lat; char somename[32]' > > > /sys/kernel/debug/tracing/synthetic_events > > > > Which is fine, but wastes a lot of space in the event. > > > > It also prevents the most commonly-defined strings in the existing > > trace events e.g. those defined using __string(), from being passed > > to > > synthetic events via the trace() action. > > > > With this change, synthetic events with dynamic fields can be > > defined: > > > > # echo 'test_latency u64 lat; char somename[]' > > > /sys/kernel/debug/tracing/synthetic_events > > > > And the trace() action can be used to generate events using either > > dynamic or static strings: > > > > # echo 'hist:keys=name:lat=common_timestamp.usecs- > > $ts0:onmatch(sys.event).test_latency($lat,name)' > > > /sys/kernel/debug/tracing/events > > > > The synthetic event dynamic strings are implemented in the same way > > as > > the existing __data_loc strings and appear as such in the format > > file. > > > > [ <rostedt@goodmis.org>: added __set_synth_event_print_fmt() > > changes: > > > > I added the following to make it work with trace-cmd. Dynamic > > strings > > must have __get_str() for events in the print_fmt otherwise it > > can't be > > parsed correctly. ] > > I confirmed this works, but have some questions; > - It seems no error message when we failed to define with wrong > syntax
Yes, I need to add an error message for this (as well as others in this file).
> > /sys/kernel/debug/tracing # echo 'myevent char name []' >> > synthetic_events > sh: write error: Invalid argument > /sys/kernel/debug/tracing # cat error_log > /sys/kernel/debug/tracing # > > - what we write and what we see in synthetic_events are different > > /sys/kernel/debug/tracing # echo 'myevent char name[]' >> > synthetic_events > /sys/kernel/debug/tracing # cat synthetic_events > myevent __data_loc char[] name > > - And it is not able to re-define with that syntax > > /sys/kernel/debug/tracing # echo > synthetic_events > /sys/kernel/debug/tracing # echo 'myevent __data_loc char[] name' >> > synthetic_events > sh: write error: Invalid argument > /sys/kernel/debug/tracing # >
Yes, only the format needs to have the __data_loc, but the event description shouldn't.
> - It seems to accept wrong name for variables > > /sys/kernel/debug/tracing # echo 'myevent char name]' >> > synthetic_events > /sys/kernel/debug/tracing # echo 'myevent2 char name;[]' >> > synthetic_events > /sys/kernel/debug/tracing # cat synthetic_events > myevent char name] > myevent2 __data_loc char[] name; >
Yes, I think all these other errors boil down to allowing illegal names. Applying your is_good_name() function for probe events to these fields should get rid of them.
> Some of those issues are not introduced from this series. I think > we'd better fix those before introducing this series so that > we can backport it to stable kernels. >
I should have patches addressing all of these shortly, tomorrow at the latest.
Thanks,
Tom
> Thank you, > >
| |