Alright, I think I got it.

I propose changing the cpufreq.c DTrace script for this one, that fires 
a profile-Ns probe at every interval and does all the accounting in a 
much simpler way.

Running this one on the command line with a 3 second interval I get the 
following values:

cpu   1 state  1330 res 531
cpu   0 state  1330 res 532
cpu   0 state   800 res 2448
cpu   1 state   800 res 2449

cpu   0 state  1330 res 823
cpu   1 state  1330 res 823
cpu   0 state   800 res 2189
cpu   1 state   800 res 2189

cpu   1 state  1330 res 1221
cpu   0 state  1330 res 1222
cpu   0 state   800 res 1724
cpu   1 state   800 res 1726

The jitter is very small, and the solution is much cleaner than the 
current one.

Rafael



Rafael Vanoni wrote:
> Hey guys
> 
> Kuriakose raised some good questions on a 'post code review' 
> conversation. There are two, somewhat related problems :
> 
> (a) the code (pt_cpufreq_dtrace_walk() and pt_cpufreq_dtrace_account()) 
> assume that the value stored in time_accounted during the last interval 
> belongs to the first aggregate entry we walk when collecting data for 
> the current interval.
> 
> (b) we should *not* return if the dt_state_time is zero. We need to 
> continue down walk() and do the accounting.
> 
> Consider the following transitions:
> 
>      t-2                      t-1                      t
>       |------------------------|-----------------------|
> 900   |              ----------|--------\              |
> 800   |         ----/          |         ----\         |
> 700   |--------/               |              ---------|
> 
> which generate the following aggregations (speed, residency):
> 
> 700 2     900 4
> 800 1     800 1
> 
> We have (because of (b)):
> 
>                    walk()     account() |    walk()     account()
>                  700    800      900    |  700    800      900
> dt_state_time     2      1        -     |   0      1
> total_time        2      1        2     |   -      0*
> dtrace_time       2      3        2     |   -      0*
> time_accounted    0      0        2     |   -      2
> 
> But prior to (b) we still had
> 
>                    walk()     account() |    walk()     account()
>                  700    800      900    |  700    800      900
> dt_state_time     2      1        -     |   0
> total_time        2      1        2     |   0      ...
> dtrace_time       2      3        3     |   0
> time_accounted    0      0        2     |   0**
> 
> ** cpuidle.c:525
> 
> So we're expecting the aggregation to be ordered descendingly by the 
> residency at each state, which is not true. It's ordered by its keys.
> 
> I'm working on a fix for this now.
> 
> Thanks,
> Rafael
> 
> 
> _______________________________________________
> tesla-dev mailing list
> tesla-dev at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/tesla-dev

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: cpufreq.d
URL: 
<http://mail.opensolaris.org/pipermail/tesla-dev/attachments/20090505/5969451c/attachment.ksh>

Reply via email to