ChangeSet 1.1043.1.25, 2003/02/18 16:03:32-08:00, greg@kroah.com [PATCH] USB serial: fix locking logic This gets rid of the port semaphore, and the serialization caused by that, and replaces it with the proper reference counting logic for the usb serial object. diff -Nru a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c --- a/drivers/usb/serial/console.c Tue Feb 18 16:36:51 2003 +++ b/drivers/usb/serial/console.c Tue Feb 18 16:36:51 2003 @@ -141,7 +141,6 @@ } port = &serial->port[0]; - down (&port->sem); port->tty = NULL; info->port = port; @@ -158,8 +157,6 @@ port->open_count = 0; } - up (&port->sem); - if (retval) { err ("could not open USB console port"); return retval; @@ -208,8 +205,6 @@ if (count == 0) return; - down (&port->sem); - dbg("%s - port %d, %d byte(s)", __FUNCTION__, port->number, count); if (!port->open_count) { @@ -224,7 +219,6 @@ retval = usb_serial_generic_write(port, 0, buf, count); exit: - up (&port->sem); dbg("%s - return value (if we had one): %d", __FUNCTION__, retval); } diff -Nru a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c --- a/drivers/usb/serial/usb-serial.c Tue Feb 18 16:36:51 2003 +++ b/drivers/usb/serial/usb-serial.c Tue Feb 18 16:36:51 2003 @@ -391,7 +391,11 @@ struct usb_serial *usb_serial_get_by_minor (unsigned int minor) { - return serial_table[minor]; + struct usb_serial *serial = serial_table[minor]; + + if (serial) + kobject_get (&serial->kobj); + return serial; } static struct usb_serial *get_free_serial (struct usb_serial *serial, int num_ports, unsigned int *minor) @@ -468,7 +472,6 @@ port = &serial->port[portNumber]; tty->driver_data = port; - down (&port->sem); port->tty = tty; /* lock this module before we call it, @@ -492,8 +495,6 @@ } } bailout: - - up (&port->sem); return retval; } @@ -516,6 +517,7 @@ } module_put(port->serial->type->owner); + kobject_put(&port->serial->kobj); } static void serial_close(struct tty_struct *tty, struct file * filp) @@ -526,16 +528,12 @@ if (!serial) return; - down (&port->sem); - dbg("%s - port %d", __FUNCTION__, port->number); /* if disconnect beat us to the punch here, there's nothing to do */ if (tty->driver_data) { __serial_close(port, filp); } - - up (&port->sem); } static int serial_write (struct tty_struct * tty, int from_user, const unsigned char *buf, int count) @@ -547,8 +545,6 @@ if (!serial) return -ENODEV; - down (&port->sem); - dbg("%s - port %d, %d byte(s)", __FUNCTION__, port->number, count); if (!port->open_count) { @@ -563,7 +559,6 @@ retval = usb_serial_generic_write(port, from_user, buf, count); exit: - up (&port->sem); return retval; } @@ -576,8 +571,6 @@ if (!serial) return -ENODEV; - down (&port->sem); - dbg("%s - port %d", __FUNCTION__, port->number); if (!port->open_count) { @@ -592,7 +585,6 @@ retval = usb_serial_generic_write_room(port); exit: - up (&port->sem); return retval; } @@ -605,8 +597,6 @@ if (!serial) return -ENODEV; - down (&port->sem); - dbg("%s = port %d", __FUNCTION__, port->number); if (!port->open_count) { @@ -621,7 +611,6 @@ retval = usb_serial_generic_chars_in_buffer(port); exit: - up (&port->sem); return retval; } @@ -633,8 +622,6 @@ if (!serial) return; - down (&port->sem); - dbg("%s - port %d", __FUNCTION__, port->number); if (!port->open_count) { @@ -647,7 +634,6 @@ serial->type->throttle(port); exit: - up (&port->sem); } static void serial_unthrottle (struct tty_struct * tty) @@ -658,8 +644,6 @@ if (!serial) return; - down (&port->sem); - dbg("%s - port %d", __FUNCTION__, port->number); if (!port->open_count) { @@ -672,7 +656,6 @@ serial->type->unthrottle(port); exit: - up (&port->sem); } static int serial_ioctl (struct tty_struct *tty, struct file * file, unsigned int cmd, unsigned long arg) @@ -684,8 +667,6 @@ if (!serial) return -ENODEV; - down (&port->sem); - dbg("%s - port %d, cmd 0x%.4x", __FUNCTION__, port->number, cmd); if (!port->open_count) { @@ -700,7 +681,6 @@ retval = -ENOIOCTLCMD; exit: - up (&port->sem); return retval; } @@ -712,8 +692,6 @@ if (!serial) return; - down (&port->sem); - dbg("%s - port %d", __FUNCTION__, port->number); if (!port->open_count) { @@ -726,7 +704,6 @@ serial->type->set_termios(port, old); exit: - up (&port->sem); } static void serial_break (struct tty_struct *tty, int break_state) @@ -737,8 +714,6 @@ if (!serial) return; - down (&port->sem); - dbg("%s - port %d", __FUNCTION__, port->number); if (!port->open_count) { @@ -751,7 +726,6 @@ serial->type->break_ctl(port, break_state); exit: - up (&port->sem); } static void serial_shutdown (struct usb_serial *serial) @@ -797,6 +771,7 @@ begin += length; length = 0; } + kobject_put(&serial->kobj); } *eof = 1; done: @@ -833,6 +808,75 @@ wake_up_interruptible(&tty->write_wait); } +static void destroy_serial (struct kobject *kobj) +{ + struct usb_serial *serial; + struct usb_serial_port *port; + int i; + + dbg ("%s", __FUNCTION__); + + serial = to_usb_serial(kobj); + + /* fail all future close/read/write/ioctl/etc calls */ + for (i = 0; i < serial->num_ports; ++i) { + port = &serial->port[i]; + if (port->tty != NULL) { + port->tty->driver_data = NULL; + while (port->open_count > 0) { + __serial_close(port, NULL); + } + } + } + + usb_put_dev(serial->dev); + serial->dev = NULL; + serial_shutdown (serial); + + for (i = 0; i < serial->num_ports; ++i) + device_unregister(&serial->port[i].dev); + + for (i = 0; i < serial->num_ports; ++i) + serial->port[i].open_count = 0; + + for (i = 0; i < serial->num_bulk_in; ++i) { + port = &serial->port[i]; + if (port->read_urb) { + usb_unlink_urb (port->read_urb); + usb_free_urb (port->read_urb); + } + if (port->bulk_in_buffer) + kfree (port->bulk_in_buffer); + } + for (i = 0; i < serial->num_bulk_out; ++i) { + port = &serial->port[i]; + if (port->write_urb) { + usb_unlink_urb (port->write_urb); + usb_free_urb (port->write_urb); + } + if (port->bulk_out_buffer) + kfree (port->bulk_out_buffer); + } + for (i = 0; i < serial->num_interrupt_in; ++i) { + port = &serial->port[i]; + if (port->interrupt_in_urb) { + usb_unlink_urb (port->interrupt_in_urb); + usb_free_urb (port->interrupt_in_urb); + } + if (port->interrupt_in_buffer) + kfree (port->interrupt_in_buffer); + } + /* return the minor range that this device had */ + return_serial (serial); + + /* free up any memory that we allocated */ + kfree (serial); +} + +static struct kobj_type usb_serial_kobj_type = { + .release = destroy_serial, +}; + static struct usb_serial * create_serial (struct usb_device *dev, struct usb_interface *interface, struct usb_serial_device_type *type) @@ -845,12 +889,16 @@ return NULL; } memset (serial, 0, sizeof(*serial)); - serial->dev = dev; + serial->dev = usb_get_dev(dev); serial->type = type; serial->interface = interface; serial->vendor = dev->descriptor.idVendor; serial->product = dev->descriptor.idProduct; + /* initialize the kobject portion of the usb_device */ + kobject_init(&serial->kobj); + serial->kobj.ktype = &usb_serial_kobj_type; + return serial; } @@ -1113,7 +1161,6 @@ port->serial = serial; port->magic = USB_SERIAL_PORT_MAGIC; INIT_WORK(&port->work, usb_serial_port_softint, port); - init_MUTEX (&port->sem); } /* if this device type has an attach function, call it */ @@ -1189,67 +1236,14 @@ { struct usb_serial *serial = usb_get_intfdata (interface); struct device *dev = &interface->dev; - struct usb_serial_port *port; - int i; dbg ("%s", __FUNCTION__); usb_set_intfdata (interface, NULL); if (serial) { - /* fail all future close/read/write/ioctl/etc calls */ - for (i = 0; i < serial->num_ports; ++i) { - port = &serial->port[i]; - down (&port->sem); - if (port->tty != NULL) { - port->tty->driver_data = NULL; - while (port->open_count > 0) { - __serial_close(port, NULL); - } - } - up (&port->sem); - } - - serial->dev = NULL; - serial_shutdown (serial); - - for (i = 0; i < serial->num_ports; ++i) - device_unregister(&serial->port[i].dev); - - for (i = 0; i < serial->num_ports; ++i) - serial->port[i].open_count = 0; - - for (i = 0; i < serial->num_bulk_in; ++i) { - port = &serial->port[i]; - if (port->read_urb) { - usb_unlink_urb (port->read_urb); - usb_free_urb (port->read_urb); - } - if (port->bulk_in_buffer) - kfree (port->bulk_in_buffer); - } - for (i = 0; i < serial->num_bulk_out; ++i) { - port = &serial->port[i]; - if (port->write_urb) { - usb_unlink_urb (port->write_urb); - usb_free_urb (port->write_urb); - } - if (port->bulk_out_buffer) - kfree (port->bulk_out_buffer); - } - for (i = 0; i < serial->num_interrupt_in; ++i) { - port = &serial->port[i]; - if (port->interrupt_in_urb) { - usb_unlink_urb (port->interrupt_in_urb); - usb_free_urb (port->interrupt_in_urb); - } - if (port->interrupt_in_buffer) - kfree (port->interrupt_in_buffer); - } - /* return the minor range that this device had */ - return_serial (serial); - - /* free up any memory that we allocated */ - kfree (serial); + /* let the last holder of this object + * cause it to be cleaned up */ + kobject_put (&serial->kobj); } dev_info(dev, "device disconnected\n"); } diff -Nru a/drivers/usb/serial/usb-serial.h b/drivers/usb/serial/usb-serial.h --- a/drivers/usb/serial/usb-serial.h Tue Feb 18 16:36:51 2003 +++ b/drivers/usb/serial/usb-serial.h Tue Feb 18 16:36:51 2003 @@ -89,7 +89,6 @@ * @write_wait: a wait_queue_head_t used by the port. * @work: work queue entry for the line discipline waking up. * @open_count: number of times this port has been opened. - * @sem: struct semaphore used to lock this structure. * * This structure is used by the usb-serial core and drivers for the specific * ports of a device. @@ -116,7 +115,6 @@ wait_queue_head_t write_wait; struct work_struct work; int open_count; - struct semaphore sem; struct device dev; }; #define to_usb_serial_port(d) container_of(d, struct usb_serial_port, dev) @@ -164,8 +162,10 @@ __u16 vendor; __u16 product; struct usb_serial_port port[MAX_NUM_PORTS]; + struct kobject kobj; void * private; }; +#define to_usb_serial(d) container_of(d, struct usb_serial, kobj) #define NUM_DONT_CARE (-1)