Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1331617 > unrolled thread

Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish routines

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish routines "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 02:00 +0100
    Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish routines "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-11 02:20 +0100
      Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish  routines Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-11 12:50 +0100

#1331617 — Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish routines

On Tue, Feb 9, 2016 at 4:46 AM, Viresh Kumar <viresh.kumar@linaro.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.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1331630

On Thu, Feb 11, 2016 at 1:59 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, Feb 9, 2016 at 4:46 AM, Viresh Kumar <viresh.kumar@linaro.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

[toc] | [prev] | [next] | [standalone]

#1331846 — Re: [PATCH V4 1/7] cpufreq: Merge cpufreq_offline_prepare/finish routines

On 11-02-16, 02:15, Rafael J. Wysocki wrote:
> 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.

Will think about that after the current code is stable a bit. It should be
possible, but need to see if it is worth it.

-- 
viresh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web