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 <zanu...@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. > > > > [ <rost...@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, > >
