| Message ID | 234906745c34fb72b0c450d897f261a22ed18d40.1363947023.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Fri, Mar 22, 2013 at 3:43 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() > with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as > zero. Rafael, Since this prevents booting on our hardware (we unregister and re-register the cpufreq driver to account for virtual cores), will this be considered as a hotfix for 3.9? > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Acked-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > drivers/cpufreq/cpufreq_stats.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c > index 2fd779e..bfd6273 100644 > --- a/drivers/cpufreq/cpufreq_stats.c > +++ b/drivers/cpufreq/cpufreq_stats.c > @@ -180,15 +180,19 @@ static void cpufreq_stats_free_sysfs(unsigned int cpu) > { > struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); > > - if (!cpufreq_frequency_get_table(cpu)) > + if (!policy) > return; > > - if (policy && !policy_is_shared(policy)) { > + if (!cpufreq_frequency_get_table(cpu)) > + goto put_ref; > + > + if (!policy_is_shared(policy)) { > pr_debug("%s: Free sysfs stat\n", __func__); > sysfs_remove_group(&policy->kobj, &stats_attr_group); > } > - if (policy) > - cpufreq_cpu_put(policy); > + > +put_ref: > + cpufreq_cpu_put(policy); > } > > static int cpufreq_stats_create_table(struct cpufreq_policy *policy, > -- > 1.7.12.rc2.18.g61b472e > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 March 2013 16:42, Amit Kucheria <amit.kucheria@linaro.org> wrote: > On Fri, Mar 22, 2013 at 3:43 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() >> with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as >> zero. > > Rafael, > > Since this prevents booting on our hardware (we unregister and > re-register the cpufreq driver to account for virtual cores), will > this be considered as a hotfix for 3.9? > >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Acked-by: Amit Kucheria <amit.kucheria@linaro.org> Sorry i forgot to mention, this should be pushed for next rc. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, March 22, 2013 04:47:29 PM Viresh Kumar wrote: > On 22 March 2013 16:42, Amit Kucheria <amit.kucheria@linaro.org> wrote: > > On Fri, Mar 22, 2013 at 3:43 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > >> In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() > >> with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as > >> zero. > > > > Rafael, > > > > Since this prevents booting on our hardware (we unregister and > > re-register the cpufreq driver to account for virtual cores), will > > this be considered as a hotfix for 3.9? > > > >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > Acked-by: Amit Kucheria <amit.kucheria@linaro.org> > > Sorry i forgot to mention, this should be pushed for next rc. Well, -rc5 is a realistic target. Thanks, Rafael
On 22 March 2013 17:29, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Sure. Can you please remind me what hardware is that, though?
Not yet upstreamed: big LITTLE :)
But, this is broken for all other platforms too.. You just need to compile
your driver as module and insmod/remove it to call
cpufreq_driver_unregister().
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 22, 2013 at 5:25 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 22 March 2013 17:29, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> Sure. Can you please remind me what hardware is that, though? > > Not yet upstreamed: big LITTLE :) Yes, the b.L cpufreq driver is being reviewed currently. And board support is being merged in. > But, this is broken for all other platforms too.. You just need to compile > your driver as module and insmod/remove it to call > cpufreq_driver_unregister(). -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, March 22, 2013 04:42:57 PM Amit Kucheria wrote: > On Fri, Mar 22, 2013 at 3:43 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() > > with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as > > zero. > > Rafael, > > Since this prevents booting on our hardware (we unregister and > re-register the cpufreq driver to account for virtual cores), will > this be considered as a hotfix for 3.9? Sure. Can you please remind me what hardware is that, though? Rafael > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Acked-by: Amit Kucheria <amit.kucheria@linaro.org> > > > --- > > drivers/cpufreq/cpufreq_stats.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c > > index 2fd779e..bfd6273 100644 > > --- a/drivers/cpufreq/cpufreq_stats.c > > +++ b/drivers/cpufreq/cpufreq_stats.c > > @@ -180,15 +180,19 @@ static void cpufreq_stats_free_sysfs(unsigned int cpu) > > { > > struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); > > > > - if (!cpufreq_frequency_get_table(cpu)) > > + if (!policy) > > return; > > > > - if (policy && !policy_is_shared(policy)) { > > + if (!cpufreq_frequency_get_table(cpu)) > > + goto put_ref; > > + > > + if (!policy_is_shared(policy)) { > > pr_debug("%s: Free sysfs stat\n", __func__); > > sysfs_remove_group(&policy->kobj, &stats_attr_group); > > } > > - if (policy) > > - cpufreq_cpu_put(policy); > > + > > +put_ref: > > + cpufreq_cpu_put(policy); > > } > > > > static int cpufreq_stats_create_table(struct cpufreq_policy *policy, > > -- > > 1.7.12.rc2.18.g61b472e > > > -- > To unsubscribe from this list: send the line "unsubscribe cpufreq" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 March 2013 17:42, Rafael J. Wysocki <rjw@sisk.pl> wrote: > Which would be useful to write in the changelog, wouldn't it? Hmm.. copy-paste with gmail is also broken, so find it attached too. New change log, no change in patch and you can trust me on that :) ----------x-------------x-------- From 034e5ac4cccd09872592a46decd38d5f24047f10 Mon Sep 17 00:00:00 2001 Message-Id: <034e5ac4cccd09872592a46decd38d5f24047f10.1363954124.git.viresh.kumar@linaro.org> From: Viresh Kumar <viresh.kumar@linaro.org> Date: Fri, 22 Mar 2013 15:15:48 +0530 Subject: [PATCH] cpufreq: stats: do cpufreq_cpu_put() corresponding to cpufreq_cpu_get In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as zero. We will get a hang if somehow cpufreq_driver_unregister() is called. And that can happen when we compile our driver as module and insmod/rmmod it. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
On Friday, March 22, 2013 05:25:13 PM Viresh Kumar wrote: > On 22 March 2013 17:29, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > Sure. Can you please remind me what hardware is that, though? > > Not yet upstreamed: big LITTLE :) > > But, this is broken for all other platforms too.. You just need to compile > your driver as module and insmod/remove it to call > cpufreq_driver_unregister(). Which would be useful to write in the changelog, wouldn't it? Rafael
On Friday, March 22, 2013 05:40:25 PM Viresh Kumar wrote: > On 22 March 2013 17:42, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > Which would be useful to write in the changelog, wouldn't it? > > Hmm.. > > copy-paste with gmail is also broken, so find it attached too. > > New change log, no change in patch and you can trust me on that :) OK, applied to bleeding-edge. Thanks, Rafael > ----------x-------------x-------- > > From 034e5ac4cccd09872592a46decd38d5f24047f10 Mon Sep 17 00:00:00 2001 > Message-Id: <034e5ac4cccd09872592a46decd38d5f24047f10.1363954124.git.viresh.kumar@linaro.org> > From: Viresh Kumar <viresh.kumar@linaro.org> > Date: Fri, 22 Mar 2013 15:15:48 +0530 > Subject: [PATCH] cpufreq: stats: do cpufreq_cpu_put() corresponding to > cpufreq_cpu_get > > In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() > with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as > zero. > > We will get a hang if somehow cpufreq_driver_unregister() is called. And that > can happen when we compile our driver as module and insmod/rmmod it. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c index 2fd779e..bfd6273 100644 --- a/drivers/cpufreq/cpufreq_stats.c +++ b/drivers/cpufreq/cpufreq_stats.c @@ -180,15 +180,19 @@ static void cpufreq_stats_free_sysfs(unsigned int cpu) { struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - if (!cpufreq_frequency_get_table(cpu)) + if (!policy) return; - if (policy && !policy_is_shared(policy)) { + if (!cpufreq_frequency_get_table(cpu)) + goto put_ref; + + if (!policy_is_shared(policy)) { pr_debug("%s: Free sysfs stat\n", __func__); sysfs_remove_group(&policy->kobj, &stats_attr_group); } - if (policy) - cpufreq_cpu_put(policy); + +put_ref: + cpufreq_cpu_put(policy); } static int cpufreq_stats_create_table(struct cpufreq_policy *policy,
In cpufreq_stats_free_sysfs() we aren't balancing calls to cpufreq_cpu_get() with cpufreq_cpu_put(). This will never let us have ref count to policy->kobj as zero. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq_stats.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)