| Message ID | 20140323150754.272874059@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sunday, March 23, 2014 03:09:32 PM Thomas Gleixner wrote: > We are about to free the data structure. Make sure no timer callback > is running. I might be paranoid, but the ->exit callback can be > invoked from so many places, that it is not entirely clear whether > del_timer is always called on the cpu on which it is enqueued. > > While looking through the call sites I noticed, that > cpufreq_init_policy() can fail and invoke cpufreq_driver->exit() but > it does not return the failure and the callsite happily proceeds. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: cpufreq <cpufreq@vger.kernel.org> > Cc: pm <linux-pm@vger.kernel.org> Dirk? > --- > > drivers/cpufreq/intel_pstate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: tip/drivers/cpufreq/intel_pstate.c > =================================================================== > --- tip.orig/drivers/cpufreq/intel_pstate.c > +++ tip/drivers/cpufreq/intel_pstate.c > @@ -777,7 +777,7 @@ static int intel_pstate_cpu_exit(struct > { > int cpu = policy->cpu; > > - del_timer(&all_cpu_data[cpu]->timer); > + del_timer_sync(&all_cpu_data[cpu]->timer); > kfree(all_cpu_data[cpu]); > all_cpu_data[cpu] = NULL; > return 0; > >
On Sun, Mar 23, 2014 at 8:39 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > We are about to free the data structure. Make sure no timer callback > is running. I might be paranoid, but the ->exit callback can be > invoked from so many places, that it is not entirely clear whether > del_timer is always called on the cpu on which it is enqueued. In normal cases it will be called only when CPU goes offline and I hope that isn't called on the dying CPU. > While looking through the call sites I noticed, that > cpufreq_init_policy() can fail and invoke cpufreq_driver->exit() but > it does not return the failure and the callsite happily proceeds. Thanks for reporting this, will get it fixed. > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: cpufreq <cpufreq@vger.kernel.org> > Cc: pm <linux-pm@vger.kernel.org> > --- > > drivers/cpufreq/intel_pstate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: tip/drivers/cpufreq/intel_pstate.c > =================================================================== > --- tip.orig/drivers/cpufreq/intel_pstate.c > +++ tip/drivers/cpufreq/intel_pstate.c > @@ -777,7 +777,7 @@ static int intel_pstate_cpu_exit(struct > { > int cpu = policy->cpu; > > - del_timer(&all_cpu_data[cpu]->timer); > + del_timer_sync(&all_cpu_data[cpu]->timer); Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- 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
Hi Thomas, On 03/23/2014 06:56 PM, Rafael J. Wysocki wrote: > On Sunday, March 23, 2014 03:09:32 PM Thomas Gleixner wrote: >> We are about to free the data structure. Make sure no timer callback >> is running. I might be paranoid, but the ->exit callback can be >> invoked from so many places, that it is not entirely clear whether >> del_timer is always called on the cpu on which it is enqueued. >> >> While looking through the call sites I noticed, that >> cpufreq_init_policy() can fail and invoke cpufreq_driver->exit() but >> it does not return the failure and the callsite happily proceeds. >> The call to del_timer() has been moved to a new callback in material in Rafaels pull request for v3.15. I will send a patch adding this change to the v3.15 material. --Dirk >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >> Cc: cpufreq <cpufreq@vger.kernel.org> >> Cc: pm <linux-pm@vger.kernel.org> > > Dirk? > >> --- >> >> drivers/cpufreq/intel_pstate.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> Index: tip/drivers/cpufreq/intel_pstate.c >> =================================================================== >> --- tip.orig/drivers/cpufreq/intel_pstate.c >> +++ tip/drivers/cpufreq/intel_pstate.c >> @@ -777,7 +777,7 @@ static int intel_pstate_cpu_exit(struct >> { >> int cpu = policy->cpu; >> >> - del_timer(&all_cpu_data[cpu]->timer); >> + del_timer_sync(&all_cpu_data[cpu]->timer); >> kfree(all_cpu_data[cpu]); >> all_cpu_data[cpu] = NULL; >> return 0; >> >> > -- 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
Index: tip/drivers/cpufreq/intel_pstate.c =================================================================== --- tip.orig/drivers/cpufreq/intel_pstate.c +++ tip/drivers/cpufreq/intel_pstate.c @@ -777,7 +777,7 @@ static int intel_pstate_cpu_exit(struct { int cpu = policy->cpu; - del_timer(&all_cpu_data[cpu]->timer); + del_timer_sync(&all_cpu_data[cpu]->timer); kfree(all_cpu_data[cpu]); all_cpu_data[cpu] = NULL; return 0;
We are about to free the data structure. Make sure no timer callback is running. I might be paranoid, but the ->exit callback can be invoked from so many places, that it is not entirely clear whether del_timer is always called on the cpu on which it is enqueued. While looking through the call sites I noticed, that cpufreq_init_policy() can fail and invoke cpufreq_driver->exit() but it does not return the failure and the callsite happily proceeds. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: cpufreq <cpufreq@vger.kernel.org> Cc: pm <linux-pm@vger.kernel.org> --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 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