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  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]
Date:   Fri, 22 Mar 2019 10:55:59 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Rafael Wysocki <rjw@...ysocki.net>,
        Peter Zijlstra <peterz@...radead.org>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Linux PM <linux-pm@...r.kernel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        sparclinux@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH V3] cpufreq: Call transition notifier only once for each policy

On Fri, Mar 22, 2019 at 7:28 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> On 21-03-19, 16:49, Thomas Gleixner wrote:
> > On Wed, 20 Mar 2019, Viresh Kumar wrote:
> > >
> > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > > index 3fae23834069..b2fe665878f7 100644
> > > --- a/arch/x86/kernel/tsc.c
> > > +++ b/arch/x86/kernel/tsc.c
> > > @@ -958,10 +958,15 @@ static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
> > >     struct cpufreq_freqs *freq = data;
> > >     unsigned long *lpj;
> > >
> > > +   if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1)) {
> > > +           mark_tsc_unstable("cpufreq changes: related CPUs affected");
> > > +           return 0;
> > > +   }
> >
> > You might add a check which ensures that policy->cpu == smp_processor_id()
> > because if this is not the case ....
>
> How about something like this ?
>
>         if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1 ||
>                          freq->policy->cpu != smp_processor_id())) {
>                 mark_tsc_unstable("cpufreq changes: related CPUs affected");
>                 return 0;
>         }
>
>
> Thanks for your feedback.

Peter suggested something like this IIRC.

Anyway, I'm still concerned that this approach in general
fundamentally doesn't work on SMP with frequency synchronization,
which is the case for the platforms affected by the problem it
attempts to overcome.

The frequency has just been changed on one CPU, presumably to the
requested value (so this cannot work when turbo is enabled anyway),
but then it also has changed for all of the other CPUs in the system
(or at least in the package), so it is not sufficient to update the
single CPU here as it is only a messenger, so to speak.  However,
updating the other CPUs from here would be fundamentally racy AFAICS.

Powered by blists - more mailing lists