linux-kernel - Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline

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: <CAJZ5v0iWxdP9qYZow2yJ2CF8GD-HTO5W1w6e8baJunXEQpMjQA@mail.gmail.com>
Date:	Thu, 11 Feb 2016 02:15:19 +0100
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	"Rafael J. Wysocki" <rafael@...nel.org>
Cc:	Viresh Kumar <viresh.kumar@...aro.org>,
	Rafael Wysocki <rjw@...ysocki.net>,
	Juri Lelli <juri.lelli@....com>,
	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Shilpasri G Bhat <shilpa.bhat@...ux.vnet.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish routines

On Thu, Feb 11, 2016 at 1:59 AM, Rafael J. Wysocki <rafael@...nel.org> wrote:
> On Tue, Feb 9, 2016 at 4:46 AM, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>> The offline routine was separated into two halves earlier by
>> 'commit 1aee40ac9c86 ("cpufreq: Invoke __cpufreq_remove_dev_finish()
>> after releasing cpu_hotplug.lock");.
>>
>> And the reasons cited were, race issues between accessing policy's sysfs
>> files and policy kobject's cleanup.
>>
>> That race isn't valid anymore, as we don't remove the policy & its
>> kobject completely on hotplugs, but do that from ->remove() callback of
>> subsys framework.
>
> Governor sysfs attributes are still removed in
> __cpufreq_governor(_EXIT), though, so had store() been used for them,
> the deadlock described in the changelog of commit 1aee40ac9c86 would
> have been possible.
>
> Fortunately, we don't use store() (which still does get_online_cpus())
> for those attributes now.  We use governor_store() for them and that
> doesn't call get_online_cpus().  So in fact this patch is only correct
> after the recent rework of the governor attributes handling.
>
> Please modify the changelog to explain that more thoroughly.

And one question tangentially related to this patch: Would it be
possible to avoid calling __cpufreq_governor(_EXIT) for CPU offline?

The fact that we still carry out the whole governor teardown at that
point is slightly disturbing, as in theory it should be possible to
keep the governor attributes in place across offline/online.

Thanks,
Rafael