All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12
@ 2013-08-06 17:23 Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 01/11] cpufreq: Cleanup header files included in core Viresh Kumar
                   ` (11 more replies)
  0 siblings, 12 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

Hi Rafael,

This is V2 of the patch series which was posted here earlier:

http://www.gossamer-threads.com/lists/linux/kernel/1759514

This patchset tries to fix & cleanup many existing cpufreq core issues. First
four patches tries to cleanup basic problems in cpufreq core. Its first patch
was earlier sent separately but now is part of this series.

Fifth patch was also sent earlier as reply to your patches and was reviewed by
Srivatsa. Sixth patch was picked from Lukasz's patchset on introducing software
"boost" feature in core. It will be used by this patchset.

And 7-10 are are the most significant part of this set. They try to make many
things simple and robust.

Last patch in the series is new and wasn't part of V1.

This is rebased of your bleeding-edge branch + two patches from you:
18a6b03 cpufreq: Avoid double kobject_put() for the same kobject in error code path
d0cde63 cpufreq: Do not hold driver module references for additional policy CPUs
abe513f Merge branch 'acpi-sleep-next' into linux-next

They are also pushed in my cpufreq-next branch.

They are tested fairly well on ARM Vexpress TC2 board (big LITTLE).

Lukasz Majewski (1):
  cpufreq: Store cpufreq policies in a list

Viresh Kumar (10):
  cpufreq: Cleanup header files included in core
  cpufreq: Re-arrange declarations in cpufreq.h
  cpufreq: Give consistent names for struct cpufreq_policy *
  cpufreq: Use sizeof(*ptr) form for finding size of a struct
  cpufreq: Pass policy to cpufreq_add_policy_cpu()
  cpufreq: Use cpufreq_policy_list for iterating over policies
  cpufreq: Fix broken usage of governor->owner's refcount
  cpufreq: Don't use cpufreq_driver->owner's refcount to protect
    critical sections
  cpufreq: Remove struct cpufreq_driver's owner field
  cpufreq: improve error checking on return values of
    __cpufreq_governor()

 Documentation/cpu-freq/cpu-drivers.txt |   2 -
 drivers/cpufreq/acpi-cpufreq.c         |   5 +-
 drivers/cpufreq/at32ap-cpufreq.c       |   1 -
 drivers/cpufreq/blackfin-cpufreq.c     |   1 -
 drivers/cpufreq/cpufreq-nforce2.c      |   1 -
 drivers/cpufreq/cpufreq.c              | 426 ++++++++++++++++-----------------
 drivers/cpufreq/cpufreq_conservative.c |  14 +-
 drivers/cpufreq/cpufreq_governor.c     |   6 -
 drivers/cpufreq/cpufreq_governor.h     |   7 +-
 drivers/cpufreq/cpufreq_ondemand.c     |  24 +-
 drivers/cpufreq/cpufreq_performance.c  |   3 +-
 drivers/cpufreq/cpufreq_powersave.c    |   3 +-
 drivers/cpufreq/cpufreq_stats.c        |  23 +-
 drivers/cpufreq/cris-artpec3-cpufreq.c |   1 -
 drivers/cpufreq/cris-etraxfs-cpufreq.c |   1 -
 drivers/cpufreq/e_powersaver.c         |   5 +-
 drivers/cpufreq/elanfreq.c             |   1 -
 drivers/cpufreq/exynos-cpufreq.c       |   2 +-
 drivers/cpufreq/freq_table.c           |   4 +-
 drivers/cpufreq/gx-suspmod.c           |   3 +-
 drivers/cpufreq/ia64-acpi-cpufreq.c    |   5 +-
 drivers/cpufreq/intel_pstate.c         |   1 -
 drivers/cpufreq/kirkwood-cpufreq.c     |   1 -
 drivers/cpufreq/longhaul.c             |   1 -
 drivers/cpufreq/longrun.c              |   1 -
 drivers/cpufreq/loongson2_cpufreq.c    |   1 -
 drivers/cpufreq/maple-cpufreq.c        |   1 -
 drivers/cpufreq/p4-clockmod.c          |   1 -
 drivers/cpufreq/pasemi-cpufreq.c       |   1 -
 drivers/cpufreq/pcc-cpufreq.c          |   1 -
 drivers/cpufreq/pmac32-cpufreq.c       |   1 -
 drivers/cpufreq/pmac64-cpufreq.c       |   6 +-
 drivers/cpufreq/powernow-k6.c          |   1 -
 drivers/cpufreq/powernow-k7.c          |  14 +-
 drivers/cpufreq/powernow-k8.c          |   7 +-
 drivers/cpufreq/ppc-corenet-cpufreq.c  |   1 -
 drivers/cpufreq/ppc_cbe_cpufreq.c      |   1 -
 drivers/cpufreq/s3c2416-cpufreq.c      |   1 -
 drivers/cpufreq/s3c24xx-cpufreq.c      |   6 +-
 drivers/cpufreq/s3c64xx-cpufreq.c      |   1 -
 drivers/cpufreq/sc520_freq.c           |   1 -
 drivers/cpufreq/sh-cpufreq.c           |   1 -
 drivers/cpufreq/sparc-us2e-cpufreq.c   |   6 +-
 drivers/cpufreq/sparc-us3-cpufreq.c    |   6 +-
 drivers/cpufreq/speedstep-centrino.c   |   1 -
 drivers/cpufreq/speedstep-ich.c        |   1 -
 drivers/cpufreq/speedstep-smi.c        |   1 -
 include/linux/cpufreq.h                | 386 ++++++++++++++---------------
 48 files changed, 442 insertions(+), 547 deletions(-)

-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH V2 01/11] cpufreq: Cleanup header files included in core
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 02/11] cpufreq: Re-arrange declarations in cpufreq.h Viresh Kumar
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

This patch intends to cleanup following issues in the header files included in
cpufreq core layers:
- Include headers in ascending order, so that we don't add same multiple times
  by mistake.
- <asm/> must be included after <linux/>, so that they override whatever they
  need.
- Remove unnecessary header files
- Don't include files already included by cpufreq.h or cpufreq_governor.h

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c              | 19 ++++++-------------
 drivers/cpufreq/cpufreq_conservative.c | 12 ------------
 drivers/cpufreq/cpufreq_governor.c     |  6 ------
 drivers/cpufreq/cpufreq_governor.h     |  5 ++---
 drivers/cpufreq/cpufreq_ondemand.c     | 12 +-----------
 drivers/cpufreq/cpufreq_performance.c  |  3 +--
 drivers/cpufreq/cpufreq_powersave.c    |  3 +--
 drivers/cpufreq/cpufreq_stats.c        |  9 +--------
 drivers/cpufreq/freq_table.c           |  4 +---
 include/linux/cpufreq.h                | 11 +++--------
 10 files changed, 16 insertions(+), 68 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a7c5aa7..aaccfa0 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -17,24 +17,17 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <asm/cputime.h>
-#include <linux/kernel.h>
-#include <linux/kernel_stat.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/notifier.h>
+#include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/delay.h>
-#include <linux/interrupt.h>
-#include <linux/spinlock.h>
-#include <linux/tick.h>
 #include <linux/device.h>
-#include <linux/slab.h>
-#include <linux/cpu.h>
-#include <linux/completion.h>
+#include <linux/init.h>
+#include <linux/kernel_stat.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/slab.h>
 #include <linux/syscore_ops.h>
-
+#include <linux/tick.h>
 #include <trace/events/power.h>
 
 /**
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index f97cb3d..d2bd9ef 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -11,19 +11,7 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/cpufreq.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/kernel_stat.h>
-#include <linux/kobject.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/notifier.h>
-#include <linux/percpu-defs.h>
 #include <linux/slab.h>
-#include <linux/sysfs.h>
-#include <linux/types.h>
-
 #include "cpufreq_governor.h"
 
 /* Conservative governor macros */
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index cd4b5f7..8742736 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -16,15 +16,9 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <asm/cputime.h>
-#include <linux/cpufreq.h>
-#include <linux/cpumask.h>
 #include <linux/export.h>
 #include <linux/kernel_stat.h>
-#include <linux/mutex.h>
 #include <linux/slab.h>
-#include <linux/types.h>
-#include <linux/workqueue.h>
 
 #include "cpufreq_governor.h"
 
diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index 0fe69f2..7db4373 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -18,10 +18,9 @@
 #define _CPUFREQ_GOVERNOR_H
 
 #include <linux/cpufreq.h>
-#include <linux/kobject.h>
+#include <linux/kernel_stat.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/workqueue.h>
-#include <linux/sysfs.h>
 
 /*
  * The polling frequency depends on the capability of the processor. Default
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 25438bb..232de60 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -12,20 +12,10 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/cpufreq.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/kernel_stat.h>
-#include <linux/kobject.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
+#include <linux/cpu.h>
 #include <linux/percpu-defs.h>
 #include <linux/slab.h>
-#include <linux/sysfs.h>
 #include <linux/tick.h>
-#include <linux/types.h>
-#include <linux/cpu.h>
-
 #include "cpufreq_governor.h"
 
 /* On-demand governor macros */
diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c
index 9fef7d6..cf117de 100644
--- a/drivers/cpufreq/cpufreq_performance.c
+++ b/drivers/cpufreq/cpufreq_performance.c
@@ -12,10 +12,9 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/module.h>
 
 static int cpufreq_governor_performance(struct cpufreq_policy *policy,
 					unsigned int event)
diff --git a/drivers/cpufreq/cpufreq_powersave.c b/drivers/cpufreq/cpufreq_powersave.c
index 32109a1..e3b874c 100644
--- a/drivers/cpufreq/cpufreq_powersave.c
+++ b/drivers/cpufreq/cpufreq_powersave.c
@@ -12,10 +12,9 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/module.h>
 
 static int cpufreq_governor_powersave(struct cpufreq_policy *policy,
 					unsigned int event)
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index cb38413..a7143b0 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -9,17 +9,10 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/kernel.h>
-#include <linux/slab.h>
 #include <linux/cpu.h>
-#include <linux/sysfs.h>
 #include <linux/cpufreq.h>
 #include <linux/module.h>
-#include <linux/jiffies.h>
-#include <linux/percpu.h>
-#include <linux/kobject.h>
-#include <linux/spinlock.h>
-#include <linux/notifier.h>
+#include <linux/slab.h>
 #include <asm/cputime.h>
 
 static spinlock_t cpufreq_stats_lock;
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index f0d8741..f111454a 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -11,10 +11,8 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
 #include <linux/cpufreq.h>
+#include <linux/module.h>
 
 /*********************************************************************
  *                     FREQUENCY TABLE HELPERS                       *
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index e1fd215..97627bb 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -11,16 +11,11 @@
 #ifndef _LINUX_CPUFREQ_H
 #define _LINUX_CPUFREQ_H
 
-#include <asm/cputime.h>
-#include <linux/mutex.h>
-#include <linux/notifier.h>
-#include <linux/threads.h>
+#include <linux/cpumask.h>
+#include <linux/completion.h>
 #include <linux/kobject.h>
+#include <linux/notifier.h>
 #include <linux/sysfs.h>
-#include <linux/completion.h>
-#include <linux/workqueue.h>
-#include <linux/cpumask.h>
-#include <asm/div64.h>
 
 #define CPUFREQ_NAME_LEN 16
 /* Print length for names. Extra 1 space for accomodating '\n' in prints */
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 02/11] cpufreq: Re-arrange declarations in cpufreq.h
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 01/11] cpufreq: Cleanup header files included in core Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 03/11] cpufreq: Give consistent names for struct cpufreq_policy * Viresh Kumar
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

They are pretty much mixed up. Although generic headers are present but
definitions/declarations are present outside them too..

This patch just moves stuff up and down to make it look better and consistent.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/cpufreq.h | 373 +++++++++++++++++++++++-------------------------
 1 file changed, 177 insertions(+), 196 deletions(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 97627bb..0d3b026 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -17,60 +17,30 @@
 #include <linux/notifier.h>
 #include <linux/sysfs.h>
 
-#define CPUFREQ_NAME_LEN 16
-/* Print length for names. Extra 1 space for accomodating '\n' in prints */
-#define CPUFREQ_NAME_PLEN (CPUFREQ_NAME_LEN + 1)
-
 /*********************************************************************
- *                     CPUFREQ NOTIFIER INTERFACE                    *
+ *                        CPUFREQ INTERFACE                          *
  *********************************************************************/
-
-#define CPUFREQ_TRANSITION_NOTIFIER	(0)
-#define CPUFREQ_POLICY_NOTIFIER		(1)
-
-#ifdef CONFIG_CPU_FREQ
-int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);
-int cpufreq_unregister_notifier(struct notifier_block *nb, unsigned int list);
-extern void disable_cpufreq(void);
-#else		/* CONFIG_CPU_FREQ */
-static inline int cpufreq_register_notifier(struct notifier_block *nb,
-						unsigned int list)
-{
-	return 0;
-}
-static inline int cpufreq_unregister_notifier(struct notifier_block *nb,
-						unsigned int list)
-{
-	return 0;
-}
-static inline void disable_cpufreq(void) { }
-#endif		/* CONFIG_CPU_FREQ */
-
-/* if (cpufreq_driver->target) exists, the ->governor decides what frequency
- * within the limits is used. If (cpufreq_driver->setpolicy> exists, these
- * two generic policies are available:
- */
-
-#define CPUFREQ_POLICY_POWERSAVE	(1)
-#define CPUFREQ_POLICY_PERFORMANCE	(2)
-
-/* Frequency values here are CPU kHz so that hardware which doesn't run
- * with some frequencies can complain without having to guess what per
- * cent / per mille means.
+/*
+ * Frequency values here are CPU kHz
+ *
  * Maximum transition latency is in nanoseconds - if it's unknown,
  * CPUFREQ_ETERNAL shall be used.
  */
 
+#define CPUFREQ_ETERNAL			(-1)
+#define CPUFREQ_NAME_LEN		16
+/* Print length for names. Extra 1 space for accomodating '\n' in prints */
+#define CPUFREQ_NAME_PLEN		(CPUFREQ_NAME_LEN + 1)
+
 struct cpufreq_governor;
 
-/* /sys/devices/system/cpu/cpufreq: entry point for global variables */
-extern struct kobject *cpufreq_global_kobject;
-int cpufreq_get_global_kobject(void);
-void cpufreq_put_global_kobject(void);
-int cpufreq_sysfs_create_file(const struct attribute *attr);
-void cpufreq_sysfs_remove_file(const struct attribute *attr);
+struct cpufreq_freqs {
+	unsigned int cpu;	/* cpu nr */
+	unsigned int old;
+	unsigned int new;
+	u8 flags;		/* flags of cpufreq_driver, see below. */
+};
 
-#define CPUFREQ_ETERNAL			(-1)
 struct cpufreq_cpuinfo {
 	unsigned int		max_freq;
 	unsigned int		min_freq;
@@ -117,111 +87,95 @@ struct cpufreq_policy {
 	int			transition_ongoing; /* Tracks transition status */
 };
 
-#define CPUFREQ_ADJUST			(0)
-#define CPUFREQ_INCOMPATIBLE		(1)
-#define CPUFREQ_NOTIFY			(2)
-#define CPUFREQ_START			(3)
-#define CPUFREQ_UPDATE_POLICY_CPU	(4)
-
 /* Only for ACPI */
 #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
 #define CPUFREQ_SHARED_TYPE_HW	 (1) /* HW does needed coordination */
 #define CPUFREQ_SHARED_TYPE_ALL	 (2) /* All dependent CPUs should set freq */
 #define CPUFREQ_SHARED_TYPE_ANY	 (3) /* Freq can be set from any dependent CPU*/
 
+struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
+void cpufreq_cpu_put(struct cpufreq_policy *data);
+
 static inline bool policy_is_shared(struct cpufreq_policy *policy)
 {
 	return cpumask_weight(policy->cpus) > 1;
 }
 
-/******************** cpufreq transition notifiers *******************/
-
-#define CPUFREQ_PRECHANGE	(0)
-#define CPUFREQ_POSTCHANGE	(1)
-#define CPUFREQ_RESUMECHANGE	(8)
-#define CPUFREQ_SUSPENDCHANGE	(9)
+/* /sys/devices/system/cpu/cpufreq: entry point for global variables */
+extern struct kobject *cpufreq_global_kobject;
+int cpufreq_get_global_kobject(void);
+void cpufreq_put_global_kobject(void);
+int cpufreq_sysfs_create_file(const struct attribute *attr);
+void cpufreq_sysfs_remove_file(const struct attribute *attr);
 
-struct cpufreq_freqs {
-	unsigned int cpu;	/* cpu nr */
-	unsigned int old;
-	unsigned int new;
-	u8 flags;		/* flags of cpufreq_driver, see below. */
-};
+#ifdef CONFIG_CPU_FREQ
+unsigned int cpufreq_get(unsigned int cpu);
+unsigned int cpufreq_quick_get(unsigned int cpu);
+unsigned int cpufreq_quick_get_max(unsigned int cpu);
+void disable_cpufreq(void);
 
-/**
- * cpufreq_scale - "old * mult / div" calculation for large values (32-bit-arch
- * safe)
- * @old:   old value
- * @div:   divisor
- * @mult:  multiplier
- *
- *
- * new = old * mult / div
- */
-static inline unsigned long cpufreq_scale(unsigned long old, u_int div,
-		u_int mult)
+u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy);
+int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu);
+int cpufreq_update_policy(unsigned int cpu);
+bool have_governor_per_policy(void);
+struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
+#else
+static inline unsigned int cpufreq_get(unsigned int cpu)
 {
-#if BITS_PER_LONG == 32
-
-	u64 result = ((u64) old) * ((u64) mult);
-	do_div(result, div);
-	return (unsigned long) result;
-
-#elif BITS_PER_LONG == 64
-
-	unsigned long result = old * ((u64) mult);
-	result /= div;
-	return result;
-
+	return 0;
+}
+static inline unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+	return 0;
+}
+static inline unsigned int cpufreq_quick_get_max(unsigned int cpu)
+{
+	return 0;
+}
+static inline void disable_cpufreq(void) { }
 #endif
-};
 
 /*********************************************************************
- *                          CPUFREQ GOVERNORS                        *
+ *                      CPUFREQ DRIVER INTERFACE                     *
  *********************************************************************/
 
-#define CPUFREQ_GOV_START	1
-#define CPUFREQ_GOV_STOP	2
-#define CPUFREQ_GOV_LIMITS	3
-#define CPUFREQ_GOV_POLICY_INIT	4
-#define CPUFREQ_GOV_POLICY_EXIT	5
+#define CPUFREQ_RELATION_L 0  /* lowest frequency at or above target */
+#define CPUFREQ_RELATION_H 1  /* highest frequency below or at target */
 
-struct cpufreq_governor {
-	char	name[CPUFREQ_NAME_LEN];
-	int	initialized;
-	int	(*governor)	(struct cpufreq_policy *policy,
-				 unsigned int event);
-	ssize_t	(*show_setspeed)	(struct cpufreq_policy *policy,
-					 char *buf);
-	int	(*store_setspeed)	(struct cpufreq_policy *policy,
-					 unsigned int freq);
-	unsigned int max_transition_latency; /* HW must be able to switch to
-			next freq faster than this value in nano secs or we
-			will fallback to performance governor */
-	struct list_head	governor_list;
-	struct module		*owner;
+struct freq_attr {
+	struct attribute attr;
+	ssize_t (*show)(struct cpufreq_policy *, char *);
+	ssize_t (*store)(struct cpufreq_policy *, const char *, size_t count);
 };
 
-/*
- * Pass a target to the cpufreq driver.
- */
-extern int cpufreq_driver_target(struct cpufreq_policy *policy,
-				 unsigned int target_freq,
-				 unsigned int relation);
-extern int __cpufreq_driver_target(struct cpufreq_policy *policy,
-				   unsigned int target_freq,
-				   unsigned int relation);
-int cpufreq_register_governor(struct cpufreq_governor *governor);
-void cpufreq_unregister_governor(struct cpufreq_governor *governor);
+#define cpufreq_freq_attr_ro(_name)		\
+static struct freq_attr _name =			\
+__ATTR(_name, 0444, show_##_name, NULL)
 
-/*********************************************************************
- *                      CPUFREQ DRIVER INTERFACE                     *
- *********************************************************************/
+#define cpufreq_freq_attr_ro_perm(_name, _perm)	\
+static struct freq_attr _name =			\
+__ATTR(_name, _perm, show_##_name, NULL)
 
-#define CPUFREQ_RELATION_L 0  /* lowest frequency at or above target */
-#define CPUFREQ_RELATION_H 1  /* highest frequency below or at target */
+#define cpufreq_freq_attr_rw(_name)		\
+static struct freq_attr _name =			\
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
+struct global_attr {
+	struct attribute attr;
+	ssize_t (*show)(struct kobject *kobj,
+			struct attribute *attr, char *buf);
+	ssize_t (*store)(struct kobject *a, struct attribute *b,
+			 const char *c, size_t count);
+};
+
+#define define_one_global_ro(_name)		\
+static struct global_attr _name =		\
+__ATTR(_name, 0444, show_##_name, NULL)
+
+#define define_one_global_rw(_name)		\
+static struct global_attr _name =		\
+__ATTR(_name, 0644, show_##_name, store_##_name)
 
-struct freq_attr;
 
 struct cpufreq_driver {
 	struct module		*owner;
@@ -258,7 +212,6 @@ struct cpufreq_driver {
 };
 
 /* flags */
-
 #define CPUFREQ_STICKY		0x01	/* the driver isn't removed even if
 					 * all ->init() calls failed */
 #define CPUFREQ_CONST_LOOPS	0x02	/* loops_per_jiffy or other kernel
@@ -270,8 +223,7 @@ struct cpufreq_driver {
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
 
-void cpufreq_notify_transition(struct cpufreq_policy *policy,
-		struct cpufreq_freqs *freqs, unsigned int state);
+const char *cpufreq_get_current_driver(void);
 
 static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
 		unsigned int min, unsigned int max)
@@ -289,87 +241,118 @@ static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
 	return;
 }
 
-struct freq_attr {
-	struct attribute attr;
-	ssize_t (*show)(struct cpufreq_policy *, char *);
-	ssize_t (*store)(struct cpufreq_policy *, const char *, size_t count);
-};
-
-#define cpufreq_freq_attr_ro(_name)		\
-static struct freq_attr _name =			\
-__ATTR(_name, 0444, show_##_name, NULL)
-
-#define cpufreq_freq_attr_ro_perm(_name, _perm)	\
-static struct freq_attr _name =			\
-__ATTR(_name, _perm, show_##_name, NULL)
-
-#define cpufreq_freq_attr_rw(_name)		\
-static struct freq_attr _name =			\
-__ATTR(_name, 0644, show_##_name, store_##_name)
+/*********************************************************************
+ *                     CPUFREQ NOTIFIER INTERFACE                    *
+ *********************************************************************/
 
-struct global_attr {
-	struct attribute attr;
-	ssize_t (*show)(struct kobject *kobj,
-			struct attribute *attr, char *buf);
-	ssize_t (*store)(struct kobject *a, struct attribute *b,
-			 const char *c, size_t count);
-};
+#define CPUFREQ_TRANSITION_NOTIFIER	(0)
+#define CPUFREQ_POLICY_NOTIFIER		(1)
 
-#define define_one_global_ro(_name)		\
-static struct global_attr _name =		\
-__ATTR(_name, 0444, show_##_name, NULL)
+/* Transition notifiers */
+#define CPUFREQ_PRECHANGE		(0)
+#define CPUFREQ_POSTCHANGE		(1)
+#define CPUFREQ_RESUMECHANGE		(8)
+#define CPUFREQ_SUSPENDCHANGE		(9)
 
-#define define_one_global_rw(_name)		\
-static struct global_attr _name =		\
-__ATTR(_name, 0644, show_##_name, store_##_name)
+/* Policy Notifiers  */
+#define CPUFREQ_ADJUST			(0)
+#define CPUFREQ_INCOMPATIBLE		(1)
+#define CPUFREQ_NOTIFY			(2)
+#define CPUFREQ_START			(3)
+#define CPUFREQ_UPDATE_POLICY_CPU	(4)
 
-struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
-void cpufreq_cpu_put(struct cpufreq_policy *data);
-const char *cpufreq_get_current_driver(void);
+#ifdef CONFIG_CPU_FREQ
+int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);
+int cpufreq_unregister_notifier(struct notifier_block *nb, unsigned int list);
 
-/*********************************************************************
- *                        CPUFREQ 2.6. INTERFACE                     *
- *********************************************************************/
-u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy);
-int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu);
-int cpufreq_update_policy(unsigned int cpu);
-bool have_governor_per_policy(void);
-struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
+void cpufreq_notify_transition(struct cpufreq_policy *policy,
+		struct cpufreq_freqs *freqs, unsigned int state);
 
-#ifdef CONFIG_CPU_FREQ
-/*
- * query the current CPU frequency (in kHz). If zero, cpufreq couldn't detect it
- */
-unsigned int cpufreq_get(unsigned int cpu);
-#else
-static inline unsigned int cpufreq_get(unsigned int cpu)
+#else /* CONFIG_CPU_FREQ */
+static inline int cpufreq_register_notifier(struct notifier_block *nb,
+						unsigned int list)
 {
 	return 0;
 }
-#endif
-
-/*
- * query the last known CPU freq (in kHz). If zero, cpufreq couldn't detect it
- */
-#ifdef CONFIG_CPU_FREQ
-unsigned int cpufreq_quick_get(unsigned int cpu);
-unsigned int cpufreq_quick_get_max(unsigned int cpu);
-#else
-static inline unsigned int cpufreq_quick_get(unsigned int cpu)
+static inline int cpufreq_unregister_notifier(struct notifier_block *nb,
+						unsigned int list)
 {
 	return 0;
 }
-static inline unsigned int cpufreq_quick_get_max(unsigned int cpu)
+#endif /* !CONFIG_CPU_FREQ */
+
+/**
+ * cpufreq_scale - "old * mult / div" calculation for large values (32-bit-arch
+ * safe)
+ * @old:   old value
+ * @div:   divisor
+ * @mult:  multiplier
+ *
+ *
+ * new = old * mult / div
+ */
+static inline unsigned long cpufreq_scale(unsigned long old, u_int div,
+		u_int mult)
 {
-	return 0;
-}
+#if BITS_PER_LONG == 32
+	u64 result = ((u64) old) * ((u64) mult);
+	do_div(result, div);
+	return (unsigned long) result;
+
+#elif BITS_PER_LONG == 64
+	unsigned long result = old * ((u64) mult);
+	result /= div;
+	return result;
 #endif
+}
 
 /*********************************************************************
- *                       CPUFREQ DEFAULT GOVERNOR                    *
+ *                          CPUFREQ GOVERNORS                        *
  *********************************************************************/
 
 /*
+ * If (cpufreq_driver->target) exists, the ->governor decides what frequency
+ * within the limits is used. If (cpufreq_driver->setpolicy> exists, these
+ * two generic policies are available:
+ */
+#define CPUFREQ_POLICY_POWERSAVE	(1)
+#define CPUFREQ_POLICY_PERFORMANCE	(2)
+
+/* Governor Events */
+#define CPUFREQ_GOV_START	1
+#define CPUFREQ_GOV_STOP	2
+#define CPUFREQ_GOV_LIMITS	3
+#define CPUFREQ_GOV_POLICY_INIT	4
+#define CPUFREQ_GOV_POLICY_EXIT	5
+
+struct cpufreq_governor {
+	char	name[CPUFREQ_NAME_LEN];
+	int	initialized;
+	int	(*governor)	(struct cpufreq_policy *policy,
+				 unsigned int event);
+	ssize_t	(*show_setspeed)	(struct cpufreq_policy *policy,
+					 char *buf);
+	int	(*store_setspeed)	(struct cpufreq_policy *policy,
+					 unsigned int freq);
+	unsigned int max_transition_latency; /* HW must be able to switch to
+			next freq faster than this value in nano secs or we
+			will fallback to performance governor */
+	struct list_head	governor_list;
+	struct module		*owner;
+};
+
+/* Pass a target to the cpufreq driver */
+int cpufreq_driver_target(struct cpufreq_policy *policy,
+				 unsigned int target_freq,
+				 unsigned int relation);
+int __cpufreq_driver_target(struct cpufreq_policy *policy,
+				   unsigned int target_freq,
+				   unsigned int relation);
+int cpufreq_register_governor(struct cpufreq_governor *governor);
+void cpufreq_unregister_governor(struct cpufreq_governor *governor);
+
+/* CPUFREQ DEFAULT GOVERNOR */
+/*
  * Performance governor is fallback governor if any other gov failed to auto
  * load due latency restrictions
  */
@@ -417,18 +400,16 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
 				   unsigned int relation,
 				   unsigned int *index);
 
-/* the following 3 funtions are for cpufreq core use only */
+void cpufreq_frequency_table_update_policy_cpu(struct cpufreq_policy *policy);
+ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf);
+
+/* the following funtion is for cpufreq core use only */
 struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu);
 
 /* the following are really really optional */
 extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
-
 void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
 				      unsigned int cpu);
-void cpufreq_frequency_table_update_policy_cpu(struct cpufreq_policy *policy);
-
 void cpufreq_frequency_table_put_attr(unsigned int cpu);
 
-ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf);
-
 #endif /* _LINUX_CPUFREQ_H */
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 03/11] cpufreq: Give consistent names for struct cpufreq_policy *
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 01/11] cpufreq: Cleanup header files included in core Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 02/11] cpufreq: Re-arrange declarations in cpufreq.h Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 04/11] cpufreq: Use sizeof(*ptr) form for finding size of a struct Viresh Kumar
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

They are named as policy, cur_policy, new_policy, data, etc.. Just name them
poicy wherever possible.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c          | 200 ++++++++++++++++++-------------------
 drivers/cpufreq/cpufreq_governor.h |   2 +-
 drivers/cpufreq/cpufreq_ondemand.c |  10 +-
 drivers/cpufreq/cpufreq_stats.c    |  12 +--
 include/linux/cpufreq.h            |   2 +-
 5 files changed, 113 insertions(+), 113 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index aaccfa0..8b087e5 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -179,7 +179,7 @@ EXPORT_SYMBOL_GPL(get_cpu_idle_time);
 
 static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs)
 {
-	struct cpufreq_policy *data;
+	struct cpufreq_policy *policy;
 	unsigned long flags;
 
 	if (cpu >= nr_cpu_ids)
@@ -195,16 +195,16 @@ static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs)
 		goto err_out_unlock;
 
 	/* get the CPU */
-	data = per_cpu(cpufreq_cpu_data, cpu);
+	policy = per_cpu(cpufreq_cpu_data, cpu);
 
-	if (!data)
+	if (!policy)
 		goto err_out_put_module;
 
-	if (!sysfs && !kobject_get(&data->kobj))
+	if (!sysfs && !kobject_get(&policy->kobj))
 		goto err_out_put_module;
 
 	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-	return data;
+	return policy;
 
 err_out_put_module:
 	module_put(cpufreq_driver->owner);
@@ -228,25 +228,25 @@ static struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu)
 	return __cpufreq_cpu_get(cpu, true);
 }
 
-static void __cpufreq_cpu_put(struct cpufreq_policy *data, bool sysfs)
+static void __cpufreq_cpu_put(struct cpufreq_policy *policy, bool sysfs)
 {
 	if (!sysfs)
-		kobject_put(&data->kobj);
+		kobject_put(&policy->kobj);
 	module_put(cpufreq_driver->owner);
 }
 
-void cpufreq_cpu_put(struct cpufreq_policy *data)
+void cpufreq_cpu_put(struct cpufreq_policy *policy)
 {
 	if (cpufreq_disabled())
 		return;
 
-	__cpufreq_cpu_put(data, false);
+	__cpufreq_cpu_put(policy, false);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
 
-static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *data)
+static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *policy)
 {
-	__cpufreq_cpu_put(data, true);
+	__cpufreq_cpu_put(policy, true);
 }
 
 /*********************************************************************
@@ -453,8 +453,8 @@ show_one(scaling_min_freq, min);
 show_one(scaling_max_freq, max);
 show_one(scaling_cur_freq, cur);
 
-static int __cpufreq_set_policy(struct cpufreq_policy *data,
-				struct cpufreq_policy *policy);
+static int __cpufreq_set_policy(struct cpufreq_policy *policy,
+				struct cpufreq_policy *new_policy);
 
 /**
  * cpufreq_per_cpu_attr_write() / store_##file_name() - sysfs write access
@@ -1142,7 +1142,7 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
 			CPUFREQ_UPDATE_POLICY_CPU, policy);
 }
 
-static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *data,
+static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
 					   unsigned int old_cpu, bool frozen)
 {
 	struct device *cpu_dev;
@@ -1150,27 +1150,27 @@ static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *data,
 	int ret;
 
 	/* first sibling now owns the new sysfs dir */
-	cpu_dev = get_cpu_device(cpumask_first(data->cpus));
+	cpu_dev = get_cpu_device(cpumask_first(policy->cpus));
 
 	/* Don't touch sysfs files during light-weight tear-down */
 	if (frozen)
 		return cpu_dev->id;
 
 	sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
-	ret = kobject_move(&data->kobj, &cpu_dev->kobj);
+	ret = kobject_move(&policy->kobj, &cpu_dev->kobj);
 	if (ret) {
 		pr_err("%s: Failed to move kobj: %d", __func__, ret);
 
 		WARN_ON(lock_policy_rwsem_write(old_cpu));
-		cpumask_set_cpu(old_cpu, data->cpus);
+		cpumask_set_cpu(old_cpu, policy->cpus);
 
 		write_lock_irqsave(&cpufreq_driver_lock, flags);
-		per_cpu(cpufreq_cpu_data, old_cpu) = data;
+		per_cpu(cpufreq_cpu_data, old_cpu) = policy;
 		write_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
 		unlock_policy_rwsem_write(old_cpu);
 
-		ret = sysfs_create_link(&cpu_dev->kobj, &data->kobj,
+		ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
 					"cpufreq");
 
 		return -EINVAL;
@@ -1192,7 +1192,7 @@ static int __cpufreq_remove_dev(struct device *dev,
 	unsigned int cpu = dev->id, cpus;
 	int new_cpu;
 	unsigned long flags;
-	struct cpufreq_policy *data;
+	struct cpufreq_policy *policy;
 	struct kobject *kobj;
 	struct completion *cmp;
 
@@ -1200,44 +1200,44 @@ static int __cpufreq_remove_dev(struct device *dev,
 
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
 
-	data = per_cpu(cpufreq_cpu_data, cpu);
+	policy = per_cpu(cpufreq_cpu_data, cpu);
 	per_cpu(cpufreq_cpu_data, cpu) = NULL;
 
 	/* Save the policy somewhere when doing a light-weight tear-down */
 	if (frozen)
-		per_cpu(cpufreq_cpu_data_fallback, cpu) = data;
+		per_cpu(cpufreq_cpu_data_fallback, cpu) = policy;
 
 	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
-	if (!data) {
+	if (!policy) {
 		pr_debug("%s: No cpu_data found\n", __func__);
 		return -EINVAL;
 	}
 
 	if (cpufreq_driver->target)
-		__cpufreq_governor(data, CPUFREQ_GOV_STOP);
+		__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 
 #ifdef CONFIG_HOTPLUG_CPU
 	if (!cpufreq_driver->setpolicy)
 		strncpy(per_cpu(cpufreq_cpu_governor, cpu),
-			data->governor->name, CPUFREQ_NAME_LEN);
+			policy->governor->name, CPUFREQ_NAME_LEN);
 #endif
 
 	WARN_ON(lock_policy_rwsem_write(cpu));
-	cpus = cpumask_weight(data->cpus);
+	cpus = cpumask_weight(policy->cpus);
 
 	if (cpus > 1)
-		cpumask_clear_cpu(cpu, data->cpus);
+		cpumask_clear_cpu(cpu, policy->cpus);
 	unlock_policy_rwsem_write(cpu);
 
-	if (cpu != data->cpu && !frozen) {
+	if (cpu != policy->cpu && !frozen) {
 		sysfs_remove_link(&dev->kobj, "cpufreq");
 	} else if (cpus > 1) {
 
-		new_cpu = cpufreq_nominate_new_policy_cpu(data, cpu, frozen);
+		new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen);
 		if (new_cpu >= 0) {
 			WARN_ON(lock_policy_rwsem_write(cpu));
-			update_policy_cpu(data, new_cpu);
+			update_policy_cpu(policy, new_cpu);
 			unlock_policy_rwsem_write(cpu);
 
 			if (!frozen) {
@@ -1250,12 +1250,12 @@ static int __cpufreq_remove_dev(struct device *dev,
 	/* If cpu is last user of policy, free policy */
 	if (cpus == 1) {
 		if (cpufreq_driver->target)
-			__cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
+			__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
 
 		if (!frozen) {
 			lock_policy_rwsem_read(cpu);
-			kobj = &data->kobj;
-			cmp = &data->kobj_unregister;
+			kobj = &policy->kobj;
+			cmp = &policy->kobj_unregister;
 			unlock_policy_rwsem_read(cpu);
 			kobject_put(kobj);
 
@@ -1275,14 +1275,14 @@ static int __cpufreq_remove_dev(struct device *dev,
 		 * subsequent light-weight ->init() to succeed.
 		 */
 		if (cpufreq_driver->exit)
-			cpufreq_driver->exit(data);
+			cpufreq_driver->exit(policy);
 
 		if (!frozen)
-			cpufreq_policy_free(data);
+			cpufreq_policy_free(policy);
 	} else {
 		if (cpufreq_driver->target) {
-			__cpufreq_governor(data, CPUFREQ_GOV_START);
-			__cpufreq_governor(data, CPUFREQ_GOV_LIMITS);
+			__cpufreq_governor(policy, CPUFREQ_GOV_START);
+			__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
 		}
 	}
 
@@ -1456,23 +1456,23 @@ static int cpufreq_bp_suspend(void)
 	int ret = 0;
 
 	int cpu = smp_processor_id();
-	struct cpufreq_policy *cpu_policy;
+	struct cpufreq_policy *policy;
 
 	pr_debug("suspending cpu %u\n", cpu);
 
 	/* If there's no policy for the boot CPU, we have nothing to do. */
-	cpu_policy = cpufreq_cpu_get(cpu);
-	if (!cpu_policy)
+	policy = cpufreq_cpu_get(cpu);
+	if (!policy)
 		return 0;
 
 	if (cpufreq_driver->suspend) {
-		ret = cpufreq_driver->suspend(cpu_policy);
+		ret = cpufreq_driver->suspend(policy);
 		if (ret)
 			printk(KERN_ERR "cpufreq: suspend failed in ->suspend "
-					"step on CPU %u\n", cpu_policy->cpu);
+					"step on CPU %u\n", policy->cpu);
 	}
 
-	cpufreq_cpu_put(cpu_policy);
+	cpufreq_cpu_put(policy);
 	return ret;
 }
 
@@ -1494,28 +1494,28 @@ static void cpufreq_bp_resume(void)
 	int ret = 0;
 
 	int cpu = smp_processor_id();
-	struct cpufreq_policy *cpu_policy;
+	struct cpufreq_policy *policy;
 
 	pr_debug("resuming cpu %u\n", cpu);
 
 	/* If there's no policy for the boot CPU, we have nothing to do. */
-	cpu_policy = cpufreq_cpu_get(cpu);
-	if (!cpu_policy)
+	policy = cpufreq_cpu_get(cpu);
+	if (!policy)
 		return;
 
 	if (cpufreq_driver->resume) {
-		ret = cpufreq_driver->resume(cpu_policy);
+		ret = cpufreq_driver->resume(policy);
 		if (ret) {
 			printk(KERN_ERR "cpufreq: resume failed in ->resume "
-					"step on CPU %u\n", cpu_policy->cpu);
+					"step on CPU %u\n", policy->cpu);
 			goto fail;
 		}
 	}
 
-	schedule_work(&cpu_policy->update);
+	schedule_work(&policy->update);
 
 fail:
-	cpufreq_cpu_put(cpu_policy);
+	cpufreq_cpu_put(policy);
 }
 
 static struct syscore_ops cpufreq_syscore_ops = {
@@ -1835,95 +1835,95 @@ EXPORT_SYMBOL(cpufreq_get_policy);
  * data   : current policy.
  * policy : policy to be set.
  */
-static int __cpufreq_set_policy(struct cpufreq_policy *data,
-				struct cpufreq_policy *policy)
+static int __cpufreq_set_policy(struct cpufreq_policy *policy,
+				struct cpufreq_policy *new_policy)
 {
 	int ret = 0, failed = 1;
 
-	pr_debug("setting new policy for CPU %u: %u - %u kHz\n", policy->cpu,
-		policy->min, policy->max);
+	pr_debug("setting new policy for CPU %u: %u - %u kHz\n", new_policy->cpu,
+		new_policy->min, new_policy->max);
 
-	memcpy(&policy->cpuinfo, &data->cpuinfo,
+	memcpy(&new_policy->cpuinfo, &policy->cpuinfo,
 				sizeof(struct cpufreq_cpuinfo));
 
-	if (policy->min > data->max || policy->max < data->min) {
+	if (new_policy->min > policy->max || new_policy->max < policy->min) {
 		ret = -EINVAL;
 		goto error_out;
 	}
 
 	/* verify the cpu speed can be set within this limit */
-	ret = cpufreq_driver->verify(policy);
+	ret = cpufreq_driver->verify(new_policy);
 	if (ret)
 		goto error_out;
 
 	/* adjust if necessary - all reasons */
 	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
-			CPUFREQ_ADJUST, policy);
+			CPUFREQ_ADJUST, new_policy);
 
 	/* adjust if necessary - hardware incompatibility*/
 	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
-			CPUFREQ_INCOMPATIBLE, policy);
+			CPUFREQ_INCOMPATIBLE, new_policy);
 
 	/*
 	 * verify the cpu speed can be set within this limit, which might be
 	 * different to the first one
 	 */
-	ret = cpufreq_driver->verify(policy);
+	ret = cpufreq_driver->verify(new_policy);
 	if (ret)
 		goto error_out;
 
 	/* notification of the new policy */
 	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
-			CPUFREQ_NOTIFY, policy);
+			CPUFREQ_NOTIFY, new_policy);
 
-	data->min = policy->min;
-	data->max = policy->max;
+	policy->min = new_policy->min;
+	policy->max = new_policy->max;
 
 	pr_debug("new min and max freqs are %u - %u kHz\n",
-					data->min, data->max);
+					policy->min, policy->max);
 
 	if (cpufreq_driver->setpolicy) {
-		data->policy = policy->policy;
+		policy->policy = new_policy->policy;
 		pr_debug("setting range\n");
-		ret = cpufreq_driver->setpolicy(policy);
+		ret = cpufreq_driver->setpolicy(new_policy);
 	} else {
-		if (policy->governor != data->governor) {
+		if (new_policy->governor != policy->governor) {
 			/* save old, working values */
-			struct cpufreq_governor *old_gov = data->governor;
+			struct cpufreq_governor *old_gov = policy->governor;
 
 			pr_debug("governor switch\n");
 
 			/* end old governor */
-			if (data->governor) {
-				__cpufreq_governor(data, CPUFREQ_GOV_STOP);
-				unlock_policy_rwsem_write(policy->cpu);
-				__cpufreq_governor(data,
+			if (policy->governor) {
+				__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+				unlock_policy_rwsem_write(new_policy->cpu);
+				__cpufreq_governor(policy,
 						CPUFREQ_GOV_POLICY_EXIT);
-				lock_policy_rwsem_write(policy->cpu);
+				lock_policy_rwsem_write(new_policy->cpu);
 			}
 
 			/* start new governor */
-			data->governor = policy->governor;
-			if (!__cpufreq_governor(data, CPUFREQ_GOV_POLICY_INIT)) {
-				if (!__cpufreq_governor(data, CPUFREQ_GOV_START)) {
+			policy->governor = new_policy->governor;
+			if (!__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT)) {
+				if (!__cpufreq_governor(policy, CPUFREQ_GOV_START)) {
 					failed = 0;
 				} else {
-					unlock_policy_rwsem_write(policy->cpu);
-					__cpufreq_governor(data,
+					unlock_policy_rwsem_write(new_policy->cpu);
+					__cpufreq_governor(policy,
 							CPUFREQ_GOV_POLICY_EXIT);
-					lock_policy_rwsem_write(policy->cpu);
+					lock_policy_rwsem_write(new_policy->cpu);
 				}
 			}
 
 			if (failed) {
 				/* new governor failed, so re-start old one */
 				pr_debug("starting governor %s failed\n",
-							data->governor->name);
+							policy->governor->name);
 				if (old_gov) {
-					data->governor = old_gov;
-					__cpufreq_governor(data,
+					policy->governor = old_gov;
+					__cpufreq_governor(policy,
 							CPUFREQ_GOV_POLICY_INIT);
-					__cpufreq_governor(data,
+					__cpufreq_governor(policy,
 							   CPUFREQ_GOV_START);
 				}
 				ret = -EINVAL;
@@ -1932,7 +1932,7 @@ static int __cpufreq_set_policy(struct cpufreq_policy *data,
 			/* might be a policy change, too, so fall through */
 		}
 		pr_debug("governor: change or update limits\n");
-		__cpufreq_governor(data, CPUFREQ_GOV_LIMITS);
+		__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
 	}
 
 error_out:
@@ -1948,11 +1948,11 @@ error_out:
  */
 int cpufreq_update_policy(unsigned int cpu)
 {
-	struct cpufreq_policy *data = cpufreq_cpu_get(cpu);
-	struct cpufreq_policy policy;
+	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+	struct cpufreq_policy new_policy;
 	int ret;
 
-	if (!data) {
+	if (!policy) {
 		ret = -ENODEV;
 		goto no_policy;
 	}
@@ -1963,34 +1963,34 @@ int cpufreq_update_policy(unsigned int cpu)
 	}
 
 	pr_debug("updating policy for CPU %u\n", cpu);
-	memcpy(&policy, data, sizeof(struct cpufreq_policy));
-	policy.min = data->user_policy.min;
-	policy.max = data->user_policy.max;
-	policy.policy = data->user_policy.policy;
-	policy.governor = data->user_policy.governor;
+	memcpy(&new_policy, policy, sizeof(struct cpufreq_policy));
+	new_policy.min = policy->user_policy.min;
+	new_policy.max = policy->user_policy.max;
+	new_policy.policy = policy->user_policy.policy;
+	new_policy.governor = policy->user_policy.governor;
 
 	/*
 	 * BIOS might change freq behind our back
 	 * -> ask driver for current freq and notify governors about a change
 	 */
 	if (cpufreq_driver->get) {
-		policy.cur = cpufreq_driver->get(cpu);
-		if (!data->cur) {
+		new_policy.cur = cpufreq_driver->get(cpu);
+		if (!policy->cur) {
 			pr_debug("Driver did not initialize current freq");
-			data->cur = policy.cur;
+			policy->cur = new_policy.cur;
 		} else {
-			if (data->cur != policy.cur && cpufreq_driver->target)
-				cpufreq_out_of_sync(cpu, data->cur,
-								policy.cur);
+			if (policy->cur != new_policy.cur && cpufreq_driver->target)
+				cpufreq_out_of_sync(cpu, policy->cur,
+								new_policy.cur);
 		}
 	}
 
-	ret = __cpufreq_set_policy(data, &policy);
+	ret = __cpufreq_set_policy(policy, &new_policy);
 
 	unlock_policy_rwsem_write(cpu);
 
 fail:
-	cpufreq_cpu_put(data);
+	cpufreq_cpu_put(policy);
 no_policy:
 	return ret;
 }
diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index 7db4373..a02d78b 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -221,7 +221,7 @@ struct od_ops {
 	void (*powersave_bias_init_cpu)(int cpu);
 	unsigned int (*powersave_bias_target)(struct cpufreq_policy *policy,
 			unsigned int freq_next, unsigned int relation);
-	void (*freq_increase)(struct cpufreq_policy *p, unsigned int freq);
+	void (*freq_increase)(struct cpufreq_policy *policy, unsigned int freq);
 };
 
 struct cs_ops {
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 232de60..8f134b3 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -132,18 +132,18 @@ static void ondemand_powersave_bias_init(void)
 	}
 }
 
-static void dbs_freq_increase(struct cpufreq_policy *p, unsigned int freq)
+static void dbs_freq_increase(struct cpufreq_policy *policy, unsigned int freq)
 {
-	struct dbs_data *dbs_data = p->governor_data;
+	struct dbs_data *dbs_data = policy->governor_data;
 	struct od_dbs_tuners *od_tuners = dbs_data->tuners;
 
 	if (od_tuners->powersave_bias)
-		freq = od_ops.powersave_bias_target(p, freq,
+		freq = od_ops.powersave_bias_target(policy, freq,
 				CPUFREQ_RELATION_H);
-	else if (p->cur == p->max)
+	else if (policy->cur == policy->max)
 		return;
 
-	__cpufreq_driver_target(p, freq, od_tuners->powersave_bias ?
+	__cpufreq_driver_target(policy, freq, od_tuners->powersave_bias ?
 			CPUFREQ_RELATION_L : CPUFREQ_RELATION_H);
 }
 
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index a7143b0..a17b14e 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -193,7 +193,7 @@ static int cpufreq_stats_create_table(struct cpufreq_policy *policy,
 {
 	unsigned int i, j, count = 0, ret = 0;
 	struct cpufreq_stats *stat;
-	struct cpufreq_policy *data;
+	struct cpufreq_policy *current_policy;
 	unsigned int alloc_size;
 	unsigned int cpu = policy->cpu;
 	if (per_cpu(cpufreq_stats_table, cpu))
@@ -202,13 +202,13 @@ static int cpufreq_stats_create_table(struct cpufreq_policy *policy,
 	if ((stat) == NULL)
 		return -ENOMEM;
 
-	data = cpufreq_cpu_get(cpu);
-	if (data == NULL) {
+	current_policy = cpufreq_cpu_get(cpu);
+	if (current_policy == NULL) {
 		ret = -EINVAL;
 		goto error_get_fail;
 	}
 
-	ret = sysfs_create_group(&data->kobj, &stats_attr_group);
+	ret = sysfs_create_group(&current_policy->kobj, &stats_attr_group);
 	if (ret)
 		goto error_out;
 
@@ -251,10 +251,10 @@ static int cpufreq_stats_create_table(struct cpufreq_policy *policy,
 	stat->last_time = get_jiffies_64();
 	stat->last_index = freq_table_get_index(stat, policy->cur);
 	spin_unlock(&cpufreq_stats_lock);
-	cpufreq_cpu_put(data);
+	cpufreq_cpu_put(current_policy);
 	return 0;
 error_out:
-	cpufreq_cpu_put(data);
+	cpufreq_cpu_put(current_policy);
 error_get_fail:
 	kfree(stat);
 	per_cpu(cpufreq_stats_table, cpu) = NULL;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 0d3b026..2920892 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -94,7 +94,7 @@ struct cpufreq_policy {
 #define CPUFREQ_SHARED_TYPE_ANY	 (3) /* Freq can be set from any dependent CPU*/
 
 struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
-void cpufreq_cpu_put(struct cpufreq_policy *data);
+void cpufreq_cpu_put(struct cpufreq_policy *policy);
 
 static inline bool policy_is_shared(struct cpufreq_policy *policy)
 {
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 04/11] cpufreq: Use sizeof(*ptr) form for finding size of a struct
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (2 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 03/11] cpufreq: Give consistent names for struct cpufreq_policy * Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 05/11] cpufreq: Pass policy to cpufreq_add_policy_cpu() Viresh Kumar
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

Chapter 14 of Documentation/CodingStyle says:

The preferred form for passing a size of a struct is the following:

	p = kmalloc(sizeof(*p), ...);

The alternative form where struct name is spelled out hurts readability and
introduces an opportunity for a bug when the pointer variable type is changed
but the corresponding sizeof that is passed to a memory allocator is not.

This wasn't followed consistently in drivers/cpufreq, lets make it more
consistent by always following this rule.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/acpi-cpufreq.c         |  4 ++--
 drivers/cpufreq/cpufreq.c              |  9 ++++-----
 drivers/cpufreq/cpufreq_conservative.c |  2 +-
 drivers/cpufreq/cpufreq_ondemand.c     |  2 +-
 drivers/cpufreq/cpufreq_stats.c        |  2 +-
 drivers/cpufreq/e_powersaver.c         |  4 ++--
 drivers/cpufreq/exynos-cpufreq.c       |  2 +-
 drivers/cpufreq/gx-suspmod.c           |  2 +-
 drivers/cpufreq/ia64-acpi-cpufreq.c    |  4 ++--
 drivers/cpufreq/pmac64-cpufreq.c       |  5 ++---
 drivers/cpufreq/powernow-k7.c          | 13 ++++++-------
 drivers/cpufreq/powernow-k8.c          |  6 +++---
 drivers/cpufreq/s3c24xx-cpufreq.c      |  6 +++---
 drivers/cpufreq/sparc-us2e-cpufreq.c   |  5 ++---
 drivers/cpufreq/sparc-us3-cpufreq.c    |  5 ++---
 15 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index e673670..44758ce 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -709,7 +709,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return blacklisted;
 #endif
 
-	data = kzalloc(sizeof(struct acpi_cpufreq_data), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
@@ -799,7 +799,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		goto err_unreg;
 	}
 
-	data->freq_table = kmalloc(sizeof(struct cpufreq_frequency_table) *
+	data->freq_table = kmalloc(sizeof(*data->freq_table) *
 		    (perf->state_count+1), GFP_KERNEL);
 	if (!data->freq_table) {
 		result = -ENOMEM;
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 8b087e5..7864a90 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -873,7 +873,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
 	struct cpufreq_policy new_policy;
 	int ret = 0;
 
-	memcpy(&new_policy, policy, sizeof(struct cpufreq_policy));
+	memcpy(&new_policy, policy, sizeof(*policy));
 	/* assure that the starting sequence is run in __cpufreq_set_policy */
 	policy->governor = NULL;
 
@@ -1824,7 +1824,7 @@ int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu)
 	if (!cpu_policy)
 		return -EINVAL;
 
-	memcpy(policy, cpu_policy, sizeof(struct cpufreq_policy));
+	memcpy(policy, cpu_policy, sizeof(*policy));
 
 	cpufreq_cpu_put(cpu_policy);
 	return 0;
@@ -1843,8 +1843,7 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
 	pr_debug("setting new policy for CPU %u: %u - %u kHz\n", new_policy->cpu,
 		new_policy->min, new_policy->max);
 
-	memcpy(&new_policy->cpuinfo, &policy->cpuinfo,
-				sizeof(struct cpufreq_cpuinfo));
+	memcpy(&new_policy->cpuinfo, &policy->cpuinfo, sizeof(policy->cpuinfo));
 
 	if (new_policy->min > policy->max || new_policy->max < policy->min) {
 		ret = -EINVAL;
@@ -1963,7 +1962,7 @@ int cpufreq_update_policy(unsigned int cpu)
 	}
 
 	pr_debug("updating policy for CPU %u\n", cpu);
-	memcpy(&new_policy, policy, sizeof(struct cpufreq_policy));
+	memcpy(&new_policy, policy, sizeof(*policy));
 	new_policy.min = policy->user_policy.min;
 	new_policy.max = policy->user_policy.max;
 	new_policy.policy = policy->user_policy.policy;
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index d2bd9ef..7f67a75 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -317,7 +317,7 @@ static int cs_init(struct dbs_data *dbs_data)
 {
 	struct cs_dbs_tuners *tuners;
 
-	tuners = kzalloc(sizeof(struct cs_dbs_tuners), GFP_KERNEL);
+	tuners = kzalloc(sizeof(*tuners), GFP_KERNEL);
 	if (!tuners) {
 		pr_err("%s: kzalloc failed\n", __func__);
 		return -ENOMEM;
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 8f134b3..87f3305 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -482,7 +482,7 @@ static int od_init(struct dbs_data *dbs_data)
 	u64 idle_time;
 	int cpu;
 
-	tuners = kzalloc(sizeof(struct od_dbs_tuners), GFP_KERNEL);
+	tuners = kzalloc(sizeof(*tuners), GFP_KERNEL);
 	if (!tuners) {
 		pr_err("%s: kzalloc failed\n", __func__);
 		return -ENOMEM;
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index a17b14e..04452f0 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -198,7 +198,7 @@ static int cpufreq_stats_create_table(struct cpufreq_policy *policy,
 	unsigned int cpu = policy->cpu;
 	if (per_cpu(cpufreq_stats_table, cpu))
 		return -EBUSY;
-	stat = kzalloc(sizeof(struct cpufreq_stats), GFP_KERNEL);
+	stat = kzalloc(sizeof(*stat), GFP_KERNEL);
 	if ((stat) == NULL)
 		return -ENOMEM;
 
diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c
index a60efae..de974be 100644
--- a/drivers/cpufreq/e_powersaver.c
+++ b/drivers/cpufreq/e_powersaver.c
@@ -54,7 +54,7 @@ static struct acpi_processor_performance *eps_acpi_cpu_perf;
 /* Minimum necessary to get acpi_processor_get_bios_limit() working */
 static int eps_acpi_init(void)
 {
-	eps_acpi_cpu_perf = kzalloc(sizeof(struct acpi_processor_performance),
+	eps_acpi_cpu_perf = kzalloc(sizeof(*eps_acpi_cpu_perf),
 				      GFP_KERNEL);
 	if (!eps_acpi_cpu_perf)
 		return -ENOMEM;
@@ -366,7 +366,7 @@ static int eps_cpu_init(struct cpufreq_policy *policy)
 		states = 2;
 
 	/* Allocate private data and frequency table for current cpu */
-	centaur = kzalloc(sizeof(struct eps_cpu_data)
+	centaur = kzalloc(sizeof(*centaur)
 		    + (states + 1) * sizeof(struct cpufreq_frequency_table),
 		    GFP_KERNEL);
 	if (!centaur)
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index 0d32f02..3664751 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -289,7 +289,7 @@ static int __init exynos_cpufreq_init(void)
 {
 	int ret = -EINVAL;
 
-	exynos_info = kzalloc(sizeof(struct exynos_dvfs_info), GFP_KERNEL);
+	exynos_info = kzalloc(sizeof(*exynos_info), GFP_KERNEL);
 	if (!exynos_info)
 		return -ENOMEM;
 
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c
index 3dfc99b..4f25fb6 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -466,7 +466,7 @@ static int __init cpufreq_gx_init(void)
 
 	pr_debug("geode suspend modulation available.\n");
 
-	params = kzalloc(sizeof(struct gxfreq_params), GFP_KERNEL);
+	params = kzalloc(sizeof(*params), GFP_KERNEL);
 	if (params == NULL)
 		return -ENOMEM;
 
diff --git a/drivers/cpufreq/ia64-acpi-cpufreq.c b/drivers/cpufreq/ia64-acpi-cpufreq.c
index 573c14e..08792dd 100644
--- a/drivers/cpufreq/ia64-acpi-cpufreq.c
+++ b/drivers/cpufreq/ia64-acpi-cpufreq.c
@@ -274,7 +274,7 @@ acpi_cpufreq_cpu_init (
 
 	pr_debug("acpi_cpufreq_cpu_init\n");
 
-	data = kzalloc(sizeof(struct cpufreq_acpi_io), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return (-ENOMEM);
 
@@ -304,7 +304,7 @@ acpi_cpufreq_cpu_init (
 	}
 
 	/* alloc freq_table */
-	data->freq_table = kmalloc(sizeof(struct cpufreq_frequency_table) *
+	data->freq_table = kmalloc(sizeof(*data->freq_table) *
 	                           (data->acpi_data.state_count + 1),
 	                           GFP_KERNEL);
 	if (!data->freq_table) {
diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
index 7ba4234..4d7799b 100644
--- a/drivers/cpufreq/pmac64-cpufreq.c
+++ b/drivers/cpufreq/pmac64-cpufreq.c
@@ -447,9 +447,8 @@ static int __init g5_neo2_cpufreq_init(struct device_node *cpus)
 		if (!shdr)
 			goto bail_noprops;
 		g5_fvt_table = (struct smu_sdbp_fvt *)&shdr[1];
-		ssize = (shdr->len * sizeof(u32)) -
-			sizeof(struct smu_sdbp_header);
-		g5_fvt_count = ssize / sizeof(struct smu_sdbp_fvt);
+		ssize = (shdr->len * sizeof(u32)) - sizeof(*shdr);
+		g5_fvt_count = ssize / sizeof(*g5_fvt_table);
 		g5_fvt_cur = 0;
 
 		/* Sanity checking */
diff --git a/drivers/cpufreq/powernow-k7.c b/drivers/cpufreq/powernow-k7.c
index 9558708..4687d40 100644
--- a/drivers/cpufreq/powernow-k7.c
+++ b/drivers/cpufreq/powernow-k7.c
@@ -177,7 +177,7 @@ static int get_ranges(unsigned char *pst)
 	unsigned int speed;
 	u8 fid, vid;
 
-	powernow_table = kzalloc((sizeof(struct cpufreq_frequency_table) *
+	powernow_table = kzalloc((sizeof(*powernow_table) *
 				(number_scales + 1)), GFP_KERNEL);
 	if (!powernow_table)
 		return -ENOMEM;
@@ -309,8 +309,7 @@ static int powernow_acpi_init(void)
 		goto err0;
 	}
 
-	acpi_processor_perf = kzalloc(sizeof(struct acpi_processor_performance),
-				      GFP_KERNEL);
+	acpi_processor_perf = kzalloc(sizeof(*acpi_processor_perf), GFP_KERNEL);
 	if (!acpi_processor_perf) {
 		retval = -ENOMEM;
 		goto err0;
@@ -346,7 +345,7 @@ static int powernow_acpi_init(void)
 		goto err2;
 	}
 
-	powernow_table = kzalloc((sizeof(struct cpufreq_frequency_table) *
+	powernow_table = kzalloc((sizeof(*powernow_table) *
 				(number_scales + 1)), GFP_KERNEL);
 	if (!powernow_table) {
 		retval = -ENOMEM;
@@ -497,7 +496,7 @@ static int powernow_decode_bios(int maxfid, int startvid)
 					"relevant to this CPU).\n",
 					psb->numpst);
 
-			p += sizeof(struct psb_s);
+			p += sizeof(*psb);
 
 			pst = (struct pst_s *) p;
 
@@ -510,12 +509,12 @@ static int powernow_decode_bios(int maxfid, int startvid)
 				    (maxfid == pst->maxfid) &&
 				    (startvid == pst->startvid)) {
 					print_pst_entry(pst, j);
-					p = (char *)pst + sizeof(struct pst_s);
+					p = (char *)pst + sizeof(*pst);
 					ret = get_ranges(p);
 					return ret;
 				} else {
 					unsigned int k;
-					p = (char *)pst + sizeof(struct pst_s);
+					p = (char *)pst + sizeof(*pst);
 					for (k = 0; k < number_scales; k++)
 						p += 2;
 				}
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index c39d189..3f3c6ca 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -623,7 +623,7 @@ static int fill_powernow_table(struct powernow_k8_data *data,
 	if (check_pst_table(data, pst, maxvid))
 		return -EINVAL;
 
-	powernow_table = kmalloc((sizeof(struct cpufreq_frequency_table)
+	powernow_table = kmalloc((sizeof(*powernow_table)
 		* (data->numps + 1)), GFP_KERNEL);
 	if (!powernow_table) {
 		printk(KERN_ERR PFX "powernow_table memory alloc failure\n");
@@ -793,7 +793,7 @@ static int powernow_k8_cpu_init_acpi(struct powernow_k8_data *data)
 	}
 
 	/* fill in data->powernow_table */
-	powernow_table = kmalloc((sizeof(struct cpufreq_frequency_table)
+	powernow_table = kmalloc((sizeof(*powernow_table)
 		* (data->acpi_data.state_count + 1)), GFP_KERNEL);
 	if (!powernow_table) {
 		pr_debug("powernow_table memory alloc failure\n");
@@ -1106,7 +1106,7 @@ static int powernowk8_cpu_init(struct cpufreq_policy *pol)
 	if (rc)
 		return -ENODEV;
 
-	data = kzalloc(sizeof(struct powernow_k8_data), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data) {
 		printk(KERN_ERR PFX "unable to alloc powernow_k8_data");
 		return -ENOMEM;
diff --git a/drivers/cpufreq/s3c24xx-cpufreq.c b/drivers/cpufreq/s3c24xx-cpufreq.c
index 87781eb..f169ee5 100644
--- a/drivers/cpufreq/s3c24xx-cpufreq.c
+++ b/drivers/cpufreq/s3c24xx-cpufreq.c
@@ -522,7 +522,7 @@ int __init s3c_cpufreq_setboard(struct s3c_cpufreq_board *board)
 	/* Copy the board information so that each board can make this
 	 * initdata. */
 
-	ours = kzalloc(sizeof(struct s3c_cpufreq_board), GFP_KERNEL);
+	ours = kzalloc(sizeof(*ours), GFP_KERNEL);
 	if (ours == NULL) {
 		printk(KERN_ERR "%s: no memory\n", __func__);
 		return -ENOMEM;
@@ -615,7 +615,7 @@ static int s3c_cpufreq_build_freq(void)
 	size = cpu_cur.info->calc_freqtable(&cpu_cur, NULL, 0);
 	size++;
 
-	ftab = kmalloc(sizeof(struct cpufreq_frequency_table) * size, GFP_KERNEL);
+	ftab = kmalloc(sizeof(*ftab) * size, GFP_KERNEL);
 	if (!ftab) {
 		printk(KERN_ERR "%s: no memory for tables\n", __func__);
 		return -ENOMEM;
@@ -691,7 +691,7 @@ int __init s3c_plltab_register(struct cpufreq_frequency_table *plls,
 	struct cpufreq_frequency_table *vals;
 	unsigned int size;
 
-	size = sizeof(struct cpufreq_frequency_table) * (plls_no + 1);
+	size = sizeof(*vals) * (plls_no + 1);
 
 	vals = kmalloc(size, GFP_KERNEL);
 	if (vals) {
diff --git a/drivers/cpufreq/sparc-us2e-cpufreq.c b/drivers/cpufreq/sparc-us2e-cpufreq.c
index 93061a4..7c43a72 100644
--- a/drivers/cpufreq/sparc-us2e-cpufreq.c
+++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
@@ -351,12 +351,11 @@ static int __init us2e_freq_init(void)
 		struct cpufreq_driver *driver;
 
 		ret = -ENOMEM;
-		driver = kzalloc(sizeof(struct cpufreq_driver), GFP_KERNEL);
+		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
 		if (!driver)
 			goto err_out;
 
-		us2e_freq_table = kzalloc(
-			(NR_CPUS * sizeof(struct us2e_freq_percpu_info)),
+		us2e_freq_table = kzalloc((NR_CPUS * sizeof(*us2e_freq_table)),
 			GFP_KERNEL);
 		if (!us2e_freq_table)
 			goto err_out;
diff --git a/drivers/cpufreq/sparc-us3-cpufreq.c b/drivers/cpufreq/sparc-us3-cpufreq.c
index 880ee29..7f500c1 100644
--- a/drivers/cpufreq/sparc-us3-cpufreq.c
+++ b/drivers/cpufreq/sparc-us3-cpufreq.c
@@ -212,12 +212,11 @@ static int __init us3_freq_init(void)
 		struct cpufreq_driver *driver;
 
 		ret = -ENOMEM;
-		driver = kzalloc(sizeof(struct cpufreq_driver), GFP_KERNEL);
+		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
 		if (!driver)
 			goto err_out;
 
-		us3_freq_table = kzalloc(
-			(NR_CPUS * sizeof(struct us3_freq_percpu_info)),
+		us3_freq_table = kzalloc((NR_CPUS * sizeof(*us3_freq_table)),
 			GFP_KERNEL);
 		if (!us3_freq_table)
 			goto err_out;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 05/11] cpufreq: Pass policy to cpufreq_add_policy_cpu()
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (3 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 04/11] cpufreq: Use sizeof(*ptr) form for finding size of a struct Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 06/11] cpufreq: Store cpufreq policies in a list Viresh Kumar
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

Caller of cpufreq_add_policy_cpu() already has pointer to policy structure and
so there is no need to find it again in cpufreq_add_policy_cpu(). Lets pass it
directly.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 7864a90..9e83d91 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -890,21 +890,17 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
-static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling,
-				  struct device *dev, bool frozen)
+static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
+				  unsigned int cpu, struct device *dev,
+				  bool frozen)
 {
-	struct cpufreq_policy *policy;
 	int ret = 0, has_target = !!cpufreq_driver->target;
 	unsigned long flags;
 
-	policy = cpufreq_cpu_get(sibling);
-	if (WARN_ON_ONCE(!policy))
-		return -ENODATA;
-
 	if (has_target)
 		__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
 
-	lock_policy_rwsem_write(sibling);
+	lock_policy_rwsem_write(policy->cpu);
 
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
 
@@ -913,7 +909,7 @@ static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling,
 	per_cpu(cpufreq_cpu_data, cpu) = policy;
 	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
-	unlock_policy_rwsem_write(sibling);
+	unlock_policy_rwsem_write(policy->cpu);
 
 	if (has_target) {
 		__cpufreq_governor(policy, CPUFREQ_GOV_START);
@@ -924,7 +920,6 @@ static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling,
 	if (!frozen)
 		ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
 
-	cpufreq_cpu_put(policy);
 	return ret;
 }
 #endif
@@ -1007,8 +1002,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 		struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
 		if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) {
 			read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-			return cpufreq_add_policy_cpu(cpu, sibling, dev,
-						      frozen);
+			return cpufreq_add_policy_cpu(cp, cpu, dev, frozen);
 		}
 	}
 	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 06/11] cpufreq: Store cpufreq policies in a list
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (4 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 05/11] cpufreq: Pass policy to cpufreq_add_policy_cpu() Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies Viresh Kumar
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Lukasz Majewski, Myungjoo Ham, Viresh Kumar

From: Lukasz Majewski <l.majewski@samsung.com>

Policies available in a cpufreq framework are now linked together. They are
accessible via cpufreq_policy_list defined at cpufreq core.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 11 +++++++++++
 include/linux/cpufreq.h   |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9e83d91..f5999c4 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -40,6 +40,7 @@ static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
 static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data_fallback);
 static DEFINE_RWLOCK(cpufreq_driver_lock);
 static DEFINE_MUTEX(cpufreq_governor_lock);
+static LIST_HEAD(cpufreq_policy_list);
 
 #ifdef CONFIG_HOTPLUG_CPU
 /* This one keeps track of the previously set governor of a removed CPU */
@@ -964,6 +965,12 @@ err_free_policy:
 
 static void cpufreq_policy_free(struct cpufreq_policy *policy)
 {
+	unsigned long flags;
+
+	write_lock_irqsave(&cpufreq_driver_lock, flags);
+	list_del(&policy->policy_list);
+	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
+
 	free_cpumask_var(policy->related_cpus);
 	free_cpumask_var(policy->cpus);
 	kfree(policy);
@@ -1077,6 +1084,10 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 		ret = cpufreq_add_dev_interface(policy, dev);
 		if (ret)
 			goto err_out_unregister;
+
+		write_lock_irqsave(&cpufreq_driver_lock, flags);
+		list_add(&policy->policy_list, &cpufreq_policy_list);
+		write_unlock_irqrestore(&cpufreq_driver_lock, flags);
 	}
 
 	cpufreq_init_policy(policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 2920892..431a05d 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -82,6 +82,7 @@ struct cpufreq_policy {
 
 	struct cpufreq_real_policy	user_policy;
 
+	struct list_head        policy_list;
 	struct kobject		kobj;
 	struct completion	kobj_unregister;
 	int			transition_ongoing; /* Tracks transition status */
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (5 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 06/11] cpufreq: Store cpufreq policies in a list Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-18 14:06   ` Rafael J. Wysocki
  2013-08-06 17:23 ` [PATCH V2 08/11] cpufreq: Fix broken usage of governor->owner's refcount Viresh Kumar
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

For iterating over all policies currently we are iterating over all CPUs and
then finding their policies. Lets use the newly created infrastructure
cpufreq_policy_list.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f5999c4..fe04b79 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -984,8 +984,8 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 	struct cpufreq_policy *policy;
 	unsigned long flags;
 #ifdef CONFIG_HOTPLUG_CPU
+	struct cpufreq_policy *tpolicy;
 	struct cpufreq_governor *gov;
-	int sibling;
 #endif
 
 	if (cpu_is_offline(cpu))
@@ -1005,11 +1005,11 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 #ifdef CONFIG_HOTPLUG_CPU
 	/* Check if this cpu was hot-unplugged earlier and has siblings */
 	read_lock_irqsave(&cpufreq_driver_lock, flags);
-	for_each_online_cpu(sibling) {
-		struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
-		if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) {
+	list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) {
+		if (cpumask_test_cpu(cpu, tpolicy->related_cpus)) {
 			read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-			return cpufreq_add_policy_cpu(cp, cpu, dev, frozen);
+			return cpufreq_add_policy_cpu(tpolicy, cpu, dev,
+					frozen);
 		}
 	}
 	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 08/11] cpufreq: Fix broken usage of governor->owner's refcount
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (6 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 09/11] cpufreq: Don't use cpufreq_driver->owner's refcount to protect critical sections Viresh Kumar
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

Governor's owner refcount usage was broken. We should increment refcount only
when CPUFREQ_GOV_POLICY_INIT event has come and should decrement only if
CPUFREQ_GOV_POLICY_EXIT has come.

Currently there can be situations where governor is in use but we have allowed
it to be unloaded which may result in undefined behavior.
Lets fix it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index fe04b79..62eddb6 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1708,8 +1708,9 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
 		}
 	}
 
-	if (!try_module_get(policy->governor->owner))
-		return -EINVAL;
+	if (event == CPUFREQ_GOV_POLICY_INIT)
+		if (!try_module_get(policy->governor->owner))
+			return -EINVAL;
 
 	pr_debug("__cpufreq_governor for CPU %u, event %u\n",
 						policy->cpu, event);
@@ -1718,6 +1719,8 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
 	if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
 	    (policy->governor_enabled && (event == CPUFREQ_GOV_START))) {
 		mutex_unlock(&cpufreq_governor_lock);
+		if (event == CPUFREQ_GOV_POLICY_INIT)
+			module_put(policy->governor->owner);
 		return -EBUSY;
 	}
 
@@ -1745,11 +1748,8 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
 		mutex_unlock(&cpufreq_governor_lock);
 	}
 
-	/* we keep one module reference alive for
-			each CPU governed by this CPU */
-	if ((event != CPUFREQ_GOV_START) || ret)
-		module_put(policy->governor->owner);
-	if ((event == CPUFREQ_GOV_STOP) && !ret)
+	if (((event == CPUFREQ_GOV_POLICY_INIT) && ret) ||
+			((event == CPUFREQ_GOV_POLICY_EXIT) && !ret))
 		module_put(policy->governor->owner);
 
 	return ret;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 09/11] cpufreq: Don't use cpufreq_driver->owner's refcount to protect critical sections
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (7 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 08/11] cpufreq: Fix broken usage of governor->owner's refcount Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 10/11] cpufreq: Remove struct cpufreq_driver's owner field Viresh Kumar
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

We are protecting critical sections of cpufreq core with the help of owner's
refcount, which isn't the correct approach. As rmmod returns error when some
routine has updated the module's refcount.

Lets use rwsem for this.. Only cpufreq_unregister_driver() will use write sem
and everybody else will use read sem.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 135 ++++++++++++++++++++--------------------------
 1 file changed, 57 insertions(+), 78 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 62eddb6..ccaf025 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -91,6 +91,12 @@ static void unlock_policy_rwsem_##mode(int cpu)				\
 unlock_policy_rwsem(read, cpu);
 unlock_policy_rwsem(write, cpu);
 
+/*
+ * rwsem to guarantee that cpufreq driver module doesn't unload during critical
+ * sections
+ */
+static DECLARE_RWSEM(cpufreq_rwsem);
+
 /* internal prototypes */
 static int __cpufreq_governor(struct cpufreq_policy *policy,
 		unsigned int event);
@@ -178,78 +184,46 @@ u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy)
 }
 EXPORT_SYMBOL_GPL(get_cpu_idle_time);
 
-static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs)
+struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
 {
-	struct cpufreq_policy *policy;
+	struct cpufreq_policy *policy = NULL;
 	unsigned long flags;
 
-	if (cpu >= nr_cpu_ids)
-		goto err_out;
+	if (cpufreq_disabled() || (cpu >= nr_cpu_ids))
+		return NULL;
+
+	if (!down_read_trylock(&cpufreq_rwsem))
+		return NULL;
 
 	/* get the cpufreq driver */
 	read_lock_irqsave(&cpufreq_driver_lock, flags);
 
-	if (!cpufreq_driver)
-		goto err_out_unlock;
-
-	if (!try_module_get(cpufreq_driver->owner))
-		goto err_out_unlock;
+	if (cpufreq_driver) {
+		/* get the CPU */
+		policy = per_cpu(cpufreq_cpu_data, cpu);
+		if (policy)
+			kobject_get(&policy->kobj);
+	}
 
-	/* get the CPU */
-	policy = per_cpu(cpufreq_cpu_data, cpu);
+	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
 	if (!policy)
-		goto err_out_put_module;
-
-	if (!sysfs && !kobject_get(&policy->kobj))
-		goto err_out_put_module;
+		up_read(&cpufreq_rwsem);
 
-	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
 	return policy;
-
-err_out_put_module:
-	module_put(cpufreq_driver->owner);
-err_out_unlock:
-	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-err_out:
-	return NULL;
-}
-
-struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
-{
-	if (cpufreq_disabled())
-		return NULL;
-
-	return __cpufreq_cpu_get(cpu, false);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cpu_get);
 
-static struct cpufreq_policy *cpufreq_cpu_get_sysfs(unsigned int cpu)
-{
-	return __cpufreq_cpu_get(cpu, true);
-}
-
-static void __cpufreq_cpu_put(struct cpufreq_policy *policy, bool sysfs)
-{
-	if (!sysfs)
-		kobject_put(&policy->kobj);
-	module_put(cpufreq_driver->owner);
-}
-
 void cpufreq_cpu_put(struct cpufreq_policy *policy)
 {
 	if (cpufreq_disabled())
 		return;
 
-	__cpufreq_cpu_put(policy, false);
+	kobject_put(&policy->kobj);
+	up_read(&cpufreq_rwsem);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cpu_put);
 
-static void cpufreq_cpu_put_sysfs(struct cpufreq_policy *policy)
-{
-	__cpufreq_cpu_put(policy, true);
-}
-
 /*********************************************************************
  *            EXTERNALLY AFFECTING FREQUENCY CHANGES                 *
  *********************************************************************/
@@ -694,12 +668,12 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
 	struct cpufreq_policy *policy = to_policy(kobj);
 	struct freq_attr *fattr = to_attr(attr);
 	ssize_t ret = -EINVAL;
-	policy = cpufreq_cpu_get_sysfs(policy->cpu);
-	if (!policy)
-		goto no_policy;
+
+	if (!down_read_trylock(&cpufreq_rwsem))
+		goto exit;
 
 	if (lock_policy_rwsem_read(policy->cpu) < 0)
-		goto fail;
+		goto up_read;
 
 	if (fattr->show)
 		ret = fattr->show(policy, buf);
@@ -707,9 +681,10 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
 		ret = -EIO;
 
 	unlock_policy_rwsem_read(policy->cpu);
-fail:
-	cpufreq_cpu_put_sysfs(policy);
-no_policy:
+
+up_read:
+	up_read(&cpufreq_rwsem);
+exit:
 	return ret;
 }
 
@@ -719,12 +694,12 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
 	struct cpufreq_policy *policy = to_policy(kobj);
 	struct freq_attr *fattr = to_attr(attr);
 	ssize_t ret = -EINVAL;
-	policy = cpufreq_cpu_get_sysfs(policy->cpu);
-	if (!policy)
-		goto no_policy;
+
+	if (!down_read_trylock(&cpufreq_rwsem))
+		goto exit;
 
 	if (lock_policy_rwsem_write(policy->cpu) < 0)
-		goto fail;
+		goto up_read;
 
 	if (fattr->store)
 		ret = fattr->store(policy, buf, count);
@@ -732,9 +707,10 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
 		ret = -EIO;
 
 	unlock_policy_rwsem_write(policy->cpu);
-fail:
-	cpufreq_cpu_put_sysfs(policy);
-no_policy:
+
+up_read:
+	up_read(&cpufreq_rwsem);
+exit:
 	return ret;
 }
 
@@ -1002,25 +978,24 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 		return 0;
 	}
 
+	if (!down_read_trylock(&cpufreq_rwsem))
+		return 0;
+
 #ifdef CONFIG_HOTPLUG_CPU
 	/* Check if this cpu was hot-unplugged earlier and has siblings */
 	read_lock_irqsave(&cpufreq_driver_lock, flags);
 	list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) {
 		if (cpumask_test_cpu(cpu, tpolicy->related_cpus)) {
 			read_unlock_irqrestore(&cpufreq_driver_lock, flags);
-			return cpufreq_add_policy_cpu(tpolicy, cpu, dev,
-					frozen);
+			ret = cpufreq_add_policy_cpu(tpolicy, cpu, dev, frozen);
+			up_read(&cpufreq_rwsem);
+			return ret;
 		}
 	}
 	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
 #endif
 #endif
 
-	if (!try_module_get(cpufreq_driver->owner)) {
-		ret = -EINVAL;
-		goto module_out;
-	}
-
 	if (frozen)
 		/* Restore the saved policy when doing light-weight init */
 		policy = cpufreq_policy_restore(cpu);
@@ -1093,7 +1068,8 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
 	cpufreq_init_policy(policy);
 
 	kobject_uevent(&policy->kobj, KOBJ_ADD);
-	module_put(cpufreq_driver->owner);
+	up_read(&cpufreq_rwsem);
+
 	pr_debug("initialization complete\n");
 
 	return 0;
@@ -1111,8 +1087,8 @@ err_set_policy_cpu:
 	per_cpu(cpufreq_policy_cpu, cpu) = -1;
 	cpufreq_policy_free(policy);
 nomem_out:
-	module_put(cpufreq_driver->owner);
-module_out:
+	up_read(&cpufreq_rwsem);
+
 	return ret;
 }
 
@@ -1424,10 +1400,9 @@ static unsigned int __cpufreq_get(unsigned int cpu)
 unsigned int cpufreq_get(unsigned int cpu)
 {
 	unsigned int ret_freq = 0;
-	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
 
-	if (!policy)
-		goto out;
+	if (!down_read_trylock(&cpufreq_rwsem))
+		return 0;
 
 	if (unlikely(lock_policy_rwsem_read(cpu)))
 		goto out_policy;
@@ -1437,8 +1412,8 @@ unsigned int cpufreq_get(unsigned int cpu)
 	unlock_policy_rwsem_read(cpu);
 
 out_policy:
-	cpufreq_cpu_put(policy);
-out:
+	up_read(&cpufreq_rwsem);
+
 	return ret_freq;
 }
 EXPORT_SYMBOL(cpufreq_get);
@@ -2131,9 +2106,13 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver)
 	subsys_interface_unregister(&cpufreq_interface);
 	unregister_hotcpu_notifier(&cpufreq_cpu_notifier);
 
+	down_write(&cpufreq_rwsem);
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
+
 	cpufreq_driver = NULL;
+
 	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
+	up_write(&cpufreq_rwsem);
 
 	return 0;
 }
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 10/11] cpufreq: Remove struct cpufreq_driver's owner field
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (8 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 09/11] cpufreq: Don't use cpufreq_driver->owner's refcount to protect critical sections Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-06 17:23 ` [PATCH V2 11/11] cpufreq: improve error checking on return values of __cpufreq_governor() Viresh Kumar
  2013-08-07  0:21 ` [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Rafael J. Wysocki
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

We don't need to set .owner = THIS_MODULE anymore in our code as this field
isn't used anymore by cpufreq core.

This patch removes it and fixes all dependent drivers.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/cpu-freq/cpu-drivers.txt | 2 --
 drivers/cpufreq/acpi-cpufreq.c         | 1 -
 drivers/cpufreq/at32ap-cpufreq.c       | 1 -
 drivers/cpufreq/blackfin-cpufreq.c     | 1 -
 drivers/cpufreq/cpufreq-nforce2.c      | 1 -
 drivers/cpufreq/cris-artpec3-cpufreq.c | 1 -
 drivers/cpufreq/cris-etraxfs-cpufreq.c | 1 -
 drivers/cpufreq/e_powersaver.c         | 1 -
 drivers/cpufreq/elanfreq.c             | 1 -
 drivers/cpufreq/gx-suspmod.c           | 1 -
 drivers/cpufreq/ia64-acpi-cpufreq.c    | 1 -
 drivers/cpufreq/intel_pstate.c         | 1 -
 drivers/cpufreq/kirkwood-cpufreq.c     | 1 -
 drivers/cpufreq/longhaul.c             | 1 -
 drivers/cpufreq/longrun.c              | 1 -
 drivers/cpufreq/loongson2_cpufreq.c    | 1 -
 drivers/cpufreq/maple-cpufreq.c        | 1 -
 drivers/cpufreq/p4-clockmod.c          | 1 -
 drivers/cpufreq/pasemi-cpufreq.c       | 1 -
 drivers/cpufreq/pcc-cpufreq.c          | 1 -
 drivers/cpufreq/pmac32-cpufreq.c       | 1 -
 drivers/cpufreq/pmac64-cpufreq.c       | 1 -
 drivers/cpufreq/powernow-k6.c          | 1 -
 drivers/cpufreq/powernow-k7.c          | 1 -
 drivers/cpufreq/powernow-k8.c          | 1 -
 drivers/cpufreq/ppc-corenet-cpufreq.c  | 1 -
 drivers/cpufreq/ppc_cbe_cpufreq.c      | 1 -
 drivers/cpufreq/s3c2416-cpufreq.c      | 1 -
 drivers/cpufreq/s3c64xx-cpufreq.c      | 1 -
 drivers/cpufreq/sc520_freq.c           | 1 -
 drivers/cpufreq/sh-cpufreq.c           | 1 -
 drivers/cpufreq/sparc-us2e-cpufreq.c   | 1 -
 drivers/cpufreq/sparc-us3-cpufreq.c    | 1 -
 drivers/cpufreq/speedstep-centrino.c   | 1 -
 drivers/cpufreq/speedstep-ich.c        | 1 -
 drivers/cpufreq/speedstep-smi.c        | 1 -
 include/linux/cpufreq.h                | 1 -
 37 files changed, 38 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 19fa98e..40282e6 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -50,8 +50,6 @@ What shall this struct cpufreq_driver contain?
 
 cpufreq_driver.name -		The name of this driver.
 
-cpufreq_driver.owner -		THIS_MODULE;
-
 cpufreq_driver.init -		A pointer to the per-CPU initialization 
 				function.
 
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 44758ce..9b5d1b1 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -936,7 +936,6 @@ static struct cpufreq_driver acpi_cpufreq_driver = {
 	.exit		= acpi_cpufreq_cpu_exit,
 	.resume		= acpi_cpufreq_resume,
 	.name		= "acpi-cpufreq",
-	.owner		= THIS_MODULE,
 	.attr		= acpi_cpufreq_attr,
 };
 
diff --git a/drivers/cpufreq/at32ap-cpufreq.c b/drivers/cpufreq/at32ap-cpufreq.c
index 6544887..e0c38d9 100644
--- a/drivers/cpufreq/at32ap-cpufreq.c
+++ b/drivers/cpufreq/at32ap-cpufreq.c
@@ -108,7 +108,6 @@ static int __init at32_cpufreq_driver_init(struct cpufreq_policy *policy)
 
 static struct cpufreq_driver at32_driver = {
 	.name		= "at32ap",
-	.owner		= THIS_MODULE,
 	.init		= at32_cpufreq_driver_init,
 	.verify		= at32_verify_speed,
 	.target		= at32_set_target,
diff --git a/drivers/cpufreq/blackfin-cpufreq.c b/drivers/cpufreq/blackfin-cpufreq.c
index 9cdbbd2..ef05978 100644
--- a/drivers/cpufreq/blackfin-cpufreq.c
+++ b/drivers/cpufreq/blackfin-cpufreq.c
@@ -225,7 +225,6 @@ static struct cpufreq_driver bfin_driver = {
 	.get = bfin_getfreq_khz,
 	.init = __bfin_cpu_init,
 	.name = "bfin cpufreq",
-	.owner = THIS_MODULE,
 	.attr = bfin_freq_attr,
 };
 
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index af1542d..b83d45f 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -379,7 +379,6 @@ static struct cpufreq_driver nforce2_driver = {
 	.get = nforce2_get,
 	.init = nforce2_cpu_init,
 	.exit = nforce2_cpu_exit,
-	.owner = THIS_MODULE,
 };
 
 #ifdef MODULE
diff --git a/drivers/cpufreq/cris-artpec3-cpufreq.c b/drivers/cpufreq/cris-artpec3-cpufreq.c
index ee142c4..cb8276d 100644
--- a/drivers/cpufreq/cris-artpec3-cpufreq.c
+++ b/drivers/cpufreq/cris-artpec3-cpufreq.c
@@ -111,7 +111,6 @@ static struct cpufreq_driver cris_freq_driver = {
 	.init	= cris_freq_cpu_init,
 	.exit	= cris_freq_cpu_exit,
 	.name	= "cris_freq",
-	.owner	= THIS_MODULE,
 	.attr	= cris_freq_attr,
 };
 
diff --git a/drivers/cpufreq/cris-etraxfs-cpufreq.c b/drivers/cpufreq/cris-etraxfs-cpufreq.c
index 1295223..72328f7 100644
--- a/drivers/cpufreq/cris-etraxfs-cpufreq.c
+++ b/drivers/cpufreq/cris-etraxfs-cpufreq.c
@@ -108,7 +108,6 @@ static struct cpufreq_driver cris_freq_driver = {
 	.init = cris_freq_cpu_init,
 	.exit = cris_freq_cpu_exit,
 	.name = "cris_freq",
-	.owner = THIS_MODULE,
 	.attr = cris_freq_attr,
 };
 
diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c
index de974be..09f64cc 100644
--- a/drivers/cpufreq/e_powersaver.c
+++ b/drivers/cpufreq/e_powersaver.c
@@ -436,7 +436,6 @@ static struct cpufreq_driver eps_driver = {
 	.exit		= eps_cpu_exit,
 	.get		= eps_get,
 	.name		= "e_powersaver",
-	.owner		= THIS_MODULE,
 	.attr		= eps_attr,
 };
 
diff --git a/drivers/cpufreq/elanfreq.c b/drivers/cpufreq/elanfreq.c
index 658d860..823a400 100644
--- a/drivers/cpufreq/elanfreq.c
+++ b/drivers/cpufreq/elanfreq.c
@@ -274,7 +274,6 @@ static struct cpufreq_driver elanfreq_driver = {
 	.init		= elanfreq_cpu_init,
 	.exit		= elanfreq_cpu_exit,
 	.name		= "elanfreq",
-	.owner		= THIS_MODULE,
 	.attr		= elanfreq_attr,
 };
 
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c
index 4f25fb6..ef5fee7 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -446,7 +446,6 @@ static struct cpufreq_driver gx_suspmod_driver = {
 	.target		= cpufreq_gx_target,
 	.init		= cpufreq_gx_cpu_init,
 	.name		= "gx-suspmod",
-	.owner		= THIS_MODULE,
 };
 
 static int __init cpufreq_gx_init(void)
diff --git a/drivers/cpufreq/ia64-acpi-cpufreq.c b/drivers/cpufreq/ia64-acpi-cpufreq.c
index 08792dd..3e14f03 100644
--- a/drivers/cpufreq/ia64-acpi-cpufreq.c
+++ b/drivers/cpufreq/ia64-acpi-cpufreq.c
@@ -409,7 +409,6 @@ static struct cpufreq_driver acpi_cpufreq_driver = {
 	.init		= acpi_cpufreq_cpu_init,
 	.exit		= acpi_cpufreq_cpu_exit,
 	.name		= "acpi-cpufreq",
-	.owner		= THIS_MODULE,
 	.attr           = acpi_cpufreq_attr,
 };
 
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 7cde885..6efd96c 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -665,7 +665,6 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.init		= intel_pstate_cpu_init,
 	.exit		= intel_pstate_cpu_exit,
 	.name		= "intel_pstate",
-	.owner		= THIS_MODULE,
 };
 
 static int __initdata no_load;
diff --git a/drivers/cpufreq/kirkwood-cpufreq.c b/drivers/cpufreq/kirkwood-cpufreq.c
index c233ea6..45e4d7f 100644
--- a/drivers/cpufreq/kirkwood-cpufreq.c
+++ b/drivers/cpufreq/kirkwood-cpufreq.c
@@ -158,7 +158,6 @@ static struct cpufreq_driver kirkwood_cpufreq_driver = {
 	.init	= kirkwood_cpufreq_cpu_init,
 	.exit	= kirkwood_cpufreq_cpu_exit,
 	.name	= "kirkwood-cpufreq",
-	.owner	= THIS_MODULE,
 	.attr	= kirkwood_cpufreq_attr,
 };
 
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
index 8c49261..4ada1cc 100644
--- a/drivers/cpufreq/longhaul.c
+++ b/drivers/cpufreq/longhaul.c
@@ -948,7 +948,6 @@ static struct cpufreq_driver longhaul_driver = {
 	.init	= longhaul_cpu_init,
 	.exit	= longhaul_cpu_exit,
 	.name	= "longhaul",
-	.owner	= THIS_MODULE,
 	.attr	= longhaul_attr,
 };
 
diff --git a/drivers/cpufreq/longrun.c b/drivers/cpufreq/longrun.c
index 0fe041d..5aa0316 100644
--- a/drivers/cpufreq/longrun.c
+++ b/drivers/cpufreq/longrun.c
@@ -286,7 +286,6 @@ static struct cpufreq_driver longrun_driver = {
 	.get		= longrun_get,
 	.init		= longrun_cpu_init,
 	.name		= "longrun",
-	.owner		= THIS_MODULE,
 };
 
 static const struct x86_cpu_id longrun_ids[] = {
diff --git a/drivers/cpufreq/loongson2_cpufreq.c b/drivers/cpufreq/loongson2_cpufreq.c
index bb838b9..e65de91 100644
--- a/drivers/cpufreq/loongson2_cpufreq.c
+++ b/drivers/cpufreq/loongson2_cpufreq.c
@@ -157,7 +157,6 @@ static struct freq_attr *loongson2_table_attr[] = {
 };
 
 static struct cpufreq_driver loongson2_cpufreq_driver = {
-	.owner = THIS_MODULE,
 	.name = "loongson2",
 	.init = loongson2_cpufreq_cpu_init,
 	.verify = loongson2_cpufreq_verify,
diff --git a/drivers/cpufreq/maple-cpufreq.c b/drivers/cpufreq/maple-cpufreq.c
index cdd6291..41c601f 100644
--- a/drivers/cpufreq/maple-cpufreq.c
+++ b/drivers/cpufreq/maple-cpufreq.c
@@ -190,7 +190,6 @@ static int maple_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 static struct cpufreq_driver maple_cpufreq_driver = {
 	.name		= "maple",
-	.owner		= THIS_MODULE,
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.init		= maple_cpufreq_cpu_init,
 	.verify		= maple_cpufreq_verify,
diff --git a/drivers/cpufreq/p4-clockmod.c b/drivers/cpufreq/p4-clockmod.c
index 9ee7817..2f0a2a6 100644
--- a/drivers/cpufreq/p4-clockmod.c
+++ b/drivers/cpufreq/p4-clockmod.c
@@ -279,7 +279,6 @@ static struct cpufreq_driver p4clockmod_driver = {
 	.exit		= cpufreq_p4_cpu_exit,
 	.get		= cpufreq_p4_get,
 	.name		= "p4-clockmod",
-	.owner		= THIS_MODULE,
 	.attr		= p4clockmod_attr,
 };
 
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index b704da4..534e43a 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -297,7 +297,6 @@ static int pas_cpufreq_target(struct cpufreq_policy *policy,
 
 static struct cpufreq_driver pas_cpufreq_driver = {
 	.name		= "pas-cpufreq",
-	.owner		= THIS_MODULE,
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.init		= pas_cpufreq_cpu_init,
 	.exit		= pas_cpufreq_cpu_exit,
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index 1581fcc4..d81c4e5 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -587,7 +587,6 @@ static struct cpufreq_driver pcc_cpufreq_driver = {
 	.init = pcc_cpufreq_cpu_init,
 	.exit = pcc_cpufreq_cpu_exit,
 	.name = "pcc-cpufreq",
-	.owner = THIS_MODULE,
 };
 
 static int __init pcc_cpufreq_init(void)
diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c
index 3104fad..38cdc63 100644
--- a/drivers/cpufreq/pmac32-cpufreq.c
+++ b/drivers/cpufreq/pmac32-cpufreq.c
@@ -477,7 +477,6 @@ static struct cpufreq_driver pmac_cpufreq_driver = {
 	.flags		= CPUFREQ_PM_NO_WARN,
 	.attr		= pmac_cpu_freqs_attr,
 	.name		= "powermac",
-	.owner		= THIS_MODULE,
 };
 
 
diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
index 4d7799b..b6850d9 100644
--- a/drivers/cpufreq/pmac64-cpufreq.c
+++ b/drivers/cpufreq/pmac64-cpufreq.c
@@ -371,7 +371,6 @@ static int g5_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 static struct cpufreq_driver g5_cpufreq_driver = {
 	.name		= "powermac",
-	.owner		= THIS_MODULE,
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.init		= g5_cpufreq_cpu_init,
 	.verify		= g5_cpufreq_verify,
diff --git a/drivers/cpufreq/powernow-k6.c b/drivers/cpufreq/powernow-k6.c
index ea8e103..85f1c8c 100644
--- a/drivers/cpufreq/powernow-k6.c
+++ b/drivers/cpufreq/powernow-k6.c
@@ -207,7 +207,6 @@ static struct cpufreq_driver powernow_k6_driver = {
 	.exit		= powernow_k6_cpu_exit,
 	.get		= powernow_k6_get,
 	.name		= "powernow-k6",
-	.owner		= THIS_MODULE,
 	.attr		= powernow_k6_attr,
 };
 
diff --git a/drivers/cpufreq/powernow-k7.c b/drivers/cpufreq/powernow-k7.c
index 4687d40..14ce480 100644
--- a/drivers/cpufreq/powernow-k7.c
+++ b/drivers/cpufreq/powernow-k7.c
@@ -716,7 +716,6 @@ static struct cpufreq_driver powernow_driver = {
 	.init		= powernow_cpu_init,
 	.exit		= powernow_cpu_exit,
 	.name		= "powernow-k7",
-	.owner		= THIS_MODULE,
 	.attr		= powernow_table_attr,
 };
 
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 3f3c6ca..2344a9e 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1240,7 +1240,6 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
 	.exit		= powernowk8_cpu_exit,
 	.get		= powernowk8_get,
 	.name		= "powernow-k8",
-	.owner		= THIS_MODULE,
 	.attr		= powernow_k8_attr,
 };
 
diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
index 3cae452..60e81d5 100644
--- a/drivers/cpufreq/ppc-corenet-cpufreq.c
+++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
@@ -300,7 +300,6 @@ static struct freq_attr *corenet_cpufreq_attr[] = {
 
 static struct cpufreq_driver ppc_corenet_cpufreq_driver = {
 	.name		= "ppc_cpufreq",
-	.owner		= THIS_MODULE,
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.init		= corenet_cpufreq_cpu_init,
 	.exit		= __exit_p(corenet_cpufreq_cpu_exit),
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c
index 5936f8d..2e448f0 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
@@ -181,7 +181,6 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
 	.init		= cbe_cpufreq_cpu_init,
 	.exit		= cbe_cpufreq_cpu_exit,
 	.name		= "cbe-cpufreq",
-	.owner		= THIS_MODULE,
 	.flags		= CPUFREQ_CONST_LOOPS,
 };
 
diff --git a/drivers/cpufreq/s3c2416-cpufreq.c b/drivers/cpufreq/s3c2416-cpufreq.c
index ce5b9fc..22dcb81 100644
--- a/drivers/cpufreq/s3c2416-cpufreq.c
+++ b/drivers/cpufreq/s3c2416-cpufreq.c
@@ -524,7 +524,6 @@ static struct freq_attr *s3c2416_cpufreq_attr[] = {
 };
 
 static struct cpufreq_driver s3c2416_cpufreq_driver = {
-	.owner		= THIS_MODULE,
 	.flags          = 0,
 	.verify		= s3c2416_cpufreq_verify_speed,
 	.target		= s3c2416_cpufreq_set_target,
diff --git a/drivers/cpufreq/s3c64xx-cpufreq.c b/drivers/cpufreq/s3c64xx-cpufreq.c
index 13bb4ba..8a72b0c 100644
--- a/drivers/cpufreq/s3c64xx-cpufreq.c
+++ b/drivers/cpufreq/s3c64xx-cpufreq.c
@@ -263,7 +263,6 @@ static int s3c64xx_cpufreq_driver_init(struct cpufreq_policy *policy)
 }
 
 static struct cpufreq_driver s3c64xx_cpufreq_driver = {
-	.owner		= THIS_MODULE,
 	.flags          = 0,
 	.verify		= s3c64xx_cpufreq_verify_speed,
 	.target		= s3c64xx_cpufreq_set_target,
diff --git a/drivers/cpufreq/sc520_freq.c b/drivers/cpufreq/sc520_freq.c
index 77a2109..d6f6c6f 100644
--- a/drivers/cpufreq/sc520_freq.c
+++ b/drivers/cpufreq/sc520_freq.c
@@ -147,7 +147,6 @@ static struct cpufreq_driver sc520_freq_driver = {
 	.init	= sc520_freq_cpu_init,
 	.exit	= sc520_freq_cpu_exit,
 	.name	= "sc520_freq",
-	.owner	= THIS_MODULE,
 	.attr	= sc520_freq_attr,
 };
 
diff --git a/drivers/cpufreq/sh-cpufreq.c b/drivers/cpufreq/sh-cpufreq.c
index 73adb64..ffc6d24 100644
--- a/drivers/cpufreq/sh-cpufreq.c
+++ b/drivers/cpufreq/sh-cpufreq.c
@@ -160,7 +160,6 @@ static struct freq_attr *sh_freq_attr[] = {
 };
 
 static struct cpufreq_driver sh_cpufreq_driver = {
-	.owner		= THIS_MODULE,
 	.name		= "sh",
 	.get		= sh_cpufreq_get,
 	.target		= sh_cpufreq_target,
diff --git a/drivers/cpufreq/sparc-us2e-cpufreq.c b/drivers/cpufreq/sparc-us2e-cpufreq.c
index 7c43a72..cf5bc2c 100644
--- a/drivers/cpufreq/sparc-us2e-cpufreq.c
+++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
@@ -365,7 +365,6 @@ static int __init us2e_freq_init(void)
 		driver->target = us2e_freq_target;
 		driver->get = us2e_freq_get;
 		driver->exit = us2e_freq_cpu_exit;
-		driver->owner = THIS_MODULE,
 		strcpy(driver->name, "UltraSPARC-IIe");
 
 		cpufreq_us2e_driver = driver;
diff --git a/drivers/cpufreq/sparc-us3-cpufreq.c b/drivers/cpufreq/sparc-us3-cpufreq.c
index 7f500c1..ac76b48 100644
--- a/drivers/cpufreq/sparc-us3-cpufreq.c
+++ b/drivers/cpufreq/sparc-us3-cpufreq.c
@@ -226,7 +226,6 @@ static int __init us3_freq_init(void)
 		driver->target = us3_freq_target;
 		driver->get = us3_freq_get;
 		driver->exit = us3_freq_cpu_exit;
-		driver->owner = THIS_MODULE,
 		strcpy(driver->name, "UltraSPARC-III");
 
 		cpufreq_us3_driver = driver;
diff --git a/drivers/cpufreq/speedstep-centrino.c b/drivers/cpufreq/speedstep-centrino.c
index 0915e71..f897d51 100644
--- a/drivers/cpufreq/speedstep-centrino.c
+++ b/drivers/cpufreq/speedstep-centrino.c
@@ -575,7 +575,6 @@ static struct cpufreq_driver centrino_driver = {
 	.target		= centrino_target,
 	.get		= get_cur_freq,
 	.attr           = centrino_attr,
-	.owner		= THIS_MODULE,
 };
 
 /*
diff --git a/drivers/cpufreq/speedstep-ich.c b/drivers/cpufreq/speedstep-ich.c
index e2e5aa9..5355abb 100644
--- a/drivers/cpufreq/speedstep-ich.c
+++ b/drivers/cpufreq/speedstep-ich.c
@@ -378,7 +378,6 @@ static struct cpufreq_driver speedstep_driver = {
 	.init	= speedstep_cpu_init,
 	.exit	= speedstep_cpu_exit,
 	.get	= speedstep_get,
-	.owner	= THIS_MODULE,
 	.attr	= speedstep_attr,
 };
 
diff --git a/drivers/cpufreq/speedstep-smi.c b/drivers/cpufreq/speedstep-smi.c
index f5a6b70..abfba4f 100644
--- a/drivers/cpufreq/speedstep-smi.c
+++ b/drivers/cpufreq/speedstep-smi.c
@@ -375,7 +375,6 @@ static struct cpufreq_driver speedstep_driver = {
 	.exit		= speedstep_cpu_exit,
 	.get		= speedstep_get,
 	.resume		= speedstep_resume,
-	.owner		= THIS_MODULE,
 	.attr		= speedstep_attr,
 };
 
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 431a05d..d568f39 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -179,7 +179,6 @@ __ATTR(_name, 0644, show_##_name, store_##_name)
 
 
 struct cpufreq_driver {
-	struct module		*owner;
 	char			name[CPUFREQ_NAME_LEN];
 	u8			flags;
 	/*
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V2 11/11] cpufreq: improve error checking on return values of __cpufreq_governor()
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (9 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 10/11] cpufreq: Remove struct cpufreq_driver's owner field Viresh Kumar
@ 2013-08-06 17:23 ` Viresh Kumar
  2013-08-07  0:21 ` [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Rafael J. Wysocki
  11 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-06 17:23 UTC (permalink / raw)
  To: rjw
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel,
	srivatsa.bhat, Viresh Kumar

__cpufreq_governor() function can fail in rare cases specially if there are bugs
in cpufreq driver. And so we must stop processing as soon as this routine fails,
otherwise it may result in undefined behavior.

And so this patch adds error checking code whenever this routine is called from
any place.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 48 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index ccaf025..06f8671 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -874,8 +874,13 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
 	int ret = 0, has_target = !!cpufreq_driver->target;
 	unsigned long flags;
 
-	if (has_target)
-		__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+	if (has_target) {
+		ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+		if (ret) {
+			pr_err("%s: Failed to stop governor\n", __func__);
+			return ret;
+		}
+	}
 
 	lock_policy_rwsem_write(policy->cpu);
 
@@ -889,8 +894,11 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy,
 	unlock_policy_rwsem_write(policy->cpu);
 
 	if (has_target) {
-		__cpufreq_governor(policy, CPUFREQ_GOV_START);
-		__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+		if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
+			(ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) {
+			pr_err("%s: Failed to start governor\n", __func__);
+			return ret;
+		}
 	}
 
 	/* Don't touch sysfs links during light-weight init */
@@ -1171,7 +1179,7 @@ static int __cpufreq_remove_dev(struct device *dev,
 				struct subsys_interface *sif, bool frozen)
 {
 	unsigned int cpu = dev->id, cpus;
-	int new_cpu;
+	int new_cpu, ret;
 	unsigned long flags;
 	struct cpufreq_policy *policy;
 	struct kobject *kobj;
@@ -1195,8 +1203,13 @@ static int __cpufreq_remove_dev(struct device *dev,
 		return -EINVAL;
 	}
 
-	if (cpufreq_driver->target)
-		__cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+	if (cpufreq_driver->target) {
+		ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+		if (ret) {
+			pr_err("%s: Failed to stop governor\n", __func__);
+			return ret;
+		}
+	}
 
 #ifdef CONFIG_HOTPLUG_CPU
 	if (!cpufreq_driver->setpolicy)
@@ -1230,8 +1243,15 @@ static int __cpufreq_remove_dev(struct device *dev,
 
 	/* If cpu is last user of policy, free policy */
 	if (cpus == 1) {
-		if (cpufreq_driver->target)
-			__cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
+		if (cpufreq_driver->target) {
+			ret = __cpufreq_governor(policy,
+					CPUFREQ_GOV_POLICY_EXIT);
+			if (ret) {
+				pr_err("%s: Failed to exit governor\n",
+						__func__);
+				return ret;
+			}
+	}
 
 		if (!frozen) {
 			lock_policy_rwsem_read(cpu);
@@ -1262,8 +1282,12 @@ static int __cpufreq_remove_dev(struct device *dev,
 			cpufreq_policy_free(policy);
 	} else {
 		if (cpufreq_driver->target) {
-			__cpufreq_governor(policy, CPUFREQ_GOV_START);
-			__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+			if ((ret = __cpufreq_governor(policy, CPUFREQ_GOV_START)) ||
+					(ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))) {
+				pr_err("%s: Failed to start governor\n",
+						__func__);
+				return ret;
+			}
 		}
 	}
 
@@ -1911,7 +1935,7 @@ static int __cpufreq_set_policy(struct cpufreq_policy *policy,
 			/* might be a policy change, too, so fall through */
 		}
 		pr_debug("governor: change or update limits\n");
-		__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+		ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
 	}
 
 error_out:
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12
  2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
                   ` (10 preceding siblings ...)
  2013-08-06 17:23 ` [PATCH V2 11/11] cpufreq: improve error checking on return values of __cpufreq_governor() Viresh Kumar
@ 2013-08-07  0:21 ` Rafael J. Wysocki
  11 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2013-08-07  0:21 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel, srivatsa.bhat

On Tuesday, August 06, 2013 10:53:02 PM Viresh Kumar wrote:
> Hi Rafael,
> 
> This is V2 of the patch series which was posted here earlier:
> 
> http://www.gossamer-threads.com/lists/linux/kernel/1759514
> 
> This patchset tries to fix & cleanup many existing cpufreq core issues. First
> four patches tries to cleanup basic problems in cpufreq core. Its first patch
> was earlier sent separately but now is part of this series.
> 
> Fifth patch was also sent earlier as reply to your patches and was reviewed by
> Srivatsa. Sixth patch was picked from Lukasz's patchset on introducing software
> "boost" feature in core. It will be used by this patchset.
> 
> And 7-10 are are the most significant part of this set. They try to make many
> things simple and robust.
> 
> Last patch in the series is new and wasn't part of V1.
> 
> This is rebased of your bleeding-edge branch + two patches from you:
> 18a6b03 cpufreq: Avoid double kobject_put() for the same kobject in error code path
> d0cde63 cpufreq: Do not hold driver module references for additional policy CPUs
> abe513f Merge branch 'acpi-sleep-next' into linux-next
> 
> They are also pushed in my cpufreq-next branch.
> 
> They are tested fairly well on ARM Vexpress TC2 board (big LITTLE).
> 
> Lukasz Majewski (1):
>   cpufreq: Store cpufreq policies in a list
> 
> Viresh Kumar (10):
>   cpufreq: Cleanup header files included in core
>   cpufreq: Re-arrange declarations in cpufreq.h
>   cpufreq: Give consistent names for struct cpufreq_policy *
>   cpufreq: Use sizeof(*ptr) form for finding size of a struct
>   cpufreq: Pass policy to cpufreq_add_policy_cpu()
>   cpufreq: Use cpufreq_policy_list for iterating over policies
>   cpufreq: Fix broken usage of governor->owner's refcount
>   cpufreq: Don't use cpufreq_driver->owner's refcount to protect
>     critical sections
>   cpufreq: Remove struct cpufreq_driver's owner field
>   cpufreq: improve error checking on return values of
>     __cpufreq_governor()

Applied to bleeding-edge except for patch [05/11] that I've applied already
earlier.

>  Documentation/cpu-freq/cpu-drivers.txt |   2 -
>  drivers/cpufreq/acpi-cpufreq.c         |   5 +-
>  drivers/cpufreq/at32ap-cpufreq.c       |   1 -
>  drivers/cpufreq/blackfin-cpufreq.c     |   1 -
>  drivers/cpufreq/cpufreq-nforce2.c      |   1 -
>  drivers/cpufreq/cpufreq.c              | 426 ++++++++++++++++-----------------
>  drivers/cpufreq/cpufreq_conservative.c |  14 +-
>  drivers/cpufreq/cpufreq_governor.c     |   6 -
>  drivers/cpufreq/cpufreq_governor.h     |   7 +-
>  drivers/cpufreq/cpufreq_ondemand.c     |  24 +-
>  drivers/cpufreq/cpufreq_performance.c  |   3 +-
>  drivers/cpufreq/cpufreq_powersave.c    |   3 +-
>  drivers/cpufreq/cpufreq_stats.c        |  23 +-
>  drivers/cpufreq/cris-artpec3-cpufreq.c |   1 -
>  drivers/cpufreq/cris-etraxfs-cpufreq.c |   1 -
>  drivers/cpufreq/e_powersaver.c         |   5 +-
>  drivers/cpufreq/elanfreq.c             |   1 -
>  drivers/cpufreq/exynos-cpufreq.c       |   2 +-
>  drivers/cpufreq/freq_table.c           |   4 +-
>  drivers/cpufreq/gx-suspmod.c           |   3 +-
>  drivers/cpufreq/ia64-acpi-cpufreq.c    |   5 +-
>  drivers/cpufreq/intel_pstate.c         |   1 -
>  drivers/cpufreq/kirkwood-cpufreq.c     |   1 -
>  drivers/cpufreq/longhaul.c             |   1 -
>  drivers/cpufreq/longrun.c              |   1 -
>  drivers/cpufreq/loongson2_cpufreq.c    |   1 -
>  drivers/cpufreq/maple-cpufreq.c        |   1 -
>  drivers/cpufreq/p4-clockmod.c          |   1 -
>  drivers/cpufreq/pasemi-cpufreq.c       |   1 -
>  drivers/cpufreq/pcc-cpufreq.c          |   1 -
>  drivers/cpufreq/pmac32-cpufreq.c       |   1 -
>  drivers/cpufreq/pmac64-cpufreq.c       |   6 +-
>  drivers/cpufreq/powernow-k6.c          |   1 -
>  drivers/cpufreq/powernow-k7.c          |  14 +-
>  drivers/cpufreq/powernow-k8.c          |   7 +-
>  drivers/cpufreq/ppc-corenet-cpufreq.c  |   1 -
>  drivers/cpufreq/ppc_cbe_cpufreq.c      |   1 -
>  drivers/cpufreq/s3c2416-cpufreq.c      |   1 -
>  drivers/cpufreq/s3c24xx-cpufreq.c      |   6 +-
>  drivers/cpufreq/s3c64xx-cpufreq.c      |   1 -
>  drivers/cpufreq/sc520_freq.c           |   1 -
>  drivers/cpufreq/sh-cpufreq.c           |   1 -
>  drivers/cpufreq/sparc-us2e-cpufreq.c   |   6 +-
>  drivers/cpufreq/sparc-us3-cpufreq.c    |   6 +-
>  drivers/cpufreq/speedstep-centrino.c   |   1 -
>  drivers/cpufreq/speedstep-ich.c        |   1 -
>  drivers/cpufreq/speedstep-smi.c        |   1 -
>  include/linux/cpufreq.h                | 386 ++++++++++++++---------------
>  48 files changed, 442 insertions(+), 547 deletions(-)

I like these statistics. :-)

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-06 17:23 ` [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies Viresh Kumar
@ 2013-08-18 14:06   ` Rafael J. Wysocki
  2013-08-19 11:27     ` Viresh Kumar
  0 siblings, 1 reply; 21+ messages in thread
From: Rafael J. Wysocki @ 2013-08-18 14:06 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linaro-kernel, patches, cpufreq, linux-pm, linux-kernel, srivatsa.bhat

On Tuesday, August 06, 2013 10:53:09 PM Viresh Kumar wrote:
> For iterating over all policies currently we are iterating over all CPUs and
> then finding their policies. Lets use the newly created infrastructure
> cpufreq_policy_list.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

I noticed that the current linux-next branch of linux-pm.git caused the
BUG_ON() in lock_policy_rwsem_##mode() to trigger when user space tried to
access cpufreq sysfs attributes before system suspend and after system
resume.

I tried to debug that and it turned out that this patch caused resume
to block indefinitely on one of my test machines and after reverting it the
BUG_ON() stopped triggering, so I've just reverted it in my tree (it is not an
important change).

I don't have the time to figure out why this change breaks things and I would
appreciate it if you tested stuff like suspend/resume on an x86 laptop or
similar with your patches applied before posting them for merging.

Thanks,
Rafael


> ---
>  drivers/cpufreq/cpufreq.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index f5999c4..fe04b79 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -984,8 +984,8 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
>  	struct cpufreq_policy *policy;
>  	unsigned long flags;
>  #ifdef CONFIG_HOTPLUG_CPU
> +	struct cpufreq_policy *tpolicy;
>  	struct cpufreq_governor *gov;
> -	int sibling;
>  #endif
>  
>  	if (cpu_is_offline(cpu))
> @@ -1005,11 +1005,11 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
>  #ifdef CONFIG_HOTPLUG_CPU
>  	/* Check if this cpu was hot-unplugged earlier and has siblings */
>  	read_lock_irqsave(&cpufreq_driver_lock, flags);
> -	for_each_online_cpu(sibling) {
> -		struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
> -		if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) {
> +	list_for_each_entry(tpolicy, &cpufreq_policy_list, policy_list) {
> +		if (cpumask_test_cpu(cpu, tpolicy->related_cpus)) {
>  			read_unlock_irqrestore(&cpufreq_driver_lock, flags);
> -			return cpufreq_add_policy_cpu(cp, cpu, dev, frozen);
> +			return cpufreq_add_policy_cpu(tpolicy, cpu, dev,
> +					frozen);
>  		}
>  	}
>  	read_unlock_irqrestore(&cpufreq_driver_lock, flags);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-18 14:06   ` Rafael J. Wysocki
@ 2013-08-19 11:27     ` Viresh Kumar
  2013-08-19 11:45       ` Amit Kucheria
                         ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-19 11:27 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lists linaro-kernel, Patch Tracking, cpufreq, linux-pm,
	Linux Kernel Mailing List, Srivatsa S. Bhat

On 18 August 2013 19:36, Rafael J. Wysocki <rjw@sisk.pl> wrote:

> I noticed that the current linux-next branch of linux-pm.git caused the
> BUG_ON() in lock_policy_rwsem_##mode() to trigger when user space tried to
> access cpufreq sysfs attributes before system suspend and after system
> resume.

Hmm...

> I tried to debug that and it turned out that this patch caused resume
> to block indefinitely on one of my test machines and after reverting it the
> BUG_ON() stopped triggering, so I've just reverted it in my tree (it is not an
> important change).
>
> I don't have the time to figure out why this change breaks things

It wasn't my patch actually.. It only made it visible that's it :)
The problem is:
- On suspend all CPUs are removed and so governors are
stopped.
- On resume, handle_update() is called for the boot cpu and
cpu_add_dev for all others.

handle_update() doesn't start governor but only plays with
CPUFREQ_GOV_LIMITS.. when we start adding other
CPUs and call: cpufreq_add_policy_cpu() which fails in
following call:

__cpufreq_governor(policy, CPUFREQ_GOV_STOP);

and so cpufreq_policy_cpu never gets initialized to
policy->cpu and stays at -1, and hence the crash.

So, there are few problems with core at this point:
- I don't understand how does the work done in
cpufreq_add_dev() gets done for boot cpu during
resume ? And so how does Srivatsa's "frozen" solution
really works (I haven't had time to investigate, its not
that I couldn't understand it :) )..

- We need to start governor boot cpu in handle_update()
and things would be solved...

> and I would
> appreciate it if you tested stuff like suspend/resume on an x86 laptop or
> similar with your patches applied before posting them for merging.

suspend/resume is broken on my ARM board and that's why
didn't test it..

Testing anything on my thinkpad (with ubuntu) is a pain.. it takes
more than an hour to compile/test a single image... I currently follow
below steps for doing that, don't know if something much
simpler/faster is available :)

https://wiki.ubuntu.com/KernelTeam/GitKernelBuild

Whole day I was able to boot test only 4-5 kernel builds.
Its too slow :(

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-19 11:27     ` Viresh Kumar
@ 2013-08-19 11:45       ` Amit Kucheria
  2013-08-20  6:32         ` Viresh Kumar
  2013-08-19 23:22         ` Rafael J. Wysocki
  2013-08-20  6:35       ` Viresh Kumar
  2 siblings, 1 reply; 21+ messages in thread
From: Amit Kucheria @ 2013-08-19 11:45 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm, Patch Tracking,
	Linux Kernel Mailing List, cpufreq, Srivatsa S. Bhat,
	Andy Whitcroft

On Mon, Aug 19, 2013 at 4:57 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> Testing anything on my thinkpad (with ubuntu) is a pain.. it takes
> more than an hour to compile/test a single image... I currently follow
> below steps for doing that, don't know if something much
> simpler/faster is available :)
>
> https://wiki.ubuntu.com/KernelTeam/GitKernelBuild
>
> Whole day I was able to boot test only 4-5 kernel builds.
> Its too slow :(

Why do you create .deb packages to test development kernels? It _is_ slow.

Instead I'd just add a new grub menu entry to /boot/grub/grub.cfg and
point it to /boot/MyzImage. After a 'make install; make
modules_install' just link /boot/MyzImage to the installed kernel in
/boot. And then generate a new initramfs using 'update-initramfs -c -k
<ver>'

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-19 11:27     ` Viresh Kumar
@ 2013-08-19 23:22         ` Rafael J. Wysocki
  2013-08-19 23:22         ` Rafael J. Wysocki
  2013-08-20  6:35       ` Viresh Kumar
  2 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2013-08-19 23:22 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Lists linaro-kernel, Patch Tracking, cpufreq, linux-pm,
	Linux Kernel Mailing List, Srivatsa S. Bhat

On Monday, August 19, 2013 04:57:19 PM Viresh Kumar wrote:
> On 18 August 2013 19:36, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > I noticed that the current linux-next branch of linux-pm.git caused the
> > BUG_ON() in lock_policy_rwsem_##mode() to trigger when user space tried to
> > access cpufreq sysfs attributes before system suspend and after system
> > resume.
> 
> Hmm...
> 
> > I tried to debug that and it turned out that this patch caused resume
> > to block indefinitely on one of my test machines and after reverting it the
> > BUG_ON() stopped triggering, so I've just reverted it in my tree (it is not an
> > important change).
> >
> > I don't have the time to figure out why this change breaks things
> 
> It wasn't my patch actually.. It only made it visible that's it :)
> The problem is:
> - On suspend all CPUs are removed and so governors are
> stopped.
> - On resume, handle_update() is called for the boot cpu and
> cpu_add_dev for all others.
> 
> handle_update() doesn't start governor but only plays with
> CPUFREQ_GOV_LIMITS.. when we start adding other
> CPUs and call: cpufreq_add_policy_cpu() which fails in
> following call:
> 
> __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> 
> and so cpufreq_policy_cpu never gets initialized to
> policy->cpu and stays at -1, and hence the crash.
> 
> So, there are few problems with core at this point:
> - I don't understand how does the work done in
> cpufreq_add_dev() gets done for boot cpu during
> resume ? And so how does Srivatsa's "frozen" solution
> really works (I haven't had time to investigate, its not
> that I couldn't understand it :) )..
> 
> - We need to start governor boot cpu in handle_update()
> and things would be solved...
> 
> > and I would
> > appreciate it if you tested stuff like suspend/resume on an x86 laptop or
> > similar with your patches applied before posting them for merging.
> 
> suspend/resume is broken on my ARM board and that's why
> didn't test it..
> 
> Testing anything on my thinkpad (with ubuntu) is a pain.. it takes
> more than an hour to compile/test a single image... I currently follow
> below steps for doing that, don't know if something much
> simpler/faster is available :)
> 
> https://wiki.ubuntu.com/KernelTeam/GitKernelBuild
> 
> Whole day I was able to boot test only 4-5 kernel builds.
> Its too slow :(

Well, that's a matter of setting up a workspace, basically.

As a general rule, you should be able test the changes you're making on
hardware that they are supposed to run on.  If that's something not very
easy to acquire, like s390, you can make an excuse of not having access to
that hardware and hope that someone will test the changes for you (or ACKs
them without testing, because they are "obviously" correct).  However, if
that's ACPI-compatible x86, that excuse pretty much doesn't work, because
you can find those things everywhere.

I have no experience with building kernels on Ubuntu to be honest, as I've been
using openSUSE as my testbed distro for several years.

>From my experience, however, it is good to figure out what needs to be included
into your test kernel and configure away everything else.  Also, I'd recommend
to build as much as possible into the kernel image and avoid compiling too many
modules, because installing modules takes time too (ideally, if you can get
away without any modules at all, that's the best option timing-wise).  Just
select only the drivers that you're going to use and unset all of the options
that don't apply to your test machine.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
@ 2013-08-19 23:22         ` Rafael J. Wysocki
  0 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2013-08-19 23:22 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Lists linaro-kernel, Patch Tracking, cpufreq, linux-pm,
	Linux Kernel Mailing List, Srivatsa S. Bhat

On Monday, August 19, 2013 04:57:19 PM Viresh Kumar wrote:
> On 18 August 2013 19:36, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > I noticed that the current linux-next branch of linux-pm.git caused the
> > BUG_ON() in lock_policy_rwsem_##mode() to trigger when user space tried to
> > access cpufreq sysfs attributes before system suspend and after system
> > resume.
> 
> Hmm...
> 
> > I tried to debug that and it turned out that this patch caused resume
> > to block indefinitely on one of my test machines and after reverting it the
> > BUG_ON() stopped triggering, so I've just reverted it in my tree (it is not an
> > important change).
> >
> > I don't have the time to figure out why this change breaks things
> 
> It wasn't my patch actually.. It only made it visible that's it :)
> The problem is:
> - On suspend all CPUs are removed and so governors are
> stopped.
> - On resume, handle_update() is called for the boot cpu and
> cpu_add_dev for all others.
> 
> handle_update() doesn't start governor but only plays with
> CPUFREQ_GOV_LIMITS.. when we start adding other
> CPUs and call: cpufreq_add_policy_cpu() which fails in
> following call:
> 
> __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
> 
> and so cpufreq_policy_cpu never gets initialized to
> policy->cpu and stays at -1, and hence the crash.
> 
> So, there are few problems with core at this point:
> - I don't understand how does the work done in
> cpufreq_add_dev() gets done for boot cpu during
> resume ? And so how does Srivatsa's "frozen" solution
> really works (I haven't had time to investigate, its not
> that I couldn't understand it :) )..
> 
> - We need to start governor boot cpu in handle_update()
> and things would be solved...
> 
> > and I would
> > appreciate it if you tested stuff like suspend/resume on an x86 laptop or
> > similar with your patches applied before posting them for merging.
> 
> suspend/resume is broken on my ARM board and that's why
> didn't test it..
> 
> Testing anything on my thinkpad (with ubuntu) is a pain.. it takes
> more than an hour to compile/test a single image... I currently follow
> below steps for doing that, don't know if something much
> simpler/faster is available :)
> 
> https://wiki.ubuntu.com/KernelTeam/GitKernelBuild
> 
> Whole day I was able to boot test only 4-5 kernel builds.
> Its too slow :(

Well, that's a matter of setting up a workspace, basically.

As a general rule, you should be able test the changes you're making on
hardware that they are supposed to run on.  If that's something not very
easy to acquire, like s390, you can make an excuse of not having access to
that hardware and hope that someone will test the changes for you (or ACKs
them without testing, because they are "obviously" correct).  However, if
that's ACPI-compatible x86, that excuse pretty much doesn't work, because
you can find those things everywhere.

I have no experience with building kernels on Ubuntu to be honest, as I've been
using openSUSE as my testbed distro for several years.

From my experience, however, it is good to figure out what needs to be included
into your test kernel and configure away everything else.  Also, I'd recommend
to build as much as possible into the kernel image and avoid compiling too many
modules, because installing modules takes time too (ideally, if you can get
away without any modules at all, that's the best option timing-wise).  Just
select only the drivers that you're going to use and unset all of the options
that don't apply to your test machine.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-19 11:45       ` Amit Kucheria
@ 2013-08-20  6:32         ` Viresh Kumar
  0 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-20  6:32 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: Rafael J. Wysocki, Lists linaro-kernel, linux-pm, Patch Tracking,
	Linux Kernel Mailing List, cpufreq, Srivatsa S. Bhat,
	Andy Whitcroft

On 19 August 2013 17:15, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> Why do you create .deb packages to test development kernels? It _is_ slow.
>
> Instead I'd just add a new grub menu entry to /boot/grub/grub.cfg and
> point it to /boot/MyzImage. After a 'make install; make
> modules_install' just link /boot/MyzImage to the installed kernel in
> /boot. And then generate a new initramfs using 'update-initramfs -c -k
> <ver>'

initramfs was fixed automatically for me with make install.. Thanks it saved
lots of my time :)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-19 23:22         ` Rafael J. Wysocki
  (?)
@ 2013-08-20  6:33         ` Viresh Kumar
  -1 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-20  6:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lists linaro-kernel, Patch Tracking, cpufreq, linux-pm,
	Linux Kernel Mailing List, Srivatsa S. Bhat

On 20 August 2013 04:52, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From my experience, however, it is good to figure out what needs to be included
> into your test kernel and configure away everything else.  Also, I'd recommend
> to build as much as possible into the kernel image and avoid compiling too many
> modules, because installing modules takes time too (ideally, if you can get
> away without any modules at all, that's the best option timing-wise).  Just
> select only the drivers that you're going to use and unset all of the options
> that don't apply to your test machine.

I haven't done this exercise yet but I have a workable solution with default
.config as suggested by Amit :)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies
  2013-08-19 11:27     ` Viresh Kumar
  2013-08-19 11:45       ` Amit Kucheria
  2013-08-19 23:22         ` Rafael J. Wysocki
@ 2013-08-20  6:35       ` Viresh Kumar
  2 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2013-08-20  6:35 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lists linaro-kernel, Patch Tracking, cpufreq, linux-pm,
	Linux Kernel Mailing List, Srivatsa S. Bhat

On 19 August 2013 16:57, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> It wasn't my patch actually.. It only made it visible that's it :)
> The problem is:
> - On suspend all CPUs are removed and so governors are
> stopped.
> - On resume, handle_update() is called for the boot cpu and
> cpu_add_dev for all others.
>
> handle_update() doesn't start governor but only plays with
> CPUFREQ_GOV_LIMITS.. when we start adding other
> CPUs and call: cpufreq_add_policy_cpu() which fails in
> following call:
>
> __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
>
> and so cpufreq_policy_cpu never gets initialized to
> policy->cpu and stays at -1, and hence the crash.
>
> So, there are few problems with core at this point:
> - I don't understand how does the work done in
> cpufreq_add_dev() gets done for boot cpu during
> resume ? And so how does Srivatsa's "frozen" solution
> really works (I haven't had time to investigate, its not
> that I couldn't understand it :) )..
>
> - We need to start governor boot cpu in handle_update()
> and things would be solved...

Whatever I wrote here was simply _Bullshit_ :(

I am about to send you a fixup patchset that fixes this issue, and
yes it was my patch which introduced this problem :(, but
because of some mishandling of cpufreq_policy_list :)

--
viresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2013-08-20  6:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-06 17:23 [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 01/11] cpufreq: Cleanup header files included in core Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 02/11] cpufreq: Re-arrange declarations in cpufreq.h Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 03/11] cpufreq: Give consistent names for struct cpufreq_policy * Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 04/11] cpufreq: Use sizeof(*ptr) form for finding size of a struct Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 05/11] cpufreq: Pass policy to cpufreq_add_policy_cpu() Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 06/11] cpufreq: Store cpufreq policies in a list Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 07/11] cpufreq: Use cpufreq_policy_list for iterating over policies Viresh Kumar
2013-08-18 14:06   ` Rafael J. Wysocki
2013-08-19 11:27     ` Viresh Kumar
2013-08-19 11:45       ` Amit Kucheria
2013-08-20  6:32         ` Viresh Kumar
2013-08-19 23:22       ` Rafael J. Wysocki
2013-08-19 23:22         ` Rafael J. Wysocki
2013-08-20  6:33         ` Viresh Kumar
2013-08-20  6:35       ` Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 08/11] cpufreq: Fix broken usage of governor->owner's refcount Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 09/11] cpufreq: Don't use cpufreq_driver->owner's refcount to protect critical sections Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 10/11] cpufreq: Remove struct cpufreq_driver's owner field Viresh Kumar
2013-08-06 17:23 ` [PATCH V2 11/11] cpufreq: improve error checking on return values of __cpufreq_governor() Viresh Kumar
2013-08-07  0:21 ` [PATCH V2 00/11] CPUFreq: Fixes & Cleanups for 3.12 Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.