diff mbox

[16/21] cpuidle: avoid unnecessary kzalloc/free of struct cpuidle_device_kobj

Message ID fb67ff6fa01bde06aeab43c4a02da2923615e6ef.1379779777.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar Sept. 22, 2013, 1:21 a.m. UTC 
We always need to allocate struct cpuidle_device_kobj for all CPUs and so there
is no real need to have a pointer to it inside struct cpuidle_device.

This patch makes a object instance of struct cpuidle_device_kobj inside struct
cpuidle_device instead of a pointer.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpuidle/sysfs.c | 24 +++++-------------------
 include/linux/cpuidle.h | 11 +++++++++--
 2 files changed, 14 insertions(+), 21 deletions(-)

Comments

Daniel Lezcano Sept. 25, 2013, 10:12 p.m. UTC | #1 
On 09/22/2013 03:21 AM, Viresh Kumar wrote:
> We always need to allocate struct cpuidle_device_kobj for all CPUs and so there
> is no real need to have a pointer to it inside struct cpuidle_device.
> 
> This patch makes a object instance of struct cpuidle_device_kobj inside struct
> cpuidle_device instead of a pointer.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

nack, it was made in purpose for kobject_init_and_add.

see commit 728ce22b696f9f1404a74d7b2279a65933553a1b
Viresh Kumar Sept. 26, 2013, 6:05 a.m. UTC | #2 
On 26 September 2013 03:42, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 09/22/2013 03:21 AM, Viresh Kumar wrote:
>> We always need to allocate struct cpuidle_device_kobj for all CPUs and so there
>> is no real need to have a pointer to it inside struct cpuidle_device.
>>
>> This patch makes a object instance of struct cpuidle_device_kobj inside struct
>> cpuidle_device instead of a pointer.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> nack, it was made in purpose for kobject_init_and_add.
>
> see commit 728ce22b696f9f1404a74d7b2279a65933553a1b

Ahh.. sorry for missing that one :(

Now that I understand why it was introduced, I am thinking if
we can make hotplug path a bit fast? By not freeing sysfs stuff
and only hiding it for time being? And so we wouldn't be required
to do unnecessary initialisations while coming back?

Would that be worth it?
Daniel Lezcano Sept. 26, 2013, 8:30 a.m. UTC | #3 
On 09/26/2013 08:05 AM, Viresh Kumar wrote:
> On 26 September 2013 03:42, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>> On 09/22/2013 03:21 AM, Viresh Kumar wrote:
>>> We always need to allocate struct cpuidle_device_kobj for all CPUs and so there
>>> is no real need to have a pointer to it inside struct cpuidle_device.
>>>
>>> This patch makes a object instance of struct cpuidle_device_kobj inside struct
>>> cpuidle_device instead of a pointer.
>>>
>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>
>> nack, it was made in purpose for kobject_init_and_add.
>>
>> see commit 728ce22b696f9f1404a74d7b2279a65933553a1b
>
> Ahh.. sorry for missing that one :(
>
> Now that I understand why it was introduced, I am thinking if
> we can make hotplug path a bit fast? By not freeing sysfs stuff
> and only hiding it for time being? And so we wouldn't be required
> to do unnecessary initialisations while coming back?
>
> Would that be worth it?

Yes if it possible.
diff mbox

Patch

diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index ade31a9..db0aac3 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -172,12 +172,6 @@  struct cpuidle_attr {
 
 #define attr_to_cpuidleattr(a) container_of(a, struct cpuidle_attr, attr)
 
-struct cpuidle_device_kobj {
-	struct cpuidle_device *dev;
-	struct completion kobj_unregister;
-	struct kobject kobj;
-};
-
 static inline struct cpuidle_device *to_cpuidle_device(struct kobject *kobj)
 {
 	struct cpuidle_device_kobj *kdev =
@@ -399,7 +393,7 @@  static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
 {
 	int i, ret = -ENOMEM;
 	struct cpuidle_state_kobj *kobj;
-	struct cpuidle_device_kobj *kdev = device->kobj_dev;
+	struct cpuidle_device_kobj *kdev = &device->kobj_dev;
 	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
 
 	/* state statistics */
@@ -525,7 +519,7 @@  static struct kobj_type ktype_driver_cpuidle = {
 static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev)
 {
 	struct cpuidle_driver_kobj *kdrv;
-	struct cpuidle_device_kobj *kdev = dev->kobj_dev;
+	struct cpuidle_device_kobj *kdev = &dev->kobj_dev;
 	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
 	int ret;
 
@@ -606,24 +600,17 @@  void cpuidle_remove_device_sysfs(struct cpuidle_device *device)
  */
 int cpuidle_add_sysfs(struct cpuidle_device *dev)
 {
-	struct cpuidle_device_kobj *kdev;
+	struct cpuidle_device_kobj *kdev = &dev->kobj_dev;
 	struct device *cpu_dev = get_cpu_device((unsigned long)dev->cpu);
 	int error;
 
-	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
-	if (!kdev)
-		return -ENOMEM;
 	kdev->dev = dev;
-	dev->kobj_dev = kdev;
-
 	init_completion(&kdev->kobj_unregister);
 
 	error = kobject_init_and_add(&kdev->kobj, &ktype_cpuidle, &cpu_dev->kobj,
 				   "cpuidle");
-	if (error) {
-		kfree(kdev);
+	if (error)
 		return error;
-	}
 
 	kobject_uevent(&kdev->kobj, KOBJ_ADD);
 
@@ -636,9 +623,8 @@  int cpuidle_add_sysfs(struct cpuidle_device *dev)
  */
 void cpuidle_remove_sysfs(struct cpuidle_device *dev)
 {
-	struct cpuidle_device_kobj *kdev = dev->kobj_dev;
+	struct cpuidle_device_kobj *kdev = &dev->kobj_dev;
 
 	kobject_put(&kdev->kobj);
 	wait_for_completion(&kdev->kobj_unregister);
-	kfree(kdev);
 }
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index c082425..1fc5cb5 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -11,6 +11,8 @@ 
 #ifndef _LINUX_CPUIDLE_H
 #define _LINUX_CPUIDLE_H
 
+#include <linux/completion.h>
+#include <linux/kobject.h>
 #include <linux/percpu.h>
 #include <linux/list.h>
 #include <linux/hrtimer.h>
@@ -59,10 +61,15 @@  struct cpuidle_state {
 
 #define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000)
 
-struct cpuidle_device_kobj;
 struct cpuidle_state_kobj;
 struct cpuidle_driver_kobj;
 
+struct cpuidle_device_kobj {
+	struct cpuidle_device *dev;
+	struct completion kobj_unregister;
+	struct kobject kobj;
+};
+
 struct cpuidle_device {
 	unsigned int		registered:1;
 	unsigned int		enabled:1;
@@ -73,7 +80,7 @@  struct cpuidle_device {
 	struct cpuidle_state_usage	states_usage[CPUIDLE_STATE_MAX];
 	struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
 	struct cpuidle_driver_kobj *kobj_driver;
-	struct cpuidle_device_kobj *kobj_dev;
+	struct cpuidle_device_kobj kobj_dev;
 	struct list_head 	device_list;
 
 #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED