Li, Aubrey wrote:
> Rafael.Vanoni wrote:
>
>> Li, Aubrey wrote:
>>> Rafael.Vanoni wrote:
>>>
>>>> I've updated the webrev @
>>>> http://cr.opensolaris.org/~rafaelv/ptop-freq/
>>>>
>>>> I'll file a separate CR to change the DTrace script in cpufreq.c, we
>>>> have other issues I think are more important at this time.
>>>>
>>>> This latest webrev is against PowerTOP's gate, not ON. I've added a
>>>> check to make sure we're deducting the accounted time for the
>>>> correct p-state. Let me know what you think.
>>>>
>>> I don't think the patch is doing the right thing to deal with
>>> accounted time.
>>>
>>> Example:
>>>
>>> Round1: 5 seconds
>>> 1) 2s in P1
>>> 2) 3s in P0
>>>
>>> Round2: 5 seconds
>>> 1) 3s in P0
>>> 2) 2s in P1
>>>
>>> Here, in the second round, accounted time is apparantly for P0, but
>>> cpu_pow->current_pstate is P1. So the suggested patch is still wrong.
>> What accounted time? Your scenario is self contained. In both cases,
>> time_accounted would be zero since the aggregation results account
>> for the entire interval.
>>
>> Rafael
Ah.. sorry, the cpufreq aggregation is ordered by key so I misread your
example.
> In Round1, time_accounted would be ZERO and dtrace script only capture
> one time probe firing. the aggregation should be
> "P1 2s"
>
> And in Round2, the dtrace probe will be fired only once as well and the
> aggregation
> should be
> "P0 6s"
>
> After dtrace walk, pt_cpufreq_snapshot() get the current p-state is P1.
> And then, pt_cpufreq_stat_account() get the time_accounted is 3s(Round1 P0
> residency,
> which is not captured by dtrace script).
time_accounted won't be 3 by the time account() is called, it will have
been deducted from 6 and then zero'ed.
> And, the suggested patch set the
> cpu_pow->speed_accounted = speed(P1). So, here time_accounted will be
> accounted on P1.
>
> Is this expected?
This is how I expect this to work:
P0 | ---|---\ |
P1 |--/ | --|
P0 0 6
P1 2 0
walk() account()| walk() account()
P0 P1 P0 | P0 P1 P1
res 0 2 - | 3 0 -
total_time 0 2 3 | 3 0 2
dtrace_time 0 2 - | 3 3 -
time_accounted 0 0 3 | 0 0 2
speed_accounted 0 0 P0 | 0 0 P1
duration - - 3 | - - 2
I don't see a problem here.. am I missing something ?
I've posted a test program that simulates all of this at the Self
Testing page. I've used it to test this patch.
Rafael
