linux-kernel - Re: [PATCH V3 1/6] cpufreq: suspend governors on system suspend/hibernate

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]

Date:	Tue, 26 Nov 2013 21:23:15 +0100
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Viresh Kumar <viresh.kumar@...aro.org>
Cc:	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	Patch Tracking <patches@...aro.org>,
	"cpufreq@...r.kernel.org" <cpufreq@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Nishanth Menon <nm@...com>,
	Stephen Warren <swarren@...dotorg.org>,
	Kgene Kim <kgene.kim@...sung.com>,
	linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
	linux-tegra@...r.kernel.org, jinchoi@...adcom.com,
	Lan Tianyu <tianyu.lan@...el.com>,
	Sebastian Capella <sebastian.capella@...aro.org>,
	Jonghwan Choi <jhbird.choi@...sung.com>
Subject: Re: [PATCH V3 1/6] cpufreq: suspend governors on system suspend/hibernate

On Tuesday, November 26, 2013 07:56:19 AM Viresh Kumar wrote:
> On 26 November 2013 04:59, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> >> @@ -1259,6 +1262,8 @@ int dpm_suspend(pm_message_t state)
> >>
> >>       might_sleep();
> >>
> >> +     cpufreq_suspend();
> >> +
> >>
> >>       mutex_lock(&dpm_list_mtx);
> >>       pm_transition = state;
> >>       async_error = 0;
> >
> > Shouldn't it do cpufreq_resume() on errors?
> 
> Yes and this is already done I believe. In case dpm_suspend() fails,
> dpm_resume() gets called. Isn't it?

OK

> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> >> +void cpufreq_suspend(void)
> >> +{
> >> +     struct cpufreq_policy *policy;
> >> +
> >> +     if (!has_target())
> >> +             return;
> >> +
> >> +     pr_debug("%s: Suspending Governors\n", __func__);
> >> +
> >> +     list_for_each_entry(policy, &cpufreq_policy_list, policy_list)
> >> +             if (__cpufreq_governor(policy, CPUFREQ_GOV_STOP))
> >> +                     pr_err("%s: Failed to stop governor for policy: %p\n",
> >> +                                     __func__, policy);
> >
> > This appears to be racy.  Is it really racy, or just seemingly?
> 
> Why does it look racy to you? Userspace should be frozen by now,
> policy_list should be stable as well as nobody would touch it.

You're stopping governors while they may be in use in principle.  Do we have
suitable synchronization in place for that?

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/