linux-kernel - Re: [PATCH v2] cpufreq: schedutil: Examine the correct CPU when we update util

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: <037169f3-61f9-b29e-9561-0f7dba213cd5@arm.com>
Date:   Thu, 2 Nov 2017 12:06:36 +0000
From:   Chris Redpath <chris.redpath@....com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        Morten Rasmussen <morten.rasmussen@....com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v2] cpufreq: schedutil: Examine the correct CPU when we
 update util

Hi Viresh

On 02/11/17 11:40, Viresh Kumar wrote:
> On 02-11-17, 11:38, Chris Redpath wrote:
>> Since:
>> 4296f23ed cpufreq: schedutil: Fix per-CPU structure initialization in
>>   sugov_start()
>
> This is still incorrect. This BUG has nothing to do with 4296f23ed
> AFAICT.
>

According to my diff, this was the commit which switched from assigning
the values directly (and not overwriting the cpu member, which was
introduced in the other commit you reference) to using a memset and
clearing the whole struct. I figured that the fix (which was for a
different issue) inadvertently exposed this, which was what I was trying
to convey here. I can remove this and just reference 674e75411fc2. It's
clear that what's broken is the remote update.

I'll reply with a v3 shortly.

--Chris


>> We lost the value of sg_cpu->cpu which is assigned during
>> sugov_register. The memset in sugov_start overwrites it with zero.
>>
>> The change here was triggered by the commit adding the remote update
>> functionality.
>> 674e75411fc2 ("sched: cpufreq: Allow remote cpufreq callbacks")
>>
>> This leads to always looking at the utilization of CPU0 instead of
>> the one we just updated when we do a utilization update callback.
>>
>> Let's fix this by consolidating the initialization code into
>> sugov_start().
>>
>> Fixes: 674e75411fc2 ("sched: cpufreq: Allow remote cpufreq callbacks")
>> Signed-off-by: Chris Redpath <chris.redpath@....com>
>> Reviewed-by: Patrick Bellasi <patrick.bellasi@....com>
>> Reviewed-by: Brendan Jackman <brendan.jackman@....com>
>> Cc: Rafael J. Wysocki <rjw@...ysocki.net>
>> Cc: Viresh Kumar <viresh.kumar@...aro.org>
>> Cc: Ingo Molnar <mingo@...hat.com>
>> Cc: Peter Zijlstra <peterz@...radead.org>
>> ---
>>   kernel/sched/cpufreq_schedutil.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
>> index 6c1a7fcfa2a7..dc68a1ccdb33 100644
>> --- a/kernel/sched/cpufreq_schedutil.c
>> +++ b/kernel/sched/cpufreq_schedutil.c
>> @@ -728,6 +728,7 @@ static int sugov_start(struct cpufreq_policy *policy)
>>              struct sugov_cpu *sg_cpu = &per_cpu(sugov_cpu, cpu);
>>
>>              memset(sg_cpu, 0, sizeof(*sg_cpu));
>> +            sg_cpu->cpu = cpu;
>>              sg_cpu->sg_policy = sg_policy;
>>              sg_cpu->flags = SCHED_CPUFREQ_RT;
>>              sg_cpu->iowait_boost_max = policy->cpuinfo.max_freq;
>> @@ -793,11 +794,6 @@ struct cpufreq_governor *cpufreq_default_governor(void)
>>
>>   static int __init sugov_register(void)
>>   {
>> -    int cpu;
>> -
>> -    for_each_possible_cpu(cpu)
>> -            per_cpu(sugov_cpu, cpu).cpu = cpu;
>> -
>>      return cpufreq_register_governor(&schedutil_gov);
>>   }
>>   fs_initcall(sugov_register);
>> --
>> 2.13.1.449.g02a2850
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.