Rafael Vanoni wrote:
> 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.
Yeah, I see the point, it always count the first cyclic being batch
processed.
And it proves the cyclic is really being batch processed.(sum= tick
freq).
>
> 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 ?
I'm sorry I still can't agree with you, :-)
because the report is wrong. the frequency is wrong, and from the
report,
We can't see cyclic is being batch processed, that means cyclic behavior
is wrong.
Both if you persist it's a good feature, you have to persuade me why,
:-)
>
>> ["-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?
>
================================================
[aubrey-laptop at amd64]./powertop -h
Usage: powertop [OPTION]
-d, --dump Read wakeups once and print list of top
offenders
-t, --time=DOUBLE Default time to gather data in seconds, ranging
between 1 and 100s
-h, --help Show this help message
-c, --cyclic Report cyclic subsystem activity
-C, --collapse Collapse events that are batch processed by the
cyclic subsyste
================================================
It would be better something like this, instead of just cyclic
----snip----
-v, --verbose show more kernel events
----snip----
>> 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.
er..., I don't want to see the batch processed cyclic, but I still
want to see the clock frequency, so clock is the best one, :-)
Thanks,
-Aubrey
