Bug 177171 - Reduced frequency window for conservative governor
Summary: Reduced frequency window for conservative governor
Alias: None
Product: Power Management
Classification: Unclassified
Component: cpufreq (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: linux-pm@vger.kernel.org
Depends on:
Reported: 2016-10-10 15:43 UTC by Aleksei Rybalkin
Modified: 2016-10-14 23:43 UTC (History)
2 users (show)

See Also:
Kernel Version: 4.8
Tree: Mainline
Regression: Yes

My kernel config (107.44 KB, application/octet-stream)
2016-10-10 15:45 UTC, Aleksei Rybalkin
cpufreq: conservative: Fix next frequency selection (2.39 KB, patch)
2016-10-11 21:22 UTC, Rafael J. Wysocki
Details | Diff
cpufreq: conservative: Fix next frequency selection (2.64 KB, patch)
2016-10-11 21:24 UTC, Rafael J. Wysocki
Details | Diff

Description Aleksei Rybalkin 2016-10-10 15:43:23 UTC

Since 4.8 the conservative governor has a reduced frequency window on my machine (ThinkPad X250), despite correct values for scaling_{min,max}_freq

$ cat /sys/devices/system/cpu/cpufreq/policy0/scaling_{min,max}_freq

The actual window became from 2000000 to 2200000 with accordingly more heating and more cpu fan activations, which is not optimal.

I have bisected the problem to this commit: 

commit d352cf47d93e39494b44b792cca8d35a3a0bd9b3
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Date:   Mon Jun 13 23:33:49 2016 +0200

    cpufreq: conservative: Do not use transition notifications

    The conservative governor registers a transition notifier so it
    can update its internal requested_freq value if it falls out of the
    policy->min...policy->max range, but requested_freq is not really

    That value is used to track the frequency requested by the governor
    previously, but policy->cur can be used instead of it and then the
    governor will not have to worry about updating the tracked value when
    the current frequency changes independently (for example, as a result
    of min or max changes).

    Accodringly, drop requested_freq from struct cs_policy_dbs_info
    and modify cs_dbs_timer() to use policy->cur instead of it.
    While at it, notice that __cpufreq_driver_target() clamps its
    target_freq argument between policy->min and policy->max, so
    the callers of it don't have to do that and make additional
    changes in cs_dbs_timer() in accordance with that.

    After these changes the transition notifier used by the conservative
    governor is not necessary any more, so drop it, which also makes it
    possible to drop the struct cs_governor definition and simplify the
    code accordingly.

    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Comment 1 Aleksei Rybalkin 2016-10-10 15:45:04 UTC
Created attachment 241331 [details]
My kernel config
Comment 2 Aleksei Rybalkin 2016-10-10 17:40:41 UTC
git revert d352cf47d93e39494b44b792cca8d35a3a0bd9b3
appears to fix the problem on 4.8.1. So it is definitely this commit.
Comment 3 Rafael J. Wysocki 2016-10-10 20:38:24 UTC
Sorry for the breakage.

Do I understand correctly that the governor doesn't select frequencies below 2000000 now, although it did that before?
Comment 4 Aleksei Rybalkin 2016-10-11 04:46:31 UTC
Yes, that is correct.
Comment 5 Rafael J. Wysocki 2016-10-11 21:22:27 UTC
Created attachment 241561 [details]
cpufreq: conservative: Fix next frequency selection

Does this patch make any difference?
Comment 6 Rafael J. Wysocki 2016-10-11 21:24:23 UTC
Created attachment 241571 [details]
cpufreq: conservative: Fix next frequency selection

Sorry, one piece was missing in the previous patch.

Please test this one instead.
Comment 7 Aleksei Rybalkin 2016-10-12 06:50:37 UTC
Yes, the patch fixes the issue. The frequencies are selected as expected.
Comment 8 Rafael J. Wysocki 2016-10-12 19:04:51 UTC
OK, thanks!

I'll submit it to the mailing lists, then.
Comment 9 Rafael J. Wysocki 2016-10-12 20:05:30 UTC
Patch submitted: https://patchwork.kernel.org/patch/9373743/
Comment 10 Rafael J. Wysocki 2016-10-14 23:43:54 UTC
Merged as commit abb6627910a1e783c8e034b35b7c80e5e7f98f41:


Note You need to log in before you can comment on or make changes to this bug.