driver core: Move driver_data back to struct device

PATCH In review Jean Delvare 2 participants 4 messages Apr 7, 2014 – Apr 10, 2014 Driver core

Share & tools

Jean Delvare

author

2014-04-07 08:53 UTC #0 lore raw

PATCH 1/1 driver core: Move driver_data back to struct device

Having to allocate memory as part of dev_set_drvdata() is a problem because that memory may never get freed if the device itself is not created. So move driver_data back to struct device.

This is a partial revert of commit b4028437.

Signed-off-by Jean Delvare <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---
Greg, we discussed that back in January (in a thread named "Freeing of dev->p") but I did not hear back from you and I can't see this fix in your driver-core tree.

As discussed in that thread, really reverting b4028437 might be an even better option, performance-wise. If we do that, we'll first have to update drivers/iommu/exynos-iommu.c and drivers/vfio/vfio.c to no longer check for the return value of dev_set_drvdata(). I can provide the patches, if you're fine with that.

 drivers/base/base.h    |    3 ---
 drivers/base/dd.c      |   13 +++----------
 include/linux/device.h |    4 ++++
 3 files changed, 7 insertions(+), 13 deletions(-)

drivers/base/base.h +0 -3

@@ -63,8 +63,6 @@ struct driver_private {
  *	binding of drivers which were unable to get all the resources needed by
  *	the device; typically because it depends on another driver getting
  *	probed first.
- * @driver_data - private pointer for driver specific info.  Will turn into a
- * list soon.
  * @device - pointer back to the struct class that this structure is
  * associated with.
  *

@@ -76,7 +74,6 @@ struct device_private {
 	struct klist_node knode_driver;
 	struct klist_node knode_bus;
 	struct list_head deferred_probe;
-	void *driver_data;
 	struct device *device;
 };
 #define to_device_private_parent(obj)	\

drivers/base/dd.c +3 ~~-10~~

@@ -577,22 +577,15 @@ void driver_detach(struct device_driver
  */
 void *dev_get_drvdata(const struct device *dev)
 {
-	if (dev && dev->p)
-		return dev->p->driver_data;
+	if (dev)
+		return dev->driver_data;
 	return NULL;
 }
 EXPORT_SYMBOL(dev_get_drvdata);
 
 int dev_set_drvdata(struct device *dev, void *data)
 {
-	int error;
-
-	if (!dev->p) {
-		error = device_private_init(dev);
-		if (error)
-			return error;
-	}
-	dev->p->driver_data = data;
+	dev->driver_data = data;
 	return 0;
 }
 EXPORT_SYMBOL(dev_set_drvdata);

include/linux/device.h +4 -0

@@ -679,6 +679,8 @@ struct acpi_dev_node {
  * 		variants, which GPIO pins act in what additional roles, and so
  * 		on.  This shrinks the "Board Support Packages" (BSPs) and
  * 		minimizes board-specific #ifdefs in drivers.
+ * @driver_data: Private pointer for driver specific info.  Will turn into a
+ *		list soon.
  * @power:	For device power management.
  * 		See Documentation/power/devices.txt for details.
  * @pm_domain:	Provide callbacks that are executed during system suspend,

@@ -740,6 +742,8 @@ struct device {
 					   device */
 	void		*platform_data;	/* Platform specific data, device
 					   core doesn't touch it */
+	void		*driver_data;	/* Driver data, set and get with
+					   dev_set/get_drvdata */
 	struct dev_pm_info	power;
 	struct dev_pm_domain	*pm_domain;

Signature · 2 lines

Jean Delvare
SUSE L3 Support

Greg Kroah-Hartman

maintainer

2014-04-07 20:55 UTC #1 lore raw

Quoted 4 lines · Jean Delvare

Having to allocate memory as part of dev_set_drvdata() is a problem because that memory may never get freed if the device itself is not created. So move driver_data back to struct device.

Quoted 8 lines

This is a partial revert of commit b4028437.
Signed-off-by Jean Delvare <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
Greg, we discussed that back in January (in a thread named "Freeing of dev->p") but I did not hear back from you and I can't see this fix in your driver-core tree.

Thanks for doing this, I'll queue this up after -rc1 is out. I still have that thread in my TODO list, unfortunatly, higher-priority items keep bumping it down :(

greg k-h

Jean Delvare

author

2014-04-10 07:50 UTC #2 lore raw

Hi Greg,

Quoted 14 lines · Greg Kroah-Hartman

Quoted 4 lines · Jean Delvare
Having to allocate memory as part of dev_set_drvdata() is a problem because that memory may never get freed if the device itself is not created. So move driver_data back to struct device.
Quoted 8 lines
This is a partial revert of commit b4028437.
Signed-off-by Jean Delvare <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
Greg, we discussed that back in January (in a thread named "Freeing of dev->p") but I did not hear back from you and I can't see this fix in your driver-core tree.

Quoted 3 lines

Thanks for doing this, I'll queue this up after -rc1 is out. I still have that thread in my TODO list, unfortunatly, higher-priority items keep bumping it down :(

No problem, I understand.

Meanwhile I have prepared a patch set with all the cleanups that could go in after that one, basically reverting all of commit b4028437 step by step. If you want to take a look, it's at:
http://jdelvare.nerim.net/devel/linux-3/driver-core/

FWIW the full patch set shaves about 1.2 MB off an allmodconfig x86_64 build. And it certainly comes with some performance gains too.

I can post the whole set now if you want, I just did not want to throw too much at you if you are already swamped.

Also, I can merge some of these patches together if you think I went too fine-grained.

Thanks,

Signature · 2 lines

Jean Delvare
SUSE L3 Support

Greg Kroah-Hartman

maintainer

2014-04-10 18:22 UTC #3 lore raw

Quoted 28 lines · Jean Delvare

Hi Greg,
Quoted 14 lines · Greg Kroah-Hartman
Quoted 4 lines · Jean Delvare
Having to allocate memory as part of dev_set_drvdata() is a problem because that memory may never get freed if the device itself is not created. So move driver_data back to struct device.
Quoted 8 lines
This is a partial revert of commit b4028437.
Signed-off-by Jean Delvare <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
Greg, we discussed that back in January (in a thread named "Freeing of dev->p") but I did not hear back from you and I can't see this fix in your driver-core tree.
Quoted 3 lines
Thanks for doing this, I'll queue this up after -rc1 is out. I still have that thread in my TODO list, unfortunatly, higher-priority items keep bumping it down :(
No problem, I understand.
Meanwhile I have prepared a patch set with all the cleanups that could go in after that one, basically reverting all of commit b4028437 step by step. If you want to take a look, it's at:
http://jdelvare.nerim.net/devel/linux-3/driver-core/

Quoted 5 lines

FWIW the full patch set shaves about 1.2 MB off an allmodconfig x86_64 build. And it certainly comes with some performance gains too.
I can post the whole set now if you want, I just did not want to throw too much at you if you are already swamped.

Sure, please send them to me, my "swampedness" isn't an issue for queueing up patches, it helps, as they will then have to be dealt with in my queue :)

thanks,

greg k-h