Messages in this thread | | | Date | Thu, 30 May 2013 20:14:51 +0300 | From | Stratos Karafotis <> | Subject | Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load |
| |
On 05/30/2013 01:29 AM, Rafael J. Wysocki wrote: > On Wednesday, May 29, 2013 06:15:56 PM Stratos Karafotis wrote: >> On 05/28/2013 11:54 PM, Rafael J. Wysocki wrote: >>> On Tuesday, May 28, 2013 08:03:19 PM Stratos Karafotis wrote: >>>> I mean any value of freq table. Please let me know if you want me to rephrase >>>> it in description. >>> >>> Yes, it would be nice to be more precise. >> >> OK sure, I will add a more precise description. >> >> >>> Which is equivalent to saying that __cpufreq_driver_getavg() is not useful and >>> may be removed (but the patch doesn't do that and I wonder why?), but surely >>> the developer who added it wouldn't agree. >>> >>> So, please explain: why can we drop __cpufreq_driver_getavg()? >>> >> >> With the new proposed method the next frequency is not dependent from current >> or average frequency. The next frequency is dependent only from load. >> So, we don't need support for freq feedback from hardware anymore. > > OK, but that's a more significant change than the changelog suggests. > The changelog should tell the whole story and explain the rationale. :-) > > So, please explain that in fact it is not necessary to use the current or > average frequency to compute the target and why that is the case. > > Also the patch should remove __cpufreq_driver_getavg() and the callback used by > it, since that code will be dead after applying it anyway. > >> Even if, due to underlying hardware coordination mechanism, CPU runs in different >> frequency than the actual, the calculation of load and of target frequency will >> remain the unaffected, with this patch. >> >> With full respect to ondemand coauthor, and if the new method is acceptable, >> I could send a patch to revert the original one about the IA32_APERF and >> IA32_MPERF MSR support. > > I'm not sure what you mean by "revert", but please do as I said above. > >>>> Thus, in the comparison with up_threshold to increase frequency we actually >>>> do this (in cases that getavg is not implemented): >>>> if (load > up_theshold) >>>> increase to max >>>> >>>> So, after the patch we keep the same comparison to decide about the max frequency. >>>> I thought, that below up_threshold is 'fair' to decide about the next >>>> frequency with formula that frequency is proportional to load. >>>> For example in a CPU with min freq 100MHz and max 1000MHz with a >>>> load 50 target frequency should be 500MHz. >>> >>> Well, that sounds reasonable, but the patch actually does more than that. >> >> I'm sorry, but I didn't understand your last point. > > Please see above. > > The changelog doesn't even mention that the code is being switched from using > measured past frequencies to not using them, because you think that there's a > better way of computing the target (which by the way I can agree with :-)). > > Thanks, > Rafael > >
OK, I will send a new patch that includes your corrections and suggestions.
Thanks so much for your time and your comments!
Stratos
| |