linux-kernel - Re: [PATCH] cpufreq: intel_pstate: Implement passive mode with HWP enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1884886.tmXHBG24oC@kreacher>
Date:   Tue, 28 Jul 2020 17:41:42 +0200
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Francisco Jerez <currojerez@...eup.net>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Documentation <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Giovanni Gherdovich <ggherdovich@...e.cz>,
        Doug Smythies <dsmythies@...us.net>
Subject: Re: [PATCH] cpufreq: intel_pstate: Implement passive mode with HWP enabled

On Tuesday, July 21, 2020 1:20:14 AM CEST Francisco Jerez wrote:
> 

[cut]

> > If there is a bug, then what exactly is it, from the users' perspective?
> >
> 
> It can be reproduced easily as follows:
> 
> | echo 1 > /sys/devices/system/cpu/intel_pstate/hwp_dynamic_boost
> | for p in /sys/devices/system/cpu/cpufreq/policy*/energy_performance_preference; do echo performance > $p; done
> 
> Let's make sure that the EPP updates landed on the turbostat output:
> 
> |[..]
> | Core    CPU     Avg_MHz Busy%   Bzy_MHz            HWP_REQ
> | -       -       1       0.05    2396    0x0000000000000000
> | 0       0       1       0.05    2153    0x0000000000002704
> | 0       4       1       0.04    2062    0x0000000000002704
> | 1       1       1       0.02    2938    0x0000000000002704
> | 1       5       2       0.09    2609    0x0000000000002704
> | 2       2       1       0.04    1857    0x0000000000002704
> | 2       6       1       0.05    2561    0x0000000000002704
> | 3       3       0       0.01    1883    0x0000000000002704
> | 3       7       2       0.07    2703    0x0000000000002704
> |[..]
> 
> Now let's do some non-trivial IO activity in order to trigger HWP
> dynamic boost, and watch while random CPUs start losing their EPP
> setting requested via sysfs:
> 
> |[..]
> | Core    CPU     Avg_MHz Busy%   Bzy_MHz            HWP_REQ
> | -       -       16      0.81    2023    0x0000000000000000
> | 0       0       7       0.66    1069    0x0000000080002704
>                                                     ^^
> | 0       4       24      2.19    1116    0x0000000080002704
>                                                     ^^
> | 1       1       18      0.68    2618    0x0000000000002704
> | 1       5       1       0.03    2005    0x0000000000002704
> | 2       2       2       0.07    2512    0x0000000000002704
> | 2       6       33      1.35    2402    0x0000000000002704
> | 3       3       1       0.04    2470    0x0000000000002704
> | 3       7       45      1.42    3185    0x0000000080002704
>                                                     ^^

Actually, that's because intel_pstate_hwp_boost_up() and
intel_pstate_hwp_boost_down() use the hwp_req_cached value
for updating the HWP Request MSR and that is only written to
by intel_pstate_hwp_set() which is only invoked on policy changes,
so the MSR writes from intel_pstate_set_energy_pref_index()
basically get discarded.

So this is a matter of synchronizing intel_pstate_set_policy() with
intel_pstate_set_energy_pref_index() and they both acquire
intel_pstate_limits_lock already, so this shouldn't be too difficult to fix.

Let me cut a patch for that.