Re: [PATCH v2] cpufreq: suspend cpufreq governors on shutdown
On Monday, December 29, 2014 09:45:42 AM Viresh Kumar wrote:
> On 24 December 2014 at 11:39, Doug Anderson wrote:
> > We should stop cpufreq governors when we shut down the system. If we
> > don't do this, we can end up with this deadlock:
> >
> > 1. cpufreq governor may be running on a CPU other than CPU0.
> > 2. In machine_restart() we call smp_send_stop() which stops CPUs.
> >If one of these CPUs was actively running a cpufreq governor
> >then it may have the mutex / spinlock needed to access the main
> >PMIC in the system (perhaps over I2C)
> > 3. If a machine needs access to the main PMIC in order to shutdown
> >then it will never get it since the mutex was lost when the other
> >CPU stopped.
> > 4. We'll hang (possibly eventually hitting the hard lockup detector).
> >
> > Let's avoid the problem by stopping the cpufreq governor at shutdown,
> > which is a sensible thing to do anyway.
> >
> > Signed-off-by: Doug Anderson
> > ---
> > Changes in v2:
> > - Add a comment about why we register
> > - Don't create cpufreq_shutdown() wrapper function
> >
> > drivers/cpufreq/cpufreq.c | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index a09a29c..33f3d65 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -27,6 +27,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -2550,6 +2551,14 @@ int cpufreq_unregister_driver(struct cpufreq_driver
> > *driver)
> > }
> > EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
> >
> > +/*
> > + * Stop cpufreq at shutdown to make sure it isn't holding any locks
> > + * or mutexes when secondary CPUs are halted.
> > + */
> > +static struct syscore_ops cpufreq_syscore_ops = {
> > + .shutdown = cpufreq_suspend,
> > +};
> > +
> > static int __init cpufreq_core_init(void)
> > {
> > if (cpufreq_disabled())
> > @@ -2558,6 +2567,8 @@ static int __init cpufreq_core_init(void)
> > cpufreq_global_kobject = kobject_create();
> > BUG_ON(!cpufreq_global_kobject);
> >
> > + register_syscore_ops(_syscore_ops);
> > +
> > return 0;
> > }
> > core_initcall(cpufreq_core_init);
>
> Acked-by: Viresh Kumar
Queued up for 3.20, thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cpufreq: suspend cpufreq governors on shutdown
On Monday, December 29, 2014 09:45:42 AM Viresh Kumar wrote:
On 24 December 2014 at 11:39, Doug Anderson diand...@chromium.org wrote:
We should stop cpufreq governors when we shut down the system. If we
don't do this, we can end up with this deadlock:
1. cpufreq governor may be running on a CPU other than CPU0.
2. In machine_restart() we call smp_send_stop() which stops CPUs.
If one of these CPUs was actively running a cpufreq governor
then it may have the mutex / spinlock needed to access the main
PMIC in the system (perhaps over I2C)
3. If a machine needs access to the main PMIC in order to shutdown
then it will never get it since the mutex was lost when the other
CPU stopped.
4. We'll hang (possibly eventually hitting the hard lockup detector).
Let's avoid the problem by stopping the cpufreq governor at shutdown,
which is a sensible thing to do anyway.
Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Add a comment about why we register
- Don't create cpufreq_shutdown() wrapper function
drivers/cpufreq/cpufreq.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a09a29c..33f3d65 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -27,6 +27,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/suspend.h
+#include linux/syscore_ops.h
#include linux/tick.h
#include trace/events/power.h
@@ -2550,6 +2551,14 @@ int cpufreq_unregister_driver(struct cpufreq_driver
*driver)
}
EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
+/*
+ * Stop cpufreq at shutdown to make sure it isn't holding any locks
+ * or mutexes when secondary CPUs are halted.
+ */
+static struct syscore_ops cpufreq_syscore_ops = {
+ .shutdown = cpufreq_suspend,
+};
+
static int __init cpufreq_core_init(void)
{
if (cpufreq_disabled())
@@ -2558,6 +2567,8 @@ static int __init cpufreq_core_init(void)
cpufreq_global_kobject = kobject_create();
BUG_ON(!cpufreq_global_kobject);
+ register_syscore_ops(cpufreq_syscore_ops);
+
return 0;
}
core_initcall(cpufreq_core_init);
Acked-by: Viresh Kumar viresh.ku...@linaro.org
Queued up for 3.20, thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cpufreq: suspend cpufreq governors on shutdown
On 24 December 2014 at 11:39, Doug Anderson wrote:
> We should stop cpufreq governors when we shut down the system. If we
> don't do this, we can end up with this deadlock:
>
> 1. cpufreq governor may be running on a CPU other than CPU0.
> 2. In machine_restart() we call smp_send_stop() which stops CPUs.
>If one of these CPUs was actively running a cpufreq governor
>then it may have the mutex / spinlock needed to access the main
>PMIC in the system (perhaps over I2C)
> 3. If a machine needs access to the main PMIC in order to shutdown
>then it will never get it since the mutex was lost when the other
>CPU stopped.
> 4. We'll hang (possibly eventually hitting the hard lockup detector).
>
> Let's avoid the problem by stopping the cpufreq governor at shutdown,
> which is a sensible thing to do anyway.
>
> Signed-off-by: Doug Anderson
> ---
> Changes in v2:
> - Add a comment about why we register
> - Don't create cpufreq_shutdown() wrapper function
>
> drivers/cpufreq/cpufreq.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index a09a29c..33f3d65 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -2550,6 +2551,14 @@ int cpufreq_unregister_driver(struct cpufreq_driver
> *driver)
> }
> EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
>
> +/*
> + * Stop cpufreq at shutdown to make sure it isn't holding any locks
> + * or mutexes when secondary CPUs are halted.
> + */
> +static struct syscore_ops cpufreq_syscore_ops = {
> + .shutdown = cpufreq_suspend,
> +};
> +
> static int __init cpufreq_core_init(void)
> {
> if (cpufreq_disabled())
> @@ -2558,6 +2567,8 @@ static int __init cpufreq_core_init(void)
> cpufreq_global_kobject = kobject_create();
> BUG_ON(!cpufreq_global_kobject);
>
> + register_syscore_ops(_syscore_ops);
> +
> return 0;
> }
> core_initcall(cpufreq_core_init);
Acked-by: Viresh Kumar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] cpufreq: suspend cpufreq governors on shutdown
On 24 December 2014 at 11:39, Doug Anderson diand...@chromium.org wrote:
We should stop cpufreq governors when we shut down the system. If we
don't do this, we can end up with this deadlock:
1. cpufreq governor may be running on a CPU other than CPU0.
2. In machine_restart() we call smp_send_stop() which stops CPUs.
If one of these CPUs was actively running a cpufreq governor
then it may have the mutex / spinlock needed to access the main
PMIC in the system (perhaps over I2C)
3. If a machine needs access to the main PMIC in order to shutdown
then it will never get it since the mutex was lost when the other
CPU stopped.
4. We'll hang (possibly eventually hitting the hard lockup detector).
Let's avoid the problem by stopping the cpufreq governor at shutdown,
which is a sensible thing to do anyway.
Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Add a comment about why we register
- Don't create cpufreq_shutdown() wrapper function
drivers/cpufreq/cpufreq.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a09a29c..33f3d65 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -27,6 +27,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/suspend.h
+#include linux/syscore_ops.h
#include linux/tick.h
#include trace/events/power.h
@@ -2550,6 +2551,14 @@ int cpufreq_unregister_driver(struct cpufreq_driver
*driver)
}
EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
+/*
+ * Stop cpufreq at shutdown to make sure it isn't holding any locks
+ * or mutexes when secondary CPUs are halted.
+ */
+static struct syscore_ops cpufreq_syscore_ops = {
+ .shutdown = cpufreq_suspend,
+};
+
static int __init cpufreq_core_init(void)
{
if (cpufreq_disabled())
@@ -2558,6 +2567,8 @@ static int __init cpufreq_core_init(void)
cpufreq_global_kobject = kobject_create();
BUG_ON(!cpufreq_global_kobject);
+ register_syscore_ops(cpufreq_syscore_ops);
+
return 0;
}
core_initcall(cpufreq_core_init);
Acked-by: Viresh Kumar viresh.ku...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] cpufreq: suspend cpufreq governors on shutdown
We should stop cpufreq governors when we shut down the system. If we
don't do this, we can end up with this deadlock:
1. cpufreq governor may be running on a CPU other than CPU0.
2. In machine_restart() we call smp_send_stop() which stops CPUs.
If one of these CPUs was actively running a cpufreq governor
then it may have the mutex / spinlock needed to access the main
PMIC in the system (perhaps over I2C)
3. If a machine needs access to the main PMIC in order to shutdown
then it will never get it since the mutex was lost when the other
CPU stopped.
4. We'll hang (possibly eventually hitting the hard lockup detector).
Let's avoid the problem by stopping the cpufreq governor at shutdown,
which is a sensible thing to do anyway.
Signed-off-by: Doug Anderson
---
Changes in v2:
- Add a comment about why we register
- Don't create cpufreq_shutdown() wrapper function
drivers/cpufreq/cpufreq.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a09a29c..33f3d65 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -2550,6 +2551,14 @@ int cpufreq_unregister_driver(struct cpufreq_driver
*driver)
}
EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
+/*
+ * Stop cpufreq at shutdown to make sure it isn't holding any locks
+ * or mutexes when secondary CPUs are halted.
+ */
+static struct syscore_ops cpufreq_syscore_ops = {
+ .shutdown = cpufreq_suspend,
+};
+
static int __init cpufreq_core_init(void)
{
if (cpufreq_disabled())
@@ -2558,6 +2567,8 @@ static int __init cpufreq_core_init(void)
cpufreq_global_kobject = kobject_create();
BUG_ON(!cpufreq_global_kobject);
+ register_syscore_ops(_syscore_ops);
+
return 0;
}
core_initcall(cpufreq_core_init);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] cpufreq: suspend cpufreq governors on shutdown
We should stop cpufreq governors when we shut down the system. If we
don't do this, we can end up with this deadlock:
1. cpufreq governor may be running on a CPU other than CPU0.
2. In machine_restart() we call smp_send_stop() which stops CPUs.
If one of these CPUs was actively running a cpufreq governor
then it may have the mutex / spinlock needed to access the main
PMIC in the system (perhaps over I2C)
3. If a machine needs access to the main PMIC in order to shutdown
then it will never get it since the mutex was lost when the other
CPU stopped.
4. We'll hang (possibly eventually hitting the hard lockup detector).
Let's avoid the problem by stopping the cpufreq governor at shutdown,
which is a sensible thing to do anyway.
Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Add a comment about why we register
- Don't create cpufreq_shutdown() wrapper function
drivers/cpufreq/cpufreq.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a09a29c..33f3d65 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -27,6 +27,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/suspend.h
+#include linux/syscore_ops.h
#include linux/tick.h
#include trace/events/power.h
@@ -2550,6 +2551,14 @@ int cpufreq_unregister_driver(struct cpufreq_driver
*driver)
}
EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
+/*
+ * Stop cpufreq at shutdown to make sure it isn't holding any locks
+ * or mutexes when secondary CPUs are halted.
+ */
+static struct syscore_ops cpufreq_syscore_ops = {
+ .shutdown = cpufreq_suspend,
+};
+
static int __init cpufreq_core_init(void)
{
if (cpufreq_disabled())
@@ -2558,6 +2567,8 @@ static int __init cpufreq_core_init(void)
cpufreq_global_kobject = kobject_create();
BUG_ON(!cpufreq_global_kobject);
+ register_syscore_ops(cpufreq_syscore_ops);
+
return 0;
}
core_initcall(cpufreq_core_init);
--
2.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
