Hi Aubrey
Thanks for your comments, see mine inline.
Li, Aubrey wrote:
> Rafael Vanoni wrote:
>
>> Aubrey Li wrote:
>>
>> I'm working on figuring out what other kinds of events are causing
>> wake-ups. Hope to have something for you guys soon.
>>
>
> Thanks for the so quick work, :-)
>
> ["-C" option] is interesting:
>
> 12.0% (58.5) <kernel> genunix`cyclic_timer
> 8.5% (41.6) <kernel> genunix`clock
> 0.2% ( 1.2) <kernel> unix`cbe_hres_tick
>
> the sum of these functions is ~100. That's the tick frequency.
> But take cbe_hres_tick as an example,
> it looks like cbe_hres_tick is called only one time per second.
> Actually it's called once per tick, every nsec_per_tick ns.
> So this is not correct, please remove this option, :-)
I agree it looks a bit strange, but it is an artifact of the cyclic
subsystem batch processing cyclics. It just happens that
unix`cbe_hres_tick is usually fired after `clock and `cyclic_timer. So
the DTrace script is only reporting the first event per expire timestamp.
I think this is a good feature for developers interested in the cyclic
subsystem, and the behavior could be explained in the manual. What do
you think ?
> ["-c" option] keeps the current script, but I think help message is not
> okay.
>
> "-c --cyclic report cyclic subsystem activety\n"
Hmm.. did you import my patch? That's not what I'm seeing here, could
you double check?
> It would be better to use this option to place powertop into the verbose
> mode,
> reporting more events in the kernel.
>
> [default option], I'd like to do the following change, because clock
> event is missing.
>
> static const char *g_prog_A =
> ----snip----
> +++ "sdt:::cyclic-start"
> +++ "/(caddr_t)((cyclic_t *)arg0)->cy_handler == (caddr_t)&`clock/"
> +++ "{"
> +++" @events_k[(caddr_t)((cyclic_t *)arg0)->cy_handler] =
> count(); "
> +++"}";
That goes a little bit against your first option of not reporting
cyclics at all :) But I agree with you.
thanks
Rafael
