On 27-06-17, 02:15, Rafael J. Wysocki wrote:
On Monday, May 22, 2017 04:57:27 PM Viresh Kumar wrote:
On 22-05-17, 19:17, Leo Yan wrote:
This afternoon Amit pointed me for this patch, should fix as below? Otherwise it seems directly assign the same value from unit 'ns' to 'us' but without any value conversion.
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index 76877a6..dcc90fc 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -538,7 +538,7 @@ static int sugov_init(struct cpufreq_policy *policy) unsigned int lat; tunables->rate_limit_us = LATENCY_MULTIPLIER;
lat = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
I think the above line is just fine and the below one is incorrect, as we wanted to convert transition latency to usec here (i.e. in the units of rate_limit_us).
lat = policy->cpuinfo.transition_latency / NSEC_PER_MSEC; if (lat) tunables->rate_limit_us *= lat; }I will let Rafael comment in as well. NSEC_PER_USEC is used in the earlier governors as well (ondemand/conservative) in exactly the same way as schedutil is using.
The reason why it is used by schedutil is because the other governors used it that way. IOW, doesn't matter. :-)
But I feel the value of LATENCY_MULTIPLIER (1000) is way too high. It currently says that if freq-switching takes time X, then we should wait for 999X time before we change the freq again.
Perhaps LATENCY_MULTIPLIER should be just 10 or 20 here. For a platform with transition_latency 500 us, rate_limit_us comes to 500 ms. Which is absurd. We ideally want it to be around 10-20 ms here. And compared to other ARM platforms, 500 us transition_latency is very low. It normally is around 1-3 ms for ARM32 platforms.
@Rafael: Will it be fine to lower down the value of LATENCY_MULTIPLIER?