Patch: linux-2.5.45/drivers/base/bus.c - new field to consolidate memory allocation in many drivers

[Adam J. Richter]

--FL5UXtIhxfXey3p5
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

	I believe that the following little change will enable
elimination of a kmalloc/kfree pair from many device drivers,
and, more importantly, eliminate the rarely tested often buggy
error leg for dealing with that memory allocation failure.

	This patch allows device drivers to tell the generic device
code to handle allocating the per-device blob of memory that is
normally stored in device.driver_data.  It does so by adding a new
field device_driver.drvdata_size, with the following semantics:

	0	- Do not attempt to allocate or free device.driver_data.
		  This is compatible with previous behavior (although we
		  now initialize driver_data to NULL before calling the
		  probe routine).

	>0	- Allocate the specified number of bytes, fill it with
		  all zeroes, and store the address in device.driver_data
		  before calling the device's probe routine.  Abort the
		  probe with -ENOMEM if memory allocation fails.  Free the
		  storage if the probe fails or when the driver is
		  removed.  (The allocation and freeing mechanisms are
		  not specified, although the current mechanism uses
		  kmalloc/kfree).

	-1	- Set device.driver_data to NULL before calling the probe
		  routine.  When the probe routine returns failure and
		  when the driver is removed, check the value of
		  device.driver_data.  If it is non-NULL, kfree it.
		  This is handy for drivers that want to do their own
		  memory allocation.  This is handy if the driver needs
		  a variable sized block or really really really does not
		  want the allocated memory initialized to all zeroes.

	You may wonder about the trade-off of initializing the
allocated memory to all zeroes (in the drvdata_size > 0 case).  Driver
probes are executed relatively rarely (as opposed to an IO path that
might get executed a thousand times per second) and it is hard to
identify bugs due to uninitialized values.  The structures that the
probles fill in often gain new parameters over time, so it is easy to
make initialization bugs that gcc cannot easily detect.  It also makes
it easier to design new fields in these structures where a value of
zero will provide backward compatible behavior.  A small bonus is that
driver initialization code only needs to fill in non-zero values
explicitly.

	If there is a case where the performance hit from clearing the
memory is sustantial, you could use drvdata_size = -1.  If it turns
out that the situation is common, we could have a flag to skip the
clearing of the memory, but I don't think that such cases will be
common.

	I realize that many drivers store the result of some high
level allocate routine in their private data (for example,
scsi_register).  Later, I intend to make versions of those routines
that take pointer to a block of zero-filled memory rather than calling
kmalloc themselves.

	Anyhow, here is the patch.  I would like to start writing
driver clean-ups that use this patch soon, so I would like to see this
addition integrated into the kernel as sson as possible.  I am running
a kernel with this patch compiled in now, although I have not yet
changed any drivers to take advantage of it.

	Pat, are you the person I should be submitting this patch
to?  Is there someone else I should be submitting this patch to?
Please let me know.  Thanks in advance.

-- 
Adam J. Richter     __     ______________   575 Oroville Road
[e-mail]     \ /                  Milpitas, California 95035
[phone]         | g g d r a s i l   United States of America
                         "Free Software For The Rest Of Us."

--FL5UXtIhxfXey3p5
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="devalloc.diff"

--- linux-2.5.45/include/linux/device.h	2002-10-30 16:43:40.000000000 -0800
+++ linux/include/linux/device.h	2002-11-02 05:20:36.000000000 -0800
@@ -114,6 +114,7 @@
 	char			* name;
 	struct bus_type		* bus;
 	struct device_class	* devclass;
+	int			drvdata_size;
 
 	rwlock_t		lock;
 	atomic_t		refcount;
--- linux-2.5.45/drivers/base/bus.c	2002-10-30 16:42:20.000000000 -0800
+++ linux/drivers/base/bus.c	2002-11-02 05:21:37.000000000 -0800
@@ -98,6 +98,17 @@
 {
 	int error = -ENODEV;
 	if (dev->bus->match(dev,drv)) {
+
+		if (drv->drvdata_size > 0) {
+			dev->driver_data = kmalloc(drv->drvdata_size);
+			if (dev->driver_data)
+				memset(dev->driver_data, 0, drv->drvdata_size);
+			else
+				return -ENOMEM;
+		}
+		else
+			dev->driver_data = NULL;
+
 		dev->driver = drv;
 		if (drv->probe) {
 			if (!(error = drv->probe(dev)))
@@ -166,6 +177,12 @@
 		devclass_remove_device(dev);
 		if (drv->remove)
 			drv->remove(dev);
+		if (drv->drvdata_size) {
+			if (dev->driver_data)
+				kfree(dev->driver_data);
+			else
+				BUG_ON(drv->drvdata_size != -1);
+		}
 		dev->driver = NULL;
 	}
 }

--FL5UXtIhxfXey3p5--