Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH] cpufreq: Make cpufreq_quick_get() safe to call. | Date | Thu, 10 Mar 2016 23:20:47 +0100 |
| |
On Thursday, March 10, 2016 04:10:36 PM Richard Cochran wrote: > The function, cpufreq_quick_get, accesses the global 'cpufreq_driver' and > its fields without taking the associated lock, cpufreq_driver_lock. > > Without the locking, nothing guarantees that 'cpufreq_driver' remains > consistent during the call. This patch fixes the issue by taking the lock > before accessing the data structure. > > Cc: Dirk Brandewie <dirk.brandewie@gmail.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Richard Cochran <rcochran@linutronix.de>
Can you please CC PM-related patches to linux-pm@vger.kernel.org? They are much easier to handle for me then.
> --- > drivers/cpufreq/cpufreq.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index e979ec7..ce02b2b 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1457,9 +1457,17 @@ unsigned int cpufreq_quick_get(unsigned int cpu) > { > struct cpufreq_policy *policy; > unsigned int ret_freq = 0; > + unsigned long flags; > + > + read_lock_irqsave(&cpufreq_driver_lock, flags); > > if (cpufreq_driver && cpufreq_driver->setpolicy && cpufreq_driver->get) > - return cpufreq_driver->get(cpu); > + ret_freq = cpufreq_driver->get(cpu); > + > + read_unlock_irqrestore(&cpufreq_driver_lock, flags); > + > + if (ret_freq) > + return ret_freq; > > policy = cpufreq_cpu_get(cpu); > if (policy) { >
I would prefer something like this:
read_lock_irqsave(&cpufreq_driver_lock, flags);
if (cpufreq_driver && cpufreq_driver->setpolicy && cpufreq_driver->get) { unsigned int ret_freq = cpufreq_driver->get(cpu);
read_unlock_irqrestore(&cpufreq_driver_lock, flags); return ret_freq; }
read_unlock_irqrestore(&cpufreq_driver_lock, flags);
Thanks, Rafael
| |