[tesla-dev] [PATCH] for bug: C-state statistics seem a bit off

[Li, Aubrey]

Eric Saxe wrote:

> Hi Aubrey,
> 
> One concern I have looking ahead, is around a similar issue to what we
> ran into with the P-states...that when the tool first starts, the
> initial state is unknown. For P-states, we get the initial
> state through
> the kstats...but for c-states we dont...and so that first
> interval until
> we make a state transition may be wrong...which is what I was
> suspecting could be wrong with what I was seeing. Granted, these
> state changes happen fairly frequently now...but as we support deeper
> states, one could imagine that a processor would (hopefully) spend
> quite a bit of time in a given state. I'm wondering if we should
> provide a 
> way via the
> kstats to get the CPU's current idle state... Thoughts?
> 
It's different from P-state, I think. P-state will not change when we
get P-state initial state 
To get the current idle state, CPU must be in C0. So if it's in C1, the
obtaining changed it. 
So I think we can just ignore the initial gap. It is not bigger than
10ms. As for the default
and reasonable sampling period, it's less than 0.2%.
What do you think? ;-)

Thanks,
-Aubrey

> Eric Saxe wrote:
>> Thanks Aubrey,
>> 
>> I was out of town Thursday and Fri, and I didn't get a chance to try
>> your changes... I'll let you know if it rectifies the issue I was
>> seeing... 
>> 
>> -Eric
>> 
>> Li, Aubrey wrote:
>> 
>>> Li, Aubrey wrote:
>>> 
>>> 
>>> 
>>>> I failed to replicate this issue on my side.
>>>> But from my point of view, we didn't do anything wrong.
>>>> So, is the following patch acceptable for this bug?
>>>> ======================================================
>>>> diff -r 2fb476052291 usr/src/cmd/powertop/display.c
>>>> --- a/usr/src/cmd/powertop/display.c	Tue Feb 12 11:55:53 2008
-0800
>>>> +++ b/usr/src/cmd/powertop/display.c	Tue Feb 19 23:08:25 2008
+0800
>>>> @@ -185,6 +185,9 @@ void show_cstates(double interval)
>>>> 
>>>> 	for (i=0; i< g_ncpus; i++)
>>>> 		total_c1 += cstate_info[i].total_time;
>>>> +
>>>> +	if (total_c1 > interval * g_ncpus)
>>>> +		total_c1 = interval * g_ncpus;
>>>> 
>>>> 	print(cstate_window, 0, 0, "%s", "Cn\t\t\tAvg	residency\n");
>>>> 	sprintf(c, "C0 (cpu running)\t\t(%.1f%%)\n",
>>>> =======================================================
>>>> 
>>>> 
>>> I committed this patch since no objection.
>>> 
>>> Thanks,
>>> -Aubrey
>>> _______________________________________________
>>> tesla-dev mailing list
>>> tesla-dev at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>>> 
>>> 
>> 
>> _______________________________________________
>> tesla-dev mailing list
>> tesla-dev at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev