Greg KH wrote:
> On Tue, Jun 06, 2006 at 10:31:51AM +0200, Franck Bui-Huu wrote:
>> ping
>
> I was waiting for an ack from the original author of this code. Al, any
> comments?
>
well, thinking more about it, I think the original author wrote its
own wait_event_xxx macros to test the event condition safely. So the
macros test the condition while lock is held and irqs are
blocked. Then before going to sleep it releases the lock and enable
irqs. And after the sleep it tries to acquire the lock again.
Therfore the macros assumes that the lock is held by the caller.
But the current code seems broken since before calling the redefined
wait_event_xxx macros it releases the lock and right after that call
it tries to hold the lock which is previously taken by the macro. Thus
the deadlock.
My current patch only removes the deadlock but the condition is tested
unsafely...
So maybe the right thing to do is:
--- a/drivers/usb/gadget/serial.c
+++ b/drivers/usb/gadget/serial.c
@@ -887,12 +887,10 @@ static void gs_close(struct tty_struct *
/* 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);
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);
- spin_lock_irqsave(&port->port_lock, flags);
}
/* free disconnected port on final close */
But we still use these duplicated macros...
If all is correct I'm going to send you a patch which will try to fix
the dead lock _and_ remove these macros.
What do you think ?
Thanks
Franck
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
