Subject: Re: [PATCH 5/5] tracing: Add 'hash' event trigger command

Hi Tom,

(2014/03/27 13:54), Tom Zanussi wrote:
> Hash triggers allow users to continually hash events which can then be
> dumped later by simply reading the trigger file. This is done
> strictly via one-liners and without any kind of programming language.
>
> The syntax follows the existing trigger syntax:
>
> # echo hash:key(s):value(s)[:sort_keys()][ if filter] > event/trigger
>
> The values used as keys and values are just the fields that define the
> trace event and available in the event's 'format' file. For example,
> the kmalloc event:
>
> root@ie:/sys/kernel/debug/tracing/events/kmem/kmalloc# cat format
> name: kmalloc
> ID: 370
> format:
> field:unsigned short common_type; offset:0; size:2; signed:0;
> field:unsigned char common_flags; offset:2; size:1; signed:0;
> field:unsigned char common_preempt_count; offset:3; size:1;signed:0;
> field:int common_pid; offset:4; size:4; signed:1;
>
> field:unsigned long call_site; offset:8; size:4; signed:0;
> field:const void * ptr; offset:12; size:4; signed:0;
> field:size_t bytes_req; offset:16; size:4; signed:0;
> field:size_t bytes_alloc; offset:20; size:4; signed:0;
> field:gfp_t gfp_flags; offset:24; size:4; signed:0;
>
> The key can be made up of one or more of these fields and any number of
> values can specified - these are automatically tallied in the hash entry
> any time the event is hit. Stacktraces can also be used as keys.
>
> For example, the following uses the stacktrace leading up to a kmalloc
> as the key for hashing kmalloc events. For each hash entry a tally of
> the bytes_alloc field is kept. Dumping out the trigger shows the sum
> of bytes allocated for each execution path that led to a kmalloc:
>
> # echo 'hash:call_site:bytes_alloc' > /sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
> # cat /sys/kernel/debug/tracing/events/kmem/kmalloc/trigger

I like the basic idea :) but I'm confused the interface what you're introduced.
I suppose that the "trigger" file is for control triggers on the event, so that
user can check what trigger rules are set on the event and remove it.
But in this patch, that is also used for a data path.

I'd like to suggest adding new "hash" file under events/GROUP/EVENT/, which is
only for dumping the hash data, and keep the "trigger" as a control path.
This makes users easier to build their own tools on the ftrace facility.

Thank you,


--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]


2014-04-03 22:43:25

by Tom Zanussi

[permalink] [raw]
Subject: Re: [PATCH 5/5] tracing: Add 'hash' event trigger command

Hi Masami,

On Thu, 2014-04-03 at 17:59 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> (2014/03/27 13:54), Tom Zanussi wrote:
> > Hash triggers allow users to continually hash events which can then be
> > dumped later by simply reading the trigger file. This is done
> > strictly via one-liners and without any kind of programming language.
> >
> > The syntax follows the existing trigger syntax:
> >
> > # echo hash:key(s):value(s)[:sort_keys()][ if filter] > event/trigger
> >
> > The values used as keys and values are just the fields that define the
> > trace event and available in the event's 'format' file. For example,
> > the kmalloc event:
> >
> > root@ie:/sys/kernel/debug/tracing/events/kmem/kmalloc# cat format
> > name: kmalloc
> > ID: 370
> > format:
> > field:unsigned short common_type; offset:0; size:2; signed:0;
> > field:unsigned char common_flags; offset:2; size:1; signed:0;
> > field:unsigned char common_preempt_count; offset:3; size:1;signed:0;
> > field:int common_pid; offset:4; size:4; signed:1;
> >
> > field:unsigned long call_site; offset:8; size:4; signed:0;
> > field:const void * ptr; offset:12; size:4; signed:0;
> > field:size_t bytes_req; offset:16; size:4; signed:0;
> > field:size_t bytes_alloc; offset:20; size:4; signed:0;
> > field:gfp_t gfp_flags; offset:24; size:4; signed:0;
> >
> > The key can be made up of one or more of these fields and any number of
> > values can specified - these are automatically tallied in the hash entry
> > any time the event is hit. Stacktraces can also be used as keys.
> >
> > For example, the following uses the stacktrace leading up to a kmalloc
> > as the key for hashing kmalloc events. For each hash entry a tally of
> > the bytes_alloc field is kept. Dumping out the trigger shows the sum
> > of bytes allocated for each execution path that led to a kmalloc:
> >
> > # echo 'hash:call_site:bytes_alloc' > /sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
> > # cat /sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
>
> I like the basic idea :) but I'm confused the interface what you're introduced.
> I suppose that the "trigger" file is for control triggers on the event, so that
> user can check what trigger rules are set on the event and remove it.
> But in this patch, that is also used for a data path.
>
> I'd like to suggest adding new "hash" file under events/GROUP/EVENT/, which is
> only for dumping the hash data, and keep the "trigger" as a control path.
> This makes users easier to build their own tools on the ftrace facility.
>

I was really trying to avoid adding a new file - my thinking was that
the trigger file is just sitting there doing nothing besides either
listing available triggers when inactive or listing active triggers when
active, which it would still do even if also providing a conduit for the
output.

I agree that it would be cleaner to have a separate file, but I don't
know if it's worth a dedicated file. Another possibility would be to
have it exist only when a hash trigger is active..

Tom


> Thank you,
>
>

Subject: Re: Re: [PATCH 5/5] tracing: Add 'hash' event trigger command

(2014/04/04 7:43), Tom Zanussi wrote:
> Hi Masami,
>
> On Thu, 2014-04-03 at 17:59 +0900, Masami Hiramatsu wrote:
>> Hi Tom,
>>
>> (2014/03/27 13:54), Tom Zanussi wrote:
>>> Hash triggers allow users to continually hash events which can then be
>>> dumped later by simply reading the trigger file. This is done
>>> strictly via one-liners and without any kind of programming language.
>>>
>>> The syntax follows the existing trigger syntax:
>>>
>>> # echo hash:key(s):value(s)[:sort_keys()][ if filter] > event/trigger
>>>
>>> The values used as keys and values are just the fields that define the
>>> trace event and available in the event's 'format' file. For example,
>>> the kmalloc event:
>>>
>>> root@ie:/sys/kernel/debug/tracing/events/kmem/kmalloc# cat format
>>> name: kmalloc
>>> ID: 370
>>> format:
>>> field:unsigned short common_type; offset:0; size:2; signed:0;
>>> field:unsigned char common_flags; offset:2; size:1; signed:0;
>>> field:unsigned char common_preempt_count; offset:3; size:1;signed:0;
>>> field:int common_pid; offset:4; size:4; signed:1;
>>>
>>> field:unsigned long call_site; offset:8; size:4; signed:0;
>>> field:const void * ptr; offset:12; size:4; signed:0;
>>> field:size_t bytes_req; offset:16; size:4; signed:0;
>>> field:size_t bytes_alloc; offset:20; size:4; signed:0;
>>> field:gfp_t gfp_flags; offset:24; size:4; signed:0;
>>>
>>> The key can be made up of one or more of these fields and any number of
>>> values can specified - these are automatically tallied in the hash entry
>>> any time the event is hit. Stacktraces can also be used as keys.
>>>
>>> For example, the following uses the stacktrace leading up to a kmalloc
>>> as the key for hashing kmalloc events. For each hash entry a tally of
>>> the bytes_alloc field is kept. Dumping out the trigger shows the sum
>>> of bytes allocated for each execution path that led to a kmalloc:
>>>
>>> # echo 'hash:call_site:bytes_alloc' > /sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
>>> # cat /sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
>>
>> I like the basic idea :) but I'm confused the interface what you're introduced.
>> I suppose that the "trigger" file is for control triggers on the event, so that
>> user can check what trigger rules are set on the event and remove it.
>> But in this patch, that is also used for a data path.
>>
>> I'd like to suggest adding new "hash" file under events/GROUP/EVENT/, which is
>> only for dumping the hash data, and keep the "trigger" as a control path.
>> This makes users easier to build their own tools on the ftrace facility.
>>
>
> I was really trying to avoid adding a new file - my thinking was that
> the trigger file is just sitting there doing nothing besides either
> listing available triggers when inactive or listing active triggers when
> active, which it would still do even if also providing a conduit for the
> output.

You don't need to avoid it unless it is really meaningless :)
Since the available triggers are limited and it doesn't relay on event
type, I think it is enough to prepare tracing/available_triggers.

> I agree that it would be cleaner to have a separate file, but I don't
> know if it's worth a dedicated file. Another possibility would be to
> have it exist only when a hash trigger is active..

Agreed. That's a good idea :)

Thank you,


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]