linux-kernel - Re: [PATCH v2] cpufreq: stats: Walk online CPUs with CPU offline/online locked

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 25 May 2016 02:48:51 +0200
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	Viresh Kumar <viresh.kumar@...aro.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Linux PM list <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: Re: [PATCH v2] cpufreq: stats: Walk online CPUs with CPU
 offline/online locked

On Wed, May 25, 2016 at 2:00 AM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> On Tuesday, May 24, 2016 05:47:17 PM Viresh Kumar wrote:
>> On 24-05-16, 14:13, Rafael J. Wysocki wrote:
>> > I don't really get it why you don't like get/put_online_cpus() so much.
>>
>> Not that I don't like them, I just wanted to see if its possible to
>> work without any additional locking.
>>
>> Anyway, so the first version of your patch did the get_online_cpus()
>> around a bigger section of the code instead of just the
>> for_each_online_cpu() loop. Should we change that?
>
> It did that, because locking around the loop alone wouldn't close the
> races I'm concerned about.
>
> That said, those same races are possible when the cpufreq driver is
> loaded along with the cpufreq_stats module or is unloaded along with
> that module.  Again, if ether a CPU goes online or the cpufreq driver
> is loaded during cpufreq_stats_init(), the new CPU *or* a new policy
> may be missed by it.  Similarly, if either a CPU goes offline or
> the cpufreq driver is unloaded during cpufreq_stats_exit(), that
> CPU or a policy that has just gone away may be missed by it.
>
> The CPU "hotplug" locking is not sufficient to close the races related
> to the cpufreq driver load/unload, so an alternative approach is needed.
> IMO, however, changing the way the notifiers work is not the way to go
> here, because the problem is specific to the stats module.  Unless there
> are other users of the notifiers with the same problem, it should be
> addressed in the stats code.
>
> The stats code looks all pants to me now, TBH, and the way it uses notifiers
> (both policy notifiers and transition notifiers) is questionable at the very
> least.  It looks like it would just be better to redesign it from scratch.

I'm actually considering making the stats code non-modular.

It won't have a reason to use notifiers then and may be simplified
quite a bit this way.  As an added benefit, if it doesn't use the
transition notifier, it won't interfere with fast frequency switching
in the schedutil governor any more.

Are there any drawbacks?