[tesla-dev] last five bugs listed on the page

[Rafael Vanoni]

Li, Aubrey wrote:
> Rafael.Vanoni wrote:
> 
>> Li, Aubrey wrote:
>>> tesla-dev-bounces at opensolaris.org wrote:
>>>
>>>> Aubrey Li wrote:
>>>>> [snip]
>>>>>>> That is the behavior in the original linux implementation.
>>>>>>> Actually there is a mechanism to auto-adjust the ticktime, but it
>>>>>>> has been removed to powertop prototype quickly delivered, it
>>>>>>> would be great if it can be recovered. Think about the future
>>>>>>> tickless kernel, the cpu wakeup times become less, making the
>>>>>>> ticktime longer makes more sense if the wakeup events is few in
>>>>>>> the current sampling period.
>>>>>> So you mean adapting the interval based on the amount of wakeups -
>>>>>> i.e few wakeups, higher interval and vice-versa ?
>>>>>>
>>>>>> I'll have a look at the original code.
>>>>>>
>>>>> That would be great, thanks, :-)
>>>> Ok, the Linux version sets maxsleep (below) to the duration of the
>>>> last transition and then sets the interval according to
>>>>
>>>> 		if (maxsleep < 5.0)
>>>> 			ticktime = 10;
>>>> 		else if (maxsleep < 30.0)
>>>> 			ticktime = 15;
>>>> 		else if (maxsleep < 100.0)
>>>> 			ticktime = 20;
>>>> 		else if (maxsleep < 400.0)
>>>> 			ticktime = 30;
>>>> 		else
>>>> 			ticktime = 45;
>>>>
>>>>
>>>> There's also
>>>>
>>>> 		if (wakeups_per_second < 0)
>>>> 			ticktime = 2;
>>>>
>>>> How about adding this functionality as the default option, and
>>>> fixing the interval if -t is used ? 
>>>>
>>> Sounds great to me.
>> Cool, here's a patch for these changes.
>>
> 
> +		last_time =
> (((double)cstate_info[longest_cstate].total_time/g_ncpus)/total_events);

This is the same expression used for the Avg residency column in the 
c-state window. longest_cstate is the index for the c-state in which the 
cpu was for the longest time during the last snapshot.

> I failed to understand what does last_time mean. the front part is the
> longest cstate residency per cpu.
> why "/total_events" here?
> 
> +		if (!user_interval) {
> +			if (last_time < 5.0 || total_events/ticktime <
> 0)

This is the same as Linux's

		if (wakeups_per_second < 0)
			ticktime = 2;

only in the same expression.

> 
> Did you notice the unit of total_time is ns? how big last_time here?

The cpu_idle dtrace script calculates residency time with

@times[self->state] = sum((timestamp - self->start)/1000000);

so it's actually in ms.

> And, is it possible (total_events/ticktime < 0)??
> +				ticktime = 2;
> +			else if (last_time < 10.0)
> +				ticktime = 7;
> +			else if (last_time < 50.0)
> +				ticktime = 35;
> +			else if (last_time < 100.0)
> +				ticktime = 75;
> +			else
> +				ticktime = 100;
> These ticktime value looks weird, what are they based?

Yes, I was hoping you'd pick this up :)

Linux sets values statically in the code, I changed them to get some 
readings and these seemed appropriate given the behavior we currently 
see in snv. What do you think we should do here ?

thanks
Rafael