* [patch] kref shrinkage patches -- 2 of 2 -- kref shrinkage
2004-07-20 12:23 [patch] kref shrinkage patches -- 1 of 2 -- kref shrinkage Ravikiran G Thirumalai
@ 2004-07-20 12:27 ` Ravikiran G Thirumalai
2004-07-21 6:14 ` Greg KH
2004-07-21 6:14 ` [patch] kref shrinkage patches -- 1 " Greg KH
1 sibling, 1 reply; 5+ messages in thread
From: Ravikiran G Thirumalai @ 2004-07-20 12:27 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel
Here's the patch to change current kref users to use the modified
kref_init and kref_put api (result of kref object shrinkage).
Thanks,
Kiran
scsi/sd.c | 8 +-
scsi/sr.c | 8 +-
usb/core/config.c | 5 +
usb/core/message.c | 3 -
usb/core/urb.c | 4 -
usb/host/ehci-mem.c | 4 -
usb/serial/usb-serial.c | 136 ++++++++++++++++++++++++------------------------
7 files changed, 85 insertions(+), 83 deletions(-)
Signed-off-by: Ravikiran Thirumalai <kiran@in.ibm.com>
diff -ruN -X dontdiff2 linux-2.6.7/drivers/scsi/sd.c kref-2.6.7/drivers/scsi/sd.c
--- linux-2.6.7/drivers/scsi/sd.c 2004-06-16 10:49:44.000000000 +0530
+++ kref-2.6.7/drivers/scsi/sd.c 2004-07-20 13:03:49.000000000 +0530
@@ -186,7 +186,7 @@
return sdkp;
out_put:
- kref_put(&sdkp->kref);
+ kref_put(&sdkp->kref, scsi_disk_release);
out_sdkp:
sdkp = NULL;
out:
@@ -198,7 +198,7 @@
{
down(&sd_ref_sem);
scsi_device_put(sdkp->device);
- kref_put(&sdkp->kref);
+ kref_put(&sdkp->kref, scsi_disk_release);
up(&sd_ref_sem);
}
@@ -1360,7 +1360,7 @@
goto out;
memset (sdkp, 0, sizeof(*sdkp));
- kref_init(&sdkp->kref, scsi_disk_release);
+ kref_init(&sdkp->kref);
/* Note: We can accomodate 64 partitions, but the genhd code
* assumes partitions allocate consecutive minors, which they don't.
@@ -1462,7 +1462,7 @@
del_gendisk(sdkp->disk);
sd_shutdown(dev);
down(&sd_ref_sem);
- kref_put(&sdkp->kref);
+ kref_put(&sdkp->kref, scsi_disk_release);
up(&sd_ref_sem);
return 0;
diff -ruN -X dontdiff2 linux-2.6.7/drivers/scsi/sr.c kref-2.6.7/drivers/scsi/sr.c
--- linux-2.6.7/drivers/scsi/sr.c 2004-06-16 10:50:27.000000000 +0530
+++ kref-2.6.7/drivers/scsi/sr.c 2004-07-20 13:03:49.000000000 +0530
@@ -144,7 +144,7 @@
goto out;
out_put:
- kref_put(&cd->kref);
+ kref_put(&cd->kref, sr_kref_release);
out_null:
cd = NULL;
out:
@@ -156,7 +156,7 @@
{
down(&sr_ref_sem);
scsi_device_put(cd->device);
- kref_put(&cd->kref);
+ kref_put(&cd->kref, sr_kref_release);
up(&sr_ref_sem);
}
@@ -573,7 +573,7 @@
goto fail;
memset(cd, 0, sizeof(*cd));
- kref_init(&cd->kref, sr_kref_release);
+ kref_init(&cd->kref);
disk = alloc_disk(1);
if (!disk)
@@ -952,7 +952,7 @@
del_gendisk(cd->disk);
down(&sr_ref_sem);
- kref_put(&cd->kref);
+ kref_put(&cd->kref, sr_kref_release);
up(&sr_ref_sem);
return 0;
diff -ruN -X dontdiff2 linux-2.6.7/drivers/usb/core/config.c kref-2.6.7/drivers/usb/core/config.c
--- linux-2.6.7/drivers/usb/core/config.c 2004-06-16 10:48:52.000000000 +0530
+++ kref-2.6.7/drivers/usb/core/config.c 2004-07-20 13:03:49.000000000 +0530
@@ -356,7 +356,7 @@
if (!intfc)
return -ENOMEM;
memset(intfc, 0, len);
- kref_init(&intfc->ref, usb_release_interface_cache);
+ kref_init(&intfc->ref);
}
/* Skip over any Class Specific or Vendor Specific descriptors;
@@ -422,7 +422,8 @@
for (i = 0; i < cf->desc.bNumInterfaces; i++) {
if (cf->intf_cache[i])
- kref_put(&cf->intf_cache[i]->ref);
+ kref_put(&cf->intf_cache[i]->ref,
+ usb_release_interface_cache);
}
}
kfree(dev->config);
diff -ruN -X dontdiff2 linux-2.6.7/drivers/usb/core/message.c kref-2.6.7/drivers/usb/core/message.c
--- linux-2.6.7/drivers/usb/core/message.c 2004-06-16 10:49:02.000000000 +0530
+++ kref-2.6.7/drivers/usb/core/message.c 2004-07-20 15:07:24.000000000 +0530
@@ -1077,11 +1077,12 @@
static void release_interface(struct device *dev)
{
+ extern void destroy_serial(struct kref *kref);
struct usb_interface *intf = to_usb_interface(dev);
struct usb_interface_cache *intfc =
altsetting_to_usb_interface_cache(intf->altsetting);
- kref_put(&intfc->ref);
+ kref_put(&intfc->ref, destroy_serial);
kfree(intf);
}
diff -ruN -X dontdiff2 linux-2.6.7/drivers/usb/core/urb.c kref-2.6.7/drivers/usb/core/urb.c
--- linux-2.6.7/drivers/usb/core/urb.c 2004-06-16 10:49:36.000000000 +0530
+++ kref-2.6.7/drivers/usb/core/urb.c 2004-07-20 13:03:49.000000000 +0530
@@ -39,7 +39,7 @@
{
if (urb) {
memset(urb, 0, sizeof(*urb));
- kref_init(&urb->kref, urb_destroy);
+ kref_init(&urb->kref);
spin_lock_init(&urb->lock);
}
}
@@ -88,7 +88,7 @@
void usb_free_urb(struct urb *urb)
{
if (urb)
- kref_put(&urb->kref);
+ kref_put(&urb->kref, urb_destroy);
}
/**
diff -ruN -X dontdiff2 linux-2.6.7/drivers/usb/host/ehci-mem.c kref-2.6.7/drivers/usb/host/ehci-mem.c
--- linux-2.6.7/drivers/usb/host/ehci-mem.c 2004-06-16 10:50:03.000000000 +0530
+++ kref-2.6.7/drivers/usb/host/ehci-mem.c 2004-07-20 13:03:49.000000000 +0530
@@ -114,7 +114,7 @@
return qh;
memset (qh, 0, sizeof *qh);
- kref_init(&qh->kref, qh_destroy);
+ kref_init(&qh->kref);
qh->ehci = ehci;
qh->qh_dma = dma;
// INIT_LIST_HEAD (&qh->qh_list);
@@ -139,7 +139,7 @@
static inline void qh_put (struct ehci_qh *qh)
{
- kref_put(&qh->kref);
+ kref_put(&qh->kref, qh_destroy);
}
/*-------------------------------------------------------------------------*/
diff -ruN -X dontdiff2 linux-2.6.7/drivers/usb/serial/usb-serial.c kref-2.6.7/drivers/usb/serial/usb-serial.c
--- linux-2.6.7/drivers/usb/serial/usb-serial.c 2004-06-16 10:49:17.000000000 +0530
+++ kref-2.6.7/drivers/usb/serial/usb-serial.c 2004-07-20 15:07:52.000000000 +0530
@@ -443,6 +443,69 @@
return;
}
+static void serial_shutdown (struct usb_serial *serial)
+{
+ dbg ("%s", __FUNCTION__);
+
+ serial->type->shutdown(serial);
+}
+
+void destroy_serial(struct kref *kref)
+{
+ struct usb_serial *serial;
+ struct usb_serial_port *port;
+ int i;
+
+ serial = to_usb_serial(kref);
+
+ dbg ("%s - %s", __FUNCTION__, serial->type->name);
+ serial_shutdown (serial);
+
+ /* return the minor range that this device had */
+ return_serial(serial);
+
+ for (i = 0; i < serial->num_ports; ++i)
+ serial->port[i]->open_count = 0;
+
+ /* the ports are cleaned up and released in port_release() */
+ for (i = 0; i < serial->num_ports; ++i)
+ if (serial->port[i]->dev.parent != NULL) {
+ device_unregister(&serial->port[i]->dev);
+ serial->port[i] = NULL;
+ }
+
+ /* If this is a "fake" port, we have to clean it up here, as it will
+ * not get cleaned up in port_release() as it was never registered with
+ * the driver core */
+ if (serial->num_ports < serial->num_port_pointers) {
+ for (i = serial->num_ports; i < serial->num_port_pointers; ++i) {
+ port = serial->port[i];
+ if (!port)
+ continue;
+ if (port->read_urb) {
+ usb_unlink_urb(port->read_urb);
+ usb_free_urb(port->read_urb);
+ }
+ if (port->write_urb) {
+ usb_unlink_urb(port->write_urb);
+ usb_free_urb(port->write_urb);
+ }
+ if (port->interrupt_in_urb) {
+ usb_unlink_urb(port->interrupt_in_urb);
+ usb_free_urb(port->interrupt_in_urb);
+ }
+ kfree(port->bulk_in_buffer);
+ kfree(port->bulk_out_buffer);
+ kfree(port->interrupt_in_buffer);
+ }
+ }
+
+ usb_put_dev(serial->dev);
+
+ /* free up any memory that we allocated */
+ kfree (serial);
+}
+
/*****************************************************************************
* Driver tty interface functions
*****************************************************************************/
@@ -487,7 +550,7 @@
if (retval) {
port->open_count = 0;
module_put(serial->type->owner);
- kref_put(&serial->kref);
+ kref_put(&serial->kref, destroy_serial);
}
}
bailout:
@@ -518,7 +581,7 @@
}
module_put(port->serial->type->owner);
- kref_put(&port->serial->kref);
+ kref_put(&port->serial->kref, destroy_serial);
}
static int serial_write (struct tty_struct * tty, int from_user, const unsigned char *buf, int count)
@@ -676,13 +739,6 @@
;
}
-static void serial_shutdown (struct usb_serial *serial)
-{
- dbg ("%s", __FUNCTION__);
-
- serial->type->shutdown(serial);
-}
-
static int serial_read_proc (char *page, char **start, off_t off, int count, int *eof, void *data)
{
struct usb_serial *serial;
@@ -716,7 +772,7 @@
begin += length;
length = 0;
}
- kref_put(&serial->kref);
+ kref_put(&serial->kref, destroy_serial);
}
*eof = 1;
done:
@@ -785,62 +841,6 @@
wake_up_interruptible(&tty->write_wait);
}
-static void destroy_serial(struct kref *kref)
-{
- struct usb_serial *serial;
- struct usb_serial_port *port;
- int i;
-
- serial = to_usb_serial(kref);
-
- dbg ("%s - %s", __FUNCTION__, serial->type->name);
- serial_shutdown (serial);
-
- /* return the minor range that this device had */
- return_serial(serial);
-
- for (i = 0; i < serial->num_ports; ++i)
- serial->port[i]->open_count = 0;
-
- /* the ports are cleaned up and released in port_release() */
- for (i = 0; i < serial->num_ports; ++i)
- if (serial->port[i]->dev.parent != NULL) {
- device_unregister(&serial->port[i]->dev);
- serial->port[i] = NULL;
- }
-
- /* If this is a "fake" port, we have to clean it up here, as it will
- * not get cleaned up in port_release() as it was never registered with
- * the driver core */
- if (serial->num_ports < serial->num_port_pointers) {
- for (i = serial->num_ports; i < serial->num_port_pointers; ++i) {
- port = serial->port[i];
- if (!port)
- continue;
- if (port->read_urb) {
- usb_unlink_urb(port->read_urb);
- usb_free_urb(port->read_urb);
- }
- if (port->write_urb) {
- usb_unlink_urb(port->write_urb);
- usb_free_urb(port->write_urb);
- }
- if (port->interrupt_in_urb) {
- usb_unlink_urb(port->interrupt_in_urb);
- usb_free_urb(port->interrupt_in_urb);
- }
- kfree(port->bulk_in_buffer);
- kfree(port->bulk_out_buffer);
- kfree(port->interrupt_in_buffer);
- }
- }
-
- usb_put_dev(serial->dev);
-
- /* free up any memory that we allocated */
- kfree (serial);
-}
-
static void port_release(struct device *dev)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
@@ -881,7 +881,7 @@
serial->interface = interface;
serial->vendor = dev->descriptor.idVendor;
serial->product = dev->descriptor.idProduct;
- kref_init(&serial->kref, destroy_serial);
+ kref_init(&serial->kref);
return serial;
}
@@ -1231,7 +1231,7 @@
if (serial) {
/* let the last holder of this object
* cause it to be cleaned up */
- kref_put(&serial->kref);
+ kref_put(&serial->kref, destroy_serial);
}
dev_info(dev, "device disconnected\n");
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] kref shrinkage patches -- 1 of 2 -- kref shrinkage
2004-07-20 12:23 [patch] kref shrinkage patches -- 1 of 2 -- kref shrinkage Ravikiran G Thirumalai
2004-07-20 12:27 ` [patch] kref shrinkage patches -- 2 " Ravikiran G Thirumalai
@ 2004-07-21 6:14 ` Greg KH
2004-07-21 14:34 ` Ravikiran G Thirumalai
1 sibling, 1 reply; 5+ messages in thread
From: Greg KH @ 2004-07-21 6:14 UTC (permalink / raw)
To: Ravikiran G Thirumalai; +Cc: linux-kernel
On Tue, Jul 20, 2004 at 05:53:11PM +0530, Ravikiran G Thirumalai wrote:
> Greg,
> Here's the first step towards getting to kref_get_rcu :)
> This patch is to shrink the kref object by removing the
> function pointer 'release' from the kref object, and modifying
> kref_init and kref_put interfaces so that kref_init will not take
> the release pointer anymore and kref_put will. Patch will probably talk
> better.
Nice patches, I've applied them with a few tweaks, as noted below.
> Just had a question about definition of kref_get though,
> why does it need to return struct kref * ? struct kref * is anywayz
> being passed to it, hence the caller has it anywayz -- so it doesn't
> value add anything afaics (but i might have limited vision so pls correct me)
Consistancy with other "get" and "put" functions. It's quite common to
do:
have_this_foo(foo_get(foo));
or:
foo_to_send_off = foo_get(foo);
> In fact, I see that the scsi applications of kref have code like
>
> <quote drivers/scsi/sd.c>
>
> if (!kref_get(&sdkp->kref))
> goto out_sdkp;
>
> </>
Heh, yeah, that's just dumb, as that failure will never happen :)
> diff -ruN -X dontdiff2 linux-2.6.7/include/linux/kref.h kref-2.6.7/include/linux/kref.h
> --- linux-2.6.7/include/linux/kref.h 2004-06-16 10:48:59.000000000 +0530
> +++ kref-2.6.7/include/linux/kref.h 2004-07-20 12:53:24.226739304 +0530
> @@ -8,6 +8,9 @@
> * Copyright (C) 2002-2003 Patrick Mochel <mochel@osdl.org>
> * Copyright (C) 2002-2003 Open Source Development Labs
> *
> + * 07/2004 - struct kref shrinkage by Ravikiran Thirumalai <kiran@in.ibm.com>
> + * Copyright (C) 2004 IBM Corp.
> + *
> * This file is released under the GPLv2.
> *
> */
This is not needed. First off, the Copyright statement for IBM is
already included in this file, for this same year, above these lines.
Also, we are not including file history in the files themselves, that's
what Changelog comments are for (which scale much better over time.)
> @@ -44,14 +42,17 @@
> /**
> * kref_put - decrement refcount for object.
> * @kref: object.
> - *
> - * Decrement the refcount, and if 0, call kref->release().
> + * @release: pointer to the function that will clean up the object
> + * when the last reference to the object is released.
> + * This pointer is required.
> + * Decrement the refcount, and if 0, call release().
> */
The extra blank line in tha comment was needed for docbook to work
properly, last time I checked.
> -void kref_put(struct kref *kref)
> +void kref_put(struct kref *kref, void (*release) (struct kref *kref))
> {
> + WARN_ON(release == NULL);
I also added a check here to test if release == kfree to catch people
like the s390 developers who want to do nasty hacks :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread