From: Viresh Kumar <viresh.kumar@linaro.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
sboyd@codeaurora.org, prarit@redhat.com, skannan@codeaurora.org,
Srivatsa Bhat <srivatsa@mit.edu>
Subject: Re: [PATCH V7 1/6] cpufreq: Don't allow updating inactive policies from sysfs
Date: Tue, 9 Jun 2015 08:16:48 +0530 [thread overview]
Message-ID: <20150609024648.GD4829@linux> (raw)
In-Reply-To: <1543467.8Qo1djPj9c@vostro.rjw.lan>
On 09-06-15, 01:19, Rafael J. Wysocki wrote:
> I'm not sure if -EPERM is the best error code for this case. It doesn't depend
> on the actual permissions, but rather on the policy status that may change
> going forward.
>
> It might be better to use -EBUSY even.
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 27 Jan 2015 13:22:23 +0530
Subject: [PATCH] cpufreq: Don't allow updating inactive policies from sysfs
Later commits would change the way policies are managed today. Policies
wouldn't be freed on cpu hotplug (currently they aren't freed only for
suspend), and while the CPU is offline, the sysfs cpufreq files would
still be present.
User may accidentally try to update the sysfs files in following
directory: '/sys/devices/system/cpu/cpuX/cpufreq/'. And that would
result in undefined behavior as policy wouldn't be active then.
Apart from updating the store() routine, we also update __cpufreq_get()
which can call cpufreq_out_of_sync(). The later routine tries to update
policy->cur and starts notifying kernel about it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 5d780ff5a10a..16214dfe78c5 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -875,11 +875,18 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
down_write(&policy->rwsem);
+ /* Updating inactive policies is invalid, so avoid doing that. */
+ if (unlikely(policy_is_inactive(policy))) {
+ ret = -EBUSY;
+ goto unlock_policy_rwsem;
+ }
+
if (fattr->store)
ret = fattr->store(policy, buf, count);
else
ret = -EIO;
+unlock_policy_rwsem:
up_write(&policy->rwsem);
up_read(&cpufreq_rwsem);
@@ -1610,6 +1617,10 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
ret_freq = cpufreq_driver->get(policy->cpu);
+ /* Updating inactive policies is invalid, so avoid doing that. */
+ if (unlikely(policy_is_inactive(policy)))
+ return ret_freq;
+
if (ret_freq && policy->cur &&
!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
/* verify no discrepancy between actual and
--
viresh
next prev parent reply other threads:[~2015-06-09 2:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-08 12:55 [PATCH V7 0/6] cpufreq: Don't loose cpufreq history on CPU hotplug Viresh Kumar
2015-06-08 12:55 ` [PATCH V7 1/6] cpufreq: Don't allow updating inactive policies from sysfs Viresh Kumar
2015-06-08 23:19 ` Rafael J. Wysocki
2015-06-09 2:46 ` Viresh Kumar [this message]
2015-06-09 21:50 ` Saravana Kannan
2015-06-08 12:55 ` [PATCH V7 2/6] cpufreq: Stop migrating sysfs files on hotplug Viresh Kumar
2015-06-08 23:23 ` Rafael J. Wysocki
2015-06-09 2:50 ` Viresh Kumar
2015-06-10 0:20 ` Saravana Kannan
2015-06-10 2:19 ` Viresh Kumar
2015-06-10 23:29 ` Rafael J. Wysocki
2015-06-08 12:55 ` [PATCH V7 3/6] cpufreq: Initialize policy->kobj while allocating policy Viresh Kumar
2015-06-08 12:55 ` [PATCH V7 4/6] cpufreq: Call cpufreq_policy_put_kobj() from cpufreq_policy_free() Viresh Kumar
2015-06-08 12:55 ` [PATCH V7 5/6] cpufreq: Restart governor as soon as possible Viresh Kumar
2015-06-08 23:27 ` Rafael J. Wysocki
2015-06-09 3:09 ` Viresh Kumar
2015-06-08 12:55 ` [PATCH V7 6/6] cpufreq: Remove cpufreq_update_policy() Viresh Kumar
2015-06-08 23:17 ` [PATCH V7 0/6] cpufreq: Don't loose cpufreq history on CPU hotplug Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150609024648.GD4829@linux \
--to=viresh.kumar@linaro.org \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-pm@vger.kernel.org \
--cc=prarit@redhat.com \
--cc=rjw@rjwysocki.net \
--cc=sboyd@codeaurora.org \
--cc=skannan@codeaurora.org \
--cc=srivatsa@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.