Hi,
maybe I found something..., can someone review/test this.
Thanks,
Thomas
------------
CPUFREQ ACPI: Only call _PPC after cpufreq ACPI init funcs got called already
Ingo Molnar provided a fix to not call _PPC at processor driver initialization
time.
Git commit #e4233dec749a3519069d9390561b5636a75c7579
But it can still happen that _PPC is called at processor driver
initialization time.
This patch should make sure that this is not possible anymore.
Signed-off-by: Thomas Renninger <[email protected]>
---
arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c | 6 ++++++
drivers/acpi/processor_perflib.c | 13 ++++++++++++-
drivers/cpufreq/cpufreq.c | 3 +++
include/linux/cpufreq.h | 1 +
4 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
b/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
index 69288f6..3233fe8 100644
--- a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
+++ b/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
@@ -96,6 +96,12 @@ static int pmi_notifier(struct notifier_block *nb,
struct cpufreq_frequency_table *cbe_freqs;
u8 node;
+ /* Should this really be called for CPUFREQ_ADJUST, CPUFREQ_INCOMPATIBLE
+ * and CPUFREQ_NOTIFY policy events?)
+ */
+ if (event == CPUFREQ_START)
+ return 0;
+
cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
node = cbe_cpu_to_node(policy->cpu);
diff --git a/drivers/acpi/processor_perflib.c
b/drivers/acpi/processor_perflib.c
index b474996..63ccf80 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -72,7 +72,13 @@ MODULE_PARM_DESC(ignore_ppc, "If the frequency of your
machine gets wrongly" \
#define PPC_REGISTERED 1
#define PPC_IN_USE 2
-static int acpi_processor_ppc_status = 0;
+/* ignore_ppc:
+ * -1 -> cpufreq low level drivers not initialized -> _PSS, etc. not called
yet
+ * ignore _PPC
+ * 0 -> cpufreq low level drivers initialized -> consider _PPC values
+ * 1 -> ignore _PPC totally -> forced by user through boot param
+ */
+static int acpi_processor_ppc_status = -1;
static int acpi_processor_ppc_notifier(struct notifier_block *nb,
unsigned long event, void *data)
@@ -81,6 +87,11 @@ static int acpi_processor_ppc_notifier(struct
notifier_block *nb,
struct acpi_processor *pr;
unsigned int ppc = 0;
+ if (event == CPUFREQ_START && ignore_ppc <= 0) {
+ ignore_ppc = 0;
+ return 0;
+ }
+
if (ignore_ppc)
return 0;
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1d41496..0471ef5 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -825,6 +825,9 @@ static int cpufreq_add_dev(struct sys_device *sys_dev)
policy->user_policy.min = policy->cpuinfo.min_freq;
policy->user_policy.max = policy->cpuinfo.max_freq;
+ blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
+ CPUFREQ_START, policy);
+
#ifdef CONFIG_SMP
#ifdef CONFIG_HOTPLUG_CPU
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index e7e91db..07cb761 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -109,6 +109,7 @@ struct cpufreq_policy {
#define CPUFREQ_ADJUST (0)
#define CPUFREQ_INCOMPATIBLE (1)
#define CPUFREQ_NOTIFY (2)
+#define CPUFREQ_START (3)
#define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
#define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */
--
1.5.4.5
On Thu, 17 Jul 2008 15:48:02 +0200
Thomas Renninger <[email protected]> wrote:
> Hi,
>
> maybe I found something..., can someone review/test this.
>
> Thanks,
>
> Thomas
>
> ------------
> CPUFREQ ACPI: Only call _PPC after cpufreq ACPI init funcs got called already
>
> Ingo Molnar provided a fix to not call _PPC at processor driver initialization
> time.
> Git commit #e4233dec749a3519069d9390561b5636a75c7579
>
> But it can still happen that _PPC is called at processor driver
> initialization time.
>
> This patch should make sure that this is not possible anymore.
There is no actual description of what this fixes, is there? Do
machines go oops, or what?
How do we proceed from here with this patch? Who should review it, who
should test it, who should ack it and who should merge it?
e4233dec749a3519069d9390561b5636a75c7579 was in January so this patch
is applicable to 2.6.25.x and to 2.6.26.x. But is it needed there?
Insufficient info.
Ho hum. I queued it, tagged as needed-in-2.6.25.x and 2.6.26.x. But I
am unsure about that.
Please help to clarify these things.
> Signed-off-by: Thomas Renninger <[email protected]>
> ---
> arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c | 6 ++++++
> drivers/acpi/processor_perflib.c | 13 ++++++++++++-
> drivers/cpufreq/cpufreq.c | 3 +++
> include/linux/cpufreq.h | 1 +
> 4 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> b/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> index 69288f6..3233fe8 100644
> --- a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> +++ b/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> @@ -96,6 +96,12 @@ static int pmi_notifier(struct notifier_block *nb,
> struct cpufreq_frequency_table *cbe_freqs;
> u8 node;
>
> + /* Should this really be called for CPUFREQ_ADJUST, CPUFREQ_INCOMPATIBLE
> + * and CPUFREQ_NOTIFY policy events?)
> + */
> + if (event == CPUFREQ_START)
> + return 0;
> +
> cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
> node = cbe_cpu_to_node(policy->cpu);
>
> diff --git a/drivers/acpi/processor_perflib.c
> b/drivers/acpi/processor_perflib.c
> index b474996..63ccf80 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -72,7 +72,13 @@ MODULE_PARM_DESC(ignore_ppc, "If the frequency of your
> machine gets wrongly" \
> #define PPC_REGISTERED 1
> #define PPC_IN_USE 2
>
> -static int acpi_processor_ppc_status = 0;
> +/* ignore_ppc:
> + * -1 -> cpufreq low level drivers not initialized -> _PSS, etc. not called
> yet
I am going to start hanging around in dark alleys in the hope of
meeting the person who invented wordwrapping.
Here it is again, fixed:
From: Thomas Renninger <[email protected]>
Ingo Molnar provided a fix to not call _PPC at processor driver
initialization time. Git commit #e4233dec749a3519069d9390561b5636a75c7579
But it can still happen that _PPC is called at processor driver
initialization time.
This patch should make sure that this is not possible anymore.
Signed-off-by: Thomas Renninger <[email protected]>
Cc: Dave Jones <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Venkatesh Pallipadi <[email protected]>
Cc: Chandra Seetharaman <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Arkadiusz Miskiewicz <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]> [2.6.25.x, 2.6.26.x]
Signed-off-by: Andrew Morton <[email protected]>
---
arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c | 6 ++++++
drivers/acpi/processor_perflib.c | 13 ++++++++++++-
drivers/cpufreq/cpufreq.c | 3 +++
include/linux/cpufreq.h | 1 +
4 files changed, 22 insertions(+), 1 deletion(-)
diff -puN arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
--- a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already
+++ a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
@@ -96,6 +96,12 @@ static int pmi_notifier(struct notifier_
struct cpufreq_frequency_table *cbe_freqs;
u8 node;
+ /* Should this really be called for CPUFREQ_ADJUST, CPUFREQ_INCOMPATIBLE
+ * and CPUFREQ_NOTIFY policy events?)
+ */
+ if (event == CPUFREQ_START)
+ return 0;
+
cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
node = cbe_cpu_to_node(policy->cpu);
diff -puN drivers/acpi/processor_perflib.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already drivers/acpi/processor_perflib.c
--- a/drivers/acpi/processor_perflib.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already
+++ a/drivers/acpi/processor_perflib.c
@@ -72,7 +72,13 @@ MODULE_PARM_DESC(ignore_ppc, "If the fre
#define PPC_REGISTERED 1
#define PPC_IN_USE 2
-static int acpi_processor_ppc_status = 0;
+/* ignore_ppc:
+ * -1 -> cpufreq low level drivers not initialized -> _PSS, etc. not called yet
+ * ignore _PPC
+ * 0 -> cpufreq low level drivers initialized -> consider _PPC values
+ * 1 -> ignore _PPC totally -> forced by user through boot param
+ */
+static int acpi_processor_ppc_status = -1;
static int acpi_processor_ppc_notifier(struct notifier_block *nb,
unsigned long event, void *data)
@@ -81,6 +87,11 @@ static int acpi_processor_ppc_notifier(s
struct acpi_processor *pr;
unsigned int ppc = 0;
+ if (event == CPUFREQ_START && ignore_ppc <= 0) {
+ ignore_ppc = 0;
+ return 0;
+ }
+
if (ignore_ppc)
return 0;
diff -puN drivers/cpufreq/cpufreq.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already drivers/cpufreq/cpufreq.c
--- a/drivers/cpufreq/cpufreq.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already
+++ a/drivers/cpufreq/cpufreq.c
@@ -825,6 +825,9 @@ static int cpufreq_add_dev(struct sys_de
policy->user_policy.min = policy->cpuinfo.min_freq;
policy->user_policy.max = policy->cpuinfo.max_freq;
+ blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
+ CPUFREQ_START, policy);
+
#ifdef CONFIG_SMP
#ifdef CONFIG_HOTPLUG_CPU
diff -puN include/linux/cpufreq.h~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already include/linux/cpufreq.h
--- a/include/linux/cpufreq.h~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init-funcs-got-called-already
+++ a/include/linux/cpufreq.h
@@ -106,6 +106,7 @@ struct cpufreq_policy {
#define CPUFREQ_ADJUST (0)
#define CPUFREQ_INCOMPATIBLE (1)
#define CPUFREQ_NOTIFY (2)
+#define CPUFREQ_START (3)
#define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
#define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */
_
On Thursday 17 July 2008 11:40:00 pm Andrew Morton wrote:
> On Thu, 17 Jul 2008 15:48:02 +0200
>
> Thomas Renninger <[email protected]> wrote:
> > Hi,
> >
> > maybe I found something..., can someone review/test this.
> >
> > Thanks,
> >
> > Thomas
> >
> > ------------
> > CPUFREQ ACPI: Only call _PPC after cpufreq ACPI init funcs got called
> > already
> >
> > Ingo Molnar provided a fix to not call _PPC at processor driver
> > initialization time.
> > Git commit #e4233dec749a3519069d9390561b5636a75c7579
> >
> > But it can still happen that _PPC is called at processor driver
> > initialization time.
> >
> > This patch should make sure that this is not possible anymore.
>
> There is no actual description of what this fixes, is there? Do
> machines go oops, or what?
ThinkPads get stuck at lowest frequency.
A very nasty bug, reported several times recently.
This seem to show up rarely, not 100% reproducable (after every X boot...).
I do not know whether this one really fixes it. I saw an interesting debug
output on the cpufreq list recently pointing in this direction.
Even if it's not this, only calling _PPC after other initial cpufreq ACPI
functions like _PDC or _PSS have been called (means cpufreq is really active)
is a good idea. _PPC functions often have a complex logic. They might depend
on variables initialized in e.g. in _PDC or _PSS...
>
> How do we proceed from here with this patch? Who should review it, who
> should test it, who should ack it and who should merge it?
I hope that Dave or someone on cpufreq list is having a look at it.
Dave should take it if it's fine.
People seeing this are in CC. After a quick review (patches are short..), I
can provide an rpm for SUSE people (there is a SUSE and a kernel bug open)
https://bugzilla.novell.com/show_bug.cgi?id=374099
>
> e4233dec749a3519069d9390561b5636a75c7579 was in January so this patch
> is applicable to 2.6.25.x and to 2.6.26.x. But is it needed there?
Probably should go in through .27 after review/testing.
> Insufficient info.
>
> Ho hum. I queued it, tagged as needed-in-2.6.25.x and 2.6.26.x. But I
> am unsure about that.
>
> Please help to clarify these things.
>
> > Signed-off-by: Thomas Renninger <[email protected]>
> > ---
> > arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c | 6 ++++++
> > drivers/acpi/processor_perflib.c | 13 ++++++++++++-
> > drivers/cpufreq/cpufreq.c | 3 +++
> > include/linux/cpufreq.h | 1 +
> > 4 files changed, 22 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> > b/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> > index 69288f6..3233fe8 100644
> > --- a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> > +++ b/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> > @@ -96,6 +96,12 @@ static int pmi_notifier(struct notifier_block *nb,
> > struct cpufreq_frequency_table *cbe_freqs;
> > u8 node;
> >
> > + /* Should this really be called for CPUFREQ_ADJUST,
> > CPUFREQ_INCOMPATIBLE + * and CPUFREQ_NOTIFY policy events?)
> > + */
> > + if (event == CPUFREQ_START)
> > + return 0;
> > +
> > cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
> > node = cbe_cpu_to_node(policy->cpu);
> >
> > diff --git a/drivers/acpi/processor_perflib.c
> > b/drivers/acpi/processor_perflib.c
> > index b474996..63ccf80 100644
> > --- a/drivers/acpi/processor_perflib.c
> > +++ b/drivers/acpi/processor_perflib.c
> > @@ -72,7 +72,13 @@ MODULE_PARM_DESC(ignore_ppc, "If the frequency of your
> > machine gets wrongly" \
> > #define PPC_REGISTERED 1
> > #define PPC_IN_USE 2
> >
> > -static int acpi_processor_ppc_status = 0;
> > +/* ignore_ppc:
> > + * -1 -> cpufreq low level drivers not initialized -> _PSS, etc. not
> > called yet
>
> I am going to start hanging around in dark alleys in the hope of
> meeting the person who invented wordwrapping.
Sorry about that, I moved to kmail recently, still do not know how this could
happen...
Thomas
>
>
> Here it is again, fixed:
>
>
> From: Thomas Renninger <[email protected]>
>
> Ingo Molnar provided a fix to not call _PPC at processor driver
> initialization time. Git commit #e4233dec749a3519069d9390561b5636a75c7579
>
> But it can still happen that _PPC is called at processor driver
> initialization time.
>
> This patch should make sure that this is not possible anymore.
>
> Signed-off-by: Thomas Renninger <[email protected]>
> Cc: Dave Jones <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Venkatesh Pallipadi <[email protected]>
> Cc: Chandra Seetharaman <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Arkadiusz Miskiewicz <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]> [2.6.25.x, 2.6.26.x]
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c | 6 ++++++
> drivers/acpi/processor_perflib.c | 13 ++++++++++++-
> drivers/cpufreq/cpufreq.c | 3 +++
> include/linux/cpufreq.h | 1 +
> 4 files changed, 22 insertions(+), 1 deletion(-)
>
> diff -puN
> arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c~cpufreq-acpi-only-call-_ppc-a
>fter-cpufreq-acpi-init-funcs-got-called-already
> arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c ---
> a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c~cpufreq-acpi-only-call-_ppc
>-after-cpufreq-acpi-init-funcs-got-called-already +++
> a/arch/powerpc/platforms/cell/cbe_cpufreq_pmi.c
> @@ -96,6 +96,12 @@ static int pmi_notifier(struct notifier_
> struct cpufreq_frequency_table *cbe_freqs;
> u8 node;
>
> + /* Should this really be called for CPUFREQ_ADJUST, CPUFREQ_INCOMPATIBLE
> + * and CPUFREQ_NOTIFY policy events?)
> + */
> + if (event == CPUFREQ_START)
> + return 0;
> +
> cbe_freqs = cpufreq_frequency_get_table(policy->cpu);
> node = cbe_cpu_to_node(policy->cpu);
>
> diff -puN
> drivers/acpi/processor_perflib.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-
>acpi-init-funcs-got-called-already drivers/acpi/processor_perflib.c ---
> a/drivers/acpi/processor_perflib.c~cpufreq-acpi-only-call-_ppc-after-cpufre
>q-acpi-init-funcs-got-called-already +++ a/drivers/acpi/processor_perflib.c
> @@ -72,7 +72,13 @@ MODULE_PARM_DESC(ignore_ppc, "If the fre
> #define PPC_REGISTERED 1
> #define PPC_IN_USE 2
>
> -static int acpi_processor_ppc_status = 0;
> +/* ignore_ppc:
> + * -1 -> cpufreq low level drivers not initialized -> _PSS, etc. not
> called yet + * ignore _PPC
> + * 0 -> cpufreq low level drivers initialized -> consider _PPC values
> + * 1 -> ignore _PPC totally -> forced by user through boot param
> + */
> +static int acpi_processor_ppc_status = -1;
>
> static int acpi_processor_ppc_notifier(struct notifier_block *nb,
> unsigned long event, void *data)
> @@ -81,6 +87,11 @@ static int acpi_processor_ppc_notifier(s
> struct acpi_processor *pr;
> unsigned int ppc = 0;
>
> + if (event == CPUFREQ_START && ignore_ppc <= 0) {
> + ignore_ppc = 0;
> + return 0;
> + }
> +
> if (ignore_ppc)
> return 0;
>
> diff -puN
> drivers/cpufreq/cpufreq.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-in
>it-funcs-got-called-already drivers/cpufreq/cpufreq.c ---
> a/drivers/cpufreq/cpufreq.c~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-
>init-funcs-got-called-already +++ a/drivers/cpufreq/cpufreq.c
> @@ -825,6 +825,9 @@ static int cpufreq_add_dev(struct sys_de
> policy->user_policy.min = policy->cpuinfo.min_freq;
> policy->user_policy.max = policy->cpuinfo.max_freq;
>
> + blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
> + CPUFREQ_START, policy);
> +
> #ifdef CONFIG_SMP
>
> #ifdef CONFIG_HOTPLUG_CPU
> diff -puN
> include/linux/cpufreq.h~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-init
>-funcs-got-called-already include/linux/cpufreq.h ---
> a/include/linux/cpufreq.h~cpufreq-acpi-only-call-_ppc-after-cpufreq-acpi-in
>it-funcs-got-called-already +++ a/include/linux/cpufreq.h
> @@ -106,6 +106,7 @@ struct cpufreq_policy {
> #define CPUFREQ_ADJUST (0)
> #define CPUFREQ_INCOMPATIBLE (1)
> #define CPUFREQ_NOTIFY (2)
> +#define CPUFREQ_START (3)
>
> #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
> #define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */
> _