linux-kernel - Re: [PATCH v2] sched/topology: remove sysctl_sched_energy

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: <5401cd08-c7fc-86ac-23df-6e6a59ef84cf@linux.vnet.ibm.com>
Date:   Fri, 1 Sep 2023 18:38:18 +0530
From:   Shrikanth Hegde <sshegde@...ux.vnet.ibm.com>
To:     Chen Yu <yu.c.chen@...el.com>
Cc:     mingo@...hat.com, peterz@...radead.org, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, vschneid@...hat.com,
        linux-kernel@...r.kernel.org, ionela.voinescu@....com,
        quentin.perret@....com, srikar@...ux.vnet.ibm.com,
        mgorman@...hsingularity.net, mingo@...nel.org
Subject: Re: [PATCH v2] sched/topology: remove sysctl_sched_energy_aware
 depending on the architecture



On 9/1/23 3:19 PM, Chen Yu wrote:
> Hi Shrikanth,
>

Hi Chen, Thanks for the review.
 
> On 2023-09-01 at 12:22:49 +0530, Shrikanth Hegde wrote:
>> Currently sysctl_sched_energy_aware doesn't alter the said behaviour on
>> some of the architectures. IIUC its meant to either force rebuild the
>> perf domains or cleanup the perf domains by echoing 1 or 0 respectively.
>>
>> perf domains are not built when there is SMT, or when there is no
>> Asymmetric CPU topologies or when there is no frequency invariance.
>> Since such cases EAS is not set and perf domains are not built. By
>> changing the values of sysctl_sched_energy_aware, its not possible to
>> force build the perf domains. Hence remove this sysctl on such platforms
>> that dont support it. Some of the settings can be changed later
>> such as smt_active by offlining the CPU's, In those cases if
>> build_perf_domains returns true, re-enable the sysctl.
>>
>> Anytime, when sysctl_sched_energy_aware is changed sched_energy_update
>> is set when building the perf domains. Making use of that to find out if
>> the change is happening by sysctl or dynamic system change.
>>
>> Taking different cases:
>> Case1. system while booting has EAS capability, sysctl will be set 1. Hence
>> perf domains will be built if needed. On changing the sysctl to 0, since
>> sched_energy_update is true, perf domains would be freed and sysctl will
>> not be removed. later sysctl is changed to 1, enabling the perf domains
>> rebuild again. Since sysctl is already there, it will skip register.
>>
>> Case2. System while booting doesn't have EAS Capability. Later after system
>> change it becomes capable of EAS. sched_energy_update is false. Though
>> sysctl is 0, will go ahead and try to enable eas. This is the current
>> behaviour. if has_eas  is true, then sysctl will be registered. After
>> that any sysctl change is same as Case1.
>>
> 
> I think this change makes sense. Just one question for case 2,
> sched_energy_update is not strictly tied with sysctl change, right?
> sched_energy_update is true in rebuild_sched_domains_energy().
> rebuild_sched_domains_energy() will not only be invoked by sysctl
> path via sched_energy_aware_handler(), but also by other path, such
> as update_scale_freq_invariant(). If the system boots with EAS capability
> disabled, then it becomes EAS capable due to the frequency invariant
> readiness(cpufreq policy change?), then
> cpufreq_notifier(CPUFREQ_CREATE_POLICY) -> init_amu_fie_callback() ->
> amu_fie_setup() -> opology_set_scale_freq_source() -> 
> update_scale_freq_invariant(true) -> rebuild_sched_domains_energy()
> Since sched_energy_update is true, the rebuild of perf domain will be skipped(but
> actually we want to create it) Please correct me if I miss something.
>

Ah, More cases!

You are right. let me see what can be done.  
maybe specific variable be used for sysctl change? it already quite a few variables there. 
Will think if this can be simplified somehow.