2004-11-09 20:03:17

by Alexander Clouter

[permalink] [raw]
Subject: [PATCH] cpufreq_(ondemand|conservative) (round three)

Evening All,

Well again I received more suggestions (thanks), a few tweaks have been added
and it was asked of me to produce diff's for 2.6.10-rc1-mm3 which I have; due
to changes in the cpufreq code the borked my patches and there is now a
different approach being used to return failed /sys changes that I needed to
amend for.

All the 2.6.9 patches to ondemand are in the 'ondemand' style of coding, or
the best I have managed to keep to. I kept with the 2.6.9 'style' for my
'conservative' patch also.

For 2.6.10-rc1-mm3 I re-wrote the little bits of the patches to fit in with
the amended style, but I also included a 'consistency' patch as some of the
/sys functions already present in the 'ondemand' governor did not 'look' like
the others. Again my 'conservative' governor has been tweaked to match.

What about this time? Still lacking any reports of "Works for Me(tm)" and
"Damn Your Code Sucks(tm)" which is a shame.... :)

Cheers all

Alex

--
________________________________________
/ Pray to God, but keep rowing to shore. \
| |
\ -- Russian Proverb /
----------------------------------------
\ ^__^
\ (oo)\_______
(__)\ )\/\
||----w |
|| ||


Attachments:
(No filename) (0.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2004-11-11 01:27:21

by Pallipadi, Venkatesh

[permalink] [raw]
Subject: RE: [PATCH] cpufreq_(ondemand|conservative) (round three)


Thanks for the patches. Here are some comments about _ondemand patches.

(1) 00_consistency patch
I think it is OK to do this for sampling_rate. But, we may have some
nasty races / corner conditions if we do this for up_threashold and
down_threshold. The race I am thinking about is: we check the new
down_value with old up_value and we may end up finally with
down_threshold greater than up_threshold.

(2) 01_ignore-nice
The idea to have this is good. Somehow I am thinking of some corner
cases here too.
If the prev_cpu_idle_up and prev_cpu_idle_down have unconditionally
include nice and total_ticks includes it conditionally, then we cannot
do the proper subtract and compare of idle times. Am I missing anything
here?

(3) 02_check-rate-and-break-out
This looks good and ready to go.

(4) 03_sys_freq_step
Looks good. One minor issue. Policy->max can change at run time (when
a/c power and battery power). So behaviour might change if you
initialize freq_step once instead of checking 5% of max during each
switching. But, doing it this way should be OK too, as 5% is not a
strict number.

Thanks,
Venki