[PATCH 0/2] v3.9-rc1 fixes for intel_pstate driver

[PATCH 0/2] v3.9-rc1 fixes for intel_pstate driver

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

These patches fix the issues reported against the intel_pstate driver.

The first bug reported by Josh Boyer was that the dirver would blow up
when loading in a VM exposed the second bug that the error path in
intel_pstate_init() was incorrect and biting Konrad Wilk as well.

https://bugzilla.redhat.com/show_bug.cgi?id=916833
https://lkml.org/lkml/2013/3/5/394

Dirk Brandewie (2):
  cpufreq/intel_pstate: Do not load on VM that do not report max P
    state.
  cpufreq/intel_pstate: Fix intel_pstate_init() error path

 drivers/cpufreq/intel_pstate.c |   42 +++++++++++++--------------------------
 1 files changed, 14 insertions(+), 28 deletions(-)

-- 
1.7.7.6

[PATCH 1/2] cpufreq/intel_pstate: Do not load on VM that do not report max P state.

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

It seems some VMs support the P state MSRs but return zeros. Fail
gracefully if we are running in this environment.

https://bugzilla.redhat.com/show_bug.cgi?id=916833

Reported-by: jwboyer@redhat.com
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 096fde0..2bfd083 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -662,6 +662,9 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 
 	cpu = all_cpu_data[policy->cpu];
 
+	if (!policy->cpuinfo.max_freq)
+		return -ENODEV;
+
 	intel_pstate_get_min_max(cpu, &min, &max);
 
 	limits.min_perf_pct = (policy->min * 100) / policy->cpuinfo.max_freq;
-- 
1.7.7.6

[PATCH 2/2] cpufreq/intel_pstate: Fix intel_pstate_init() error path

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

If cpufreq_register_driver() fails just free memory that has been
allocated and return. intel_pstate_exit() function is removed sine we
are built-in only now there is no reason for a module exit proceedure.

Reported-by:Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c |   39 +++++++++++----------------------------
 1 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 2bfd083..f6dd1e7 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -750,37 +750,11 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.owner		= THIS_MODULE,
 };
 
-static void intel_pstate_exit(void)
-{
-	int cpu;
-
-	sysfs_remove_group(intel_pstate_kobject,
-				&intel_pstate_attr_group);
-	debugfs_remove_recursive(debugfs_parent);
-
-	cpufreq_unregister_driver(&intel_pstate_driver);
-
-	if (!all_cpu_data)
-		return;
-
-	get_online_cpus();
-	for_each_online_cpu(cpu) {
-		if (all_cpu_data[cpu]) {
-			del_timer_sync(&all_cpu_data[cpu]->timer);
-			kfree(all_cpu_data[cpu]);
-		}
-	}
-
-	put_online_cpus();
-	vfree(all_cpu_data);
-}
-module_exit(intel_pstate_exit);
-
 static int __initdata no_load;
 
 static int __init intel_pstate_init(void)
 {
-	int rc = 0;
+	int cpu, rc = 0;
 	const struct x86_cpu_id *id;
 
 	if (no_load)
@@ -805,7 +779,16 @@ static int __init intel_pstate_init(void)
 	intel_pstate_sysfs_expose_params();
 	return rc;
 out:
-	intel_pstate_exit();
+	get_online_cpus();
+	for_each_online_cpu(cpu) {
+		if (all_cpu_data[cpu]) {
+			del_timer_sync(&all_cpu_data[cpu]->timer);
+			kfree(all_cpu_data[cpu]);
+		}
+	}
+
+	put_online_cpus();
+	vfree(all_cpu_data);
 	return -ENODEV;
 }
 device_initcall(intel_pstate_init);
-- 
1.7.7.6

Re: [PATCH 2/2] cpufreq/intel_pstate: Fix intel_pstate_init() error path

[Viresh Kumar]

On Wed, Mar 6, 2013 at 6:15 AM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> If cpufreq_register_driver() fails just free memory that has been
> allocated and return. intel_pstate_exit() function is removed sine we

s/sine/since

> are built-in only now there is no reason for a module exit proceedure.

s/proceedure/procedure

> Reported-by:Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c |   39 +++++++++++----------------------------
>  1 files changed, 11 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 2bfd083..f6dd1e7 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -750,37 +750,11 @@ static struct cpufreq_driver intel_pstate_driver = {
>         .owner          = THIS_MODULE,
>  };
>
> -static void intel_pstate_exit(void)
> -{
> -       int cpu;
> -
> -       sysfs_remove_group(intel_pstate_kobject,
> -                               &intel_pstate_attr_group);
> -       debugfs_remove_recursive(debugfs_parent);
> -
> -       cpufreq_unregister_driver(&intel_pstate_driver);
> -
> -       if (!all_cpu_data)
> -               return;
> -
> -       get_online_cpus();
> -       for_each_online_cpu(cpu) {
> -               if (all_cpu_data[cpu]) {
> -                       del_timer_sync(&all_cpu_data[cpu]->timer);
> -                       kfree(all_cpu_data[cpu]);
> -               }
> -       }
> -
> -       put_online_cpus();
> -       vfree(all_cpu_data);
> -}
> -module_exit(intel_pstate_exit);
> -
>  static int __initdata no_load;
>
>  static int __init intel_pstate_init(void)
>  {
> -       int rc = 0;
> +       int cpu, rc = 0;
>         const struct x86_cpu_id *id;
>
>         if (no_load)
> @@ -805,7 +779,16 @@ static int __init intel_pstate_init(void)
>         intel_pstate_sysfs_expose_params();
>         return rc;
>  out:
> -       intel_pstate_exit();
> +       get_online_cpus();
> +       for_each_online_cpu(cpu) {
> +               if (all_cpu_data[cpu]) {
> +                       del_timer_sync(&all_cpu_data[cpu]->timer);
> +                       kfree(all_cpu_data[cpu]);
> +               }
> +       }
> +
> +       put_online_cpus();
> +       vfree(all_cpu_data);
>         return -ENODEV;
>  }
>  device_initcall(intel_pstate_init);
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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 0/2] v3.9-rc1 fixes for intel_pstate driver

[Viresh Kumar]

On Wed, Mar 6, 2013 at 6:15 AM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> These patches fix the issues reported against the intel_pstate driver.
>
> The first bug reported by Josh Boyer was that the dirver would blow up

s/dirver/driver :)

> when loading in a VM exposed the second bug that the error path in
> intel_pstate_init() was incorrect and biting Konrad Wilk as well.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=916833
> https://lkml.org/lkml/2013/3/5/394

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

[PATCH 2/2] cpufreq/intel_pstate: Fix intel_pstate_init() error path

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

If cpufreq_register_driver() fails just free memory that has been
allocated and return. intel_pstate_exit() function is removed since we
are built-in only now there is no reason for a module exit procedure.

Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c |   39 +++++++++++----------------------------
 1 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 2bfd083..f6dd1e7 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -750,37 +750,11 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.owner		= THIS_MODULE,
 };
 
-static void intel_pstate_exit(void)
-{
-	int cpu;
-
-	sysfs_remove_group(intel_pstate_kobject,
-				&intel_pstate_attr_group);
-	debugfs_remove_recursive(debugfs_parent);
-
-	cpufreq_unregister_driver(&intel_pstate_driver);
-
-	if (!all_cpu_data)
-		return;
-
-	get_online_cpus();
-	for_each_online_cpu(cpu) {
-		if (all_cpu_data[cpu]) {
-			del_timer_sync(&all_cpu_data[cpu]->timer);
-			kfree(all_cpu_data[cpu]);
-		}
-	}
-
-	put_online_cpus();
-	vfree(all_cpu_data);
-}
-module_exit(intel_pstate_exit);
-
 static int __initdata no_load;
 
 static int __init intel_pstate_init(void)
 {
-	int rc = 0;
+	int cpu, rc = 0;
 	const struct x86_cpu_id *id;
 
 	if (no_load)
@@ -805,7 +779,16 @@ static int __init intel_pstate_init(void)
 	intel_pstate_sysfs_expose_params();
 	return rc;
 out:
-	intel_pstate_exit();
+	get_online_cpus();
+	for_each_online_cpu(cpu) {
+		if (all_cpu_data[cpu]) {
+			del_timer_sync(&all_cpu_data[cpu]->timer);
+			kfree(all_cpu_data[cpu]);
+		}
+	}
+
+	put_online_cpus();
+	vfree(all_cpu_data);
 	return -ENODEV;
 }
 device_initcall(intel_pstate_init);
-- 
1.7.7.6

Re: [PATCH 2/2] cpufreq/intel_pstate: Fix intel_pstate_init() error path

[Viresh Kumar]

On Thu, Mar 7, 2013 at 12:36 AM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> If cpufreq_register_driver() fails just free memory that has been
> allocated and return. intel_pstate_exit() function is removed since we
> are built-in only now there is no reason for a module exit procedure.
>
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c |   39 +++++++++++----------------------------
>  1 files changed, 11 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 2bfd083..f6dd1e7 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -750,37 +750,11 @@ static struct cpufreq_driver intel_pstate_driver = {
>         .owner          = THIS_MODULE,
>  };
>
> -static void intel_pstate_exit(void)
> -{
> -       int cpu;
> -
> -       sysfs_remove_group(intel_pstate_kobject,
> -                               &intel_pstate_attr_group);
> -       debugfs_remove_recursive(debugfs_parent);
> -
> -       cpufreq_unregister_driver(&intel_pstate_driver);
> -
> -       if (!all_cpu_data)
> -               return;
> -
> -       get_online_cpus();
> -       for_each_online_cpu(cpu) {
> -               if (all_cpu_data[cpu]) {
> -                       del_timer_sync(&all_cpu_data[cpu]->timer);
> -                       kfree(all_cpu_data[cpu]);
> -               }
> -       }
> -
> -       put_online_cpus();
> -       vfree(all_cpu_data);
> -}
> -module_exit(intel_pstate_exit);
> -
>  static int __initdata no_load;
>
>  static int __init intel_pstate_init(void)
>  {
> -       int rc = 0;
> +       int cpu, rc = 0;
>         const struct x86_cpu_id *id;
>
>         if (no_load)
> @@ -805,7 +779,16 @@ static int __init intel_pstate_init(void)
>         intel_pstate_sysfs_expose_params();
>         return rc;
>  out:
> -       intel_pstate_exit();
> +       get_online_cpus();
> +       for_each_online_cpu(cpu) {
> +               if (all_cpu_data[cpu]) {
> +                       del_timer_sync(&all_cpu_data[cpu]->timer);
> +                       kfree(all_cpu_data[cpu]);
> +               }
> +       }
> +
> +       put_online_cpus();
> +       vfree(all_cpu_data);
>         return -ENODEV;
>  }
>  device_initcall(intel_pstate_init);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>