From 943e1b4d60dc7acfabe2ebad31189bcb3b853688 Mon Sep 17 00:00:00 2001 From: Franck Bui-Huu Date: Wed, 14 Jun 2006 10:29:21 +0200 Subject: [PATCH] USB: gadget-serial: fix a deadlock when closing the serial device When closing the device, the driver acquires/release twice the port lock before/after waiting for the data to be completely sent. Therefore it will dead lock. This patch fixes it and also uses the generic scheduler services for waiting for an event. Signed-off-by: Franck Bui-Huu Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/serial.c | 95 ++++++++------------------------------------- 1 file changed, 17 insertions(+), 78 deletions(-) (limited to 'drivers/usb/gadget/serial.c') diff --git a/drivers/usb/gadget/serial.c b/drivers/usb/gadget/serial.c index b992546c394d..b58f015554b7 100644 --- a/drivers/usb/gadget/serial.c +++ b/drivers/usb/gadget/serial.c @@ -51,82 +51,10 @@ #include "gadget_chips.h" -/* Wait Cond */ - -#define __wait_cond_interruptible(wq, condition, lock, flags, ret) \ -do { \ - wait_queue_t __wait; \ - init_waitqueue_entry(&__wait, current); \ - \ - add_wait_queue(&wq, &__wait); \ - for (;;) { \ - set_current_state(TASK_INTERRUPTIBLE); \ - if (condition) \ - break; \ - if (!signal_pending(current)) { \ - spin_unlock_irqrestore(lock, flags); \ - schedule(); \ - spin_lock_irqsave(lock, flags); \ - continue; \ - } \ - ret = -ERESTARTSYS; \ - break; \ - } \ - current->state = TASK_RUNNING; \ - remove_wait_queue(&wq, &__wait); \ -} while (0) - -#define wait_cond_interruptible(wq, condition, lock, flags) \ -({ \ - int __ret = 0; \ - if (!(condition)) \ - __wait_cond_interruptible(wq, condition, lock, flags, \ - __ret); \ - __ret; \ -}) - -#define __wait_cond_interruptible_timeout(wq, condition, lock, flags, \ - timeout, ret) \ -do { \ - signed long __timeout = timeout; \ - wait_queue_t __wait; \ - init_waitqueue_entry(&__wait, current); \ - \ - add_wait_queue(&wq, &__wait); \ - for (;;) { \ - set_current_state(TASK_INTERRUPTIBLE); \ - if (__timeout == 0) \ - break; \ - if (condition) \ - break; \ - if (!signal_pending(current)) { \ - spin_unlock_irqrestore(lock, flags); \ - __timeout = schedule_timeout(__timeout); \ - spin_lock_irqsave(lock, flags); \ - continue; \ - } \ - ret = -ERESTARTSYS; \ - break; \ - } \ - current->state = TASK_RUNNING; \ - remove_wait_queue(&wq, &__wait); \ -} while (0) - -#define wait_cond_interruptible_timeout(wq, condition, lock, flags, \ - timeout) \ -({ \ - int __ret = 0; \ - if (!(condition)) \ - __wait_cond_interruptible_timeout(wq, condition, lock, \ - flags, timeout, __ret); \ - __ret; \ -}) - - /* Defines */ -#define GS_VERSION_STR "v2.0" -#define GS_VERSION_NUM 0x0200 +#define GS_VERSION_STR "v2.1" +#define GS_VERSION_NUM 0x0201 #define GS_LONG_NAME "Gadget Serial" #define GS_SHORT_NAME "g_serial" @@ -843,6 +771,18 @@ exit_unlock_dev: /* * gs_close */ + +#define GS_WRITE_FINISHED_EVENT_SAFELY(p) \ +({ \ + unsigned long flags; \ + int cond; \ + \ + spin_lock_irqsave(&(p)->port_lock, flags); \ + cond = !(p)->port_dev || !gs_buf_data_avail((p)->port_write_buf); \ + spin_unlock_irqrestore(&(p)->port_lock, flags); \ + cond; \ +}) + static void gs_close(struct tty_struct *tty, struct file *file) { unsigned long flags; @@ -888,10 +828,9 @@ static void gs_close(struct tty_struct *tty, struct file *file) /* at most GS_CLOSE_TIMEOUT seconds */ if (gs_buf_data_avail(port->port_write_buf) > 0) { spin_unlock_irqrestore(&port->port_lock, flags); - wait_cond_interruptible_timeout(port->port_write_wait, - port->port_dev == NULL - || gs_buf_data_avail(port->port_write_buf) == 0, - &port->port_lock, flags, GS_CLOSE_TIMEOUT * HZ); + wait_event_interruptible_timeout(port->port_write_wait, + GS_WRITE_FINISHED_EVENT_SAFELY(port), + GS_CLOSE_TIMEOUT * HZ); spin_lock_irqsave(&port->port_lock, flags); } -- cgit v1.2.3 From ca094f1186ef50ef8983325072cdc4f051830f13 Mon Sep 17 00:00:00 2001 From: Franck Bui-Huu Date: Wed, 14 Jun 2006 10:47:18 +0200 Subject: [PATCH] USB: gadget-serial: do not save/restore IRQ flags in gs_close() As pointed out by David Brownell, we know that IRQs are never blocked when calling gs_close function. So the save/restore IRQ flags are pointless. Signed-off-by: Franck Bui-Huu Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/serial.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'drivers/usb/gadget/serial.c') diff --git a/drivers/usb/gadget/serial.c b/drivers/usb/gadget/serial.c index b58f015554b7..e477edd681d3 100644 --- a/drivers/usb/gadget/serial.c +++ b/drivers/usb/gadget/serial.c @@ -53,8 +53,8 @@ /* Defines */ -#define GS_VERSION_STR "v2.1" -#define GS_VERSION_NUM 0x0201 +#define GS_VERSION_STR "v2.2" +#define GS_VERSION_NUM 0x0202 #define GS_LONG_NAME "Gadget Serial" #define GS_SHORT_NAME "g_serial" @@ -774,18 +774,16 @@ exit_unlock_dev: #define GS_WRITE_FINISHED_EVENT_SAFELY(p) \ ({ \ - unsigned long flags; \ int cond; \ \ - spin_lock_irqsave(&(p)->port_lock, flags); \ + spin_lock_irq(&(p)->port_lock); \ cond = !(p)->port_dev || !gs_buf_data_avail((p)->port_write_buf); \ - spin_unlock_irqrestore(&(p)->port_lock, flags); \ + spin_unlock_irq(&(p)->port_lock); \ cond; \ }) static void gs_close(struct tty_struct *tty, struct file *file) { - unsigned long flags; struct gs_port *port = tty->driver_data; struct semaphore *sem; @@ -799,7 +797,7 @@ static void gs_close(struct tty_struct *tty, struct file *file) sem = &gs_open_close_sem[port->port_num]; down(sem); - spin_lock_irqsave(&port->port_lock, flags); + spin_lock_irq(&port->port_lock); if (port->port_open_count == 0) { printk(KERN_ERR @@ -827,11 +825,11 @@ static void gs_close(struct tty_struct *tty, struct file *file) /* wait for write buffer to drain, or */ /* at most GS_CLOSE_TIMEOUT seconds */ if (gs_buf_data_avail(port->port_write_buf) > 0) { - spin_unlock_irqrestore(&port->port_lock, flags); + spin_unlock_irq(&port->port_lock); wait_event_interruptible_timeout(port->port_write_wait, GS_WRITE_FINISHED_EVENT_SAFELY(port), GS_CLOSE_TIMEOUT * HZ); - spin_lock_irqsave(&port->port_lock, flags); + spin_lock_irq(&port->port_lock); } /* free disconnected port on final close */ @@ -851,7 +849,7 @@ static void gs_close(struct tty_struct *tty, struct file *file) port->port_num, tty, file); exit: - spin_unlock_irqrestore(&port->port_lock, flags); + spin_unlock_irq(&port->port_lock); up(sem); } -- cgit v1.2.3 From a8c28f2389942bab376e39351d27525499630248 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Tue, 13 Jun 2006 09:57:47 -0700 Subject: [PATCH] USB: move to This moves to to reduce some of the clutter of usb header files. Signed-off-by: David Brownell Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/gadget/serial.c') diff --git a/drivers/usb/gadget/serial.c b/drivers/usb/gadget/serial.c index e477edd681d3..9d6e1d295528 100644 --- a/drivers/usb/gadget/serial.c +++ b/drivers/usb/gadget/serial.c @@ -45,7 +45,7 @@ #include #include -#include +#include #include #include "gadget_chips.h" -- cgit v1.2.3