Am Freitag, 16. März 2007 00:02 schrieb Greg KH:
> > > However, shouldn't we just be calling "shutdown" here instead? Lots of
> >
> > Currently the locking is not good enough to call shutdown() so early.
> > It could be strengthened. Make your choice. I can and will do it
> > either way. Just the current state of affairs is bad.
>
> I agree, but we still need the shutdown() function to free the memory.
If but only if we touch the subdriver after shutdown(). If we can avoid
that, shutdown() can be called in disconnect(). Here's an untested patch
to do just that. I'll test it tomorrow.
The principal approach is to never call down into the subdriver after
a device is known disconnected, then to wait for all ongoing operations
to finish and then call shutdown.
Regards
Oliver
--- linux-2.6.20/drivers/usb/serial/usb-serial.c 2007-03-16
01:32:38.000000000 +0100
+++ linux-2.6.21-rc3-git7/drivers/usb/serial/usb-serial.c 2007-03-16
01:26:28.000000000 +0100
@@ -59,8 +59,10 @@
static int debug;
static struct usb_serial *serial_table[SERIAL_TTY_MINORS]; /* initially
all NULL */
+static u32 deathrow[SERIAL_TTY_MINORS/32]; /* disconnected
but open*/
static spinlock_t table_lock;
static LIST_HEAD(usb_serial_driver_list);
+static DECLARE_WAIT_QUEUE_HEAD(usbserial_henchman);
struct usb_serial *usb_serial_get_by_index(unsigned index)
{
@@ -104,6 +106,7 @@
for (i = *minor; (i < (*minor + num_ports)) && (i <
SERIAL_TTY_MINORS); ++i) {
serial_table[i] = serial;
serial->port[j++]->number = i;
+ __clear_bit(i % 32, &deathrow[i / 32]);
}
spin_unlock(&table_lock);
return serial;
@@ -112,6 +115,54 @@
return NULL;
}
+/* requires locking from caller*/
+static int on_deathrow(struct usb_serial_port *port)
+{
+ int number = port->number;
+
+ return deathrow[number / 32] & (1 << (number % 32));
+}
+
+/* requires locking from caller*/
+static void schedule_execution(struct usb_serial_port *port)
+{
+ int number = port->number;
+
+ deathrow[number / 32] |= (1 << (number % 32));
+}
+
+static int lock_serial(struct usb_serial_port *port)
+{
+ struct usb_serial *serial = port->serial;
+ unsigned long flags;
+ int terminated;
+
+ spin_lock_irqsave(&serial->lock, flags);
+ terminated = on_deathrow(port);
+ if (!terminated)
+ serial->busy++;
+ spin_unlock_irqrestore(&serial->lock, flags);
+
+ if (terminated)
+ return -ENODEV;
+ return 0;
+}
+
+static void unlock_serial(struct usb_serial_port *port)
+{
+ struct usb_serial *serial = port->serial;
+ unsigned long flags;
+ int waiters;
+
+ spin_lock_irqsave(&serial->lock, flags);
+ serial->busy--;
+ waiters = serial->busy;
+ spin_unlock_irqrestore(&serial->lock, flags);
+
+ if (!waiters)
+ wake_up(&usbserial_henchman);
+}
+
static void return_serial(struct usb_serial *serial)
{
int i;
@@ -148,9 +199,6 @@
serial->port[i] = NULL;
}
- if (serial->type->shutdown)
- serial->type->shutdown(serial);
-
/* return the minor range that this device had */
return_serial(serial);
@@ -203,9 +251,13 @@
goto bailout_kref_put;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto bailout_kref_put;
+
if (mutex_lock_interruptible(&port->mutex)) {
retval = -ERESTARTSYS;
- goto bailout_kref_put;
+ goto bailout_kref_put_unlock;
}
++port->open_count;
@@ -233,6 +285,7 @@
}
mutex_unlock(&port->mutex);
+ unlock_serial(port);
return 0;
bailout_module_put:
@@ -242,6 +295,8 @@
tty->driver_data = NULL;
port->tty = NULL;
mutex_unlock(&port->mutex);
+bailout_kref_put_unlock:
+ unlock_serial(port);
bailout_kref_put:
usb_serial_put(serial);
return retval;
@@ -298,9 +353,15 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
+
/* pass on to the driver specific version of this function */
retval = port->serial->type->write(port, buf, count);
+ unlock_serial(port);
+
exit:
return retval;
}
@@ -320,9 +381,14 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
/* pass on to the driver specific version of this function */
retval = port->serial->type->write_room(port);
+ unlock_serial(port);
+
exit:
return retval;
}
@@ -342,9 +408,13 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
/* pass on to the driver specific version of this function */
retval = port->serial->type->chars_in_buffer(port);
+ unlock_serial(port);
exit:
return retval;
}
@@ -364,8 +434,12 @@
}
/* pass on to the driver specific version of this function */
- if (port->serial->type->throttle)
+ if (port->serial->type->throttle) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->throttle(port);
+ unlock_serial(port);
+ }
}
static void serial_unthrottle (struct tty_struct * tty)
@@ -383,8 +457,12 @@
}
/* pass on to the driver specific version of this function */
- if (port->serial->type->unthrottle)
+ if (port->serial->type->unthrottle) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->unthrottle(port);
+ unlock_serial(port);
+ }
}
static int serial_ioctl (struct tty_struct *tty, struct file * file, unsigned
int cmd, unsigned long arg)
@@ -402,12 +480,16 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
/* pass on to the driver specific version of this function if it is
available */
if (port->serial->type->ioctl)
retval = port->serial->type->ioctl(port, file, cmd, arg);
else
retval = -ENOIOCTLCMD;
-
+
+ unlock_serial(port);
exit:
return retval;
}
@@ -427,8 +509,12 @@
}
/* pass on to the driver specific version of this function if it is
available */
- if (port->serial->type->set_termios)
+ if (port->serial->type->set_termios) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->set_termios(port, old);
+ unlock_serial(port);
+ }
}
static void serial_break (struct tty_struct *tty, int break_state)
@@ -446,8 +532,12 @@
}
/* pass on to the driver specific version of this function if it is
available */
- if (port->serial->type->break_ctl)
+ if (port->serial->type->break_ctl) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->break_ctl(port, break_state);
+ unlock_serial(port);
+ }
}
static int serial_read_proc (char *page, char **start, off_t off, int count,
int *eof, void *data)
@@ -500,6 +590,7 @@
static int serial_tiocmget (struct tty_struct *tty, struct file *file)
{
struct usb_serial_port *port = tty->driver_data;
+ int retval;
if (!port)
return -ENODEV;
@@ -511,16 +602,26 @@
return -ENODEV;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
+
if (port->serial->type->tiocmget)
- return port->serial->type->tiocmget(port, file);
+ retval = port->serial->type->tiocmget(port, file);
+ else
+ retval = -EINVAL;
+
+ unlock_serial(port);
+exit:
- return -EINVAL;
+ return retval;
}
static int serial_tiocmset (struct tty_struct *tty, struct file *file,
unsigned int set, unsigned int clear)
{
struct usb_serial_port *port = tty->driver_data;
+ int retval;
if (!port)
return -ENODEV;
@@ -532,10 +633,18 @@
return -ENODEV;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
+
if (port->serial->type->tiocmset)
- return port->serial->type->tiocmset(port, file, set, clear);
+ retval = port->serial->type->tiocmset(port, file, set, clear);
+ else
+ retval = -EINVAL;
- return -EINVAL;
+ unlock_serial(port);
+exit:
+ return retval;
}
/*
@@ -612,6 +721,7 @@
serial->type = driver;
serial->interface = interface;
kref_init(&serial->kref);
+ spin_lock_init(&serial->lock);
return serial;
}
@@ -1058,12 +1168,20 @@
if (serial) {
for (i = 0; i < serial->num_ports; ++i) {
port = serial->port[i];
+ spin_lock_irq(&serial->lock);
+ schedule_execution(port);
+ spin_unlock_irq(&serial->lock);
if (port) {
if (port->tty)
tty_hangup(port->tty);
kill_traffic(port);
}
}
+ wait_event(usbserial_henchman, serial->busy == 0);
+ /* allow the subdriver to cleanly kill
+ * private URBs */
+ if (serial->type->shutdown)
+ (serial->type->shutdown)(serial);
/* let the last holder of this object
* cause it to be cleaned up */
usb_serial_put(serial);
--- linux-2.6.20/include/linux/usb/serial.h 2007-03-16 01:32:25.000000000
+0100
+++ linux-2.6.21-rc3-git7/include/linux/usb/serial.h 2007-03-16
01:20:55.000000000 +0100
@@ -128,6 +128,7 @@
struct usb_device * dev;
struct usb_serial_driver * type;
struct usb_interface * interface;
+ spinlock_t lock;
unsigned char minor;
unsigned char num_ports;
unsigned char num_port_pointers;
@@ -135,6 +136,7 @@
char num_interrupt_out;
char num_bulk_in;
char num_bulk_out;
+ char busy;
struct usb_serial_port * port[MAX_NUM_PORTS];
struct kref kref;
void * private;
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
