"USB: use kfifo to buffer usb-generic serial writes" causes gobi_loader to hang
in [Kernel]
|
Prev: [REGRESSION] "USB: use kfifo to buffer usb-generic serial writes" causes gobi_loader to hang
Next: "USB: use kfifo to buffer usb-generic serial writes" causes gobi_loader to hang
From: Marcel Holtmann on 17 Jan 2010 03:20 Hi Matthew, > gobi_loader writes firmware to gobi USB devices (qcserial) via the USB > serial layer by writing it a megabyte at a time (matching the Windows > driver behaviour). Commit 8e8dce065088833fc418bfa5fbf035cb0726c04c > breaks it - strace shows the write simply hanging. ssyrq-t says the > backtrace is: > > Jan 17 15:37:37 localhost kernel: Call Trace: > Jan 17 15:37:37 localhost kernel: [<ffffffff8128986c>] n_tty_write+0x2da/0x351 > Jan 17 15:37:37 localhost kernel: [<ffffffff810464b4>] ? default_wake_function+0x0/0x14 > Jan 17 15:37:37 localhost kernel: [<ffffffff81286969>] tty_write+0x19d/0x245 > Jan 17 15:37:37 localhost kernel: [<ffffffff81289592>] ? n_tty_write+0x0/0x351 > Jan 17 15:37:37 localhost kernel: [<ffffffff81103e44>] vfs_write+0xae/0x10b > Jan 17 15:37:37 localhost kernel: [<ffffffff81103f61>] sys_write+0x4a/0x6e > Jan 17 15:37:37 localhost kernel: [<ffffffff81009e45>] tracesys+0xd9/0xde > > The commit before this works fine. maybe I should have finished my libusb version of the gobi_loader ;) Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 19 Jan 2010 08:30 On Tue, 19 Jan 2010 14:20:29 +0100 Oliver Neukum <oliver(a)neukum.org> wrote: > Am Montag, 18. Januar 2010 21:14:16 schrieb Matthew Garrett: > > > drivers/usb/serial/generic.c: usb_serial_generic_write - port 0, 1565 > > bytes > > drivers/usb/serial/generic.c: usb_serial_generic_write - put 0 bytes > > into fifo > > drivers/usb/serial/generic.c: usb_serial_generic_read_bulk_callback - > > port 0 > > If the FIFO is full we can do nothing. Please try the attached patch. If the FIFO is full how are we getting called - we should have throttled earlier if there is any flow control (including 'look no URBs' cases) ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 19 Jan 2010 09:20 On Tue, 19 Jan 2010 14:57:00 +0100 Oliver Neukum <oliver(a)neukum.org> wrote: > Am Dienstag, 19. Januar 2010 14:29:34 schrieb Alan Cox: > > On Tue, 19 Jan 2010 14:20:29 +0100 > > Oliver Neukum <oliver(a)neukum.org> wrote: > > > > > Am Montag, 18. Januar 2010 21:14:16 schrieb Matthew Garrett: > > > > > > > drivers/usb/serial/generic.c: usb_serial_generic_write - port 0, 1565 > > > > bytes > > > > drivers/usb/serial/generic.c: usb_serial_generic_write - put 0 bytes > > > > into fifo > > > > drivers/usb/serial/generic.c: usb_serial_generic_read_bulk_callback - > > > > port 0 > > > > > > If the FIFO is full we can do nothing. Please try the attached patch. > > > > If the FIFO is full how are we getting called - we should have throttled > > earlier if there is any flow control (including 'look no URBs' cases) ? > > The log shows no call to usb_serial_generic_write_room() > Do you consider this a bug in the tty layer? Some of the line disciplines just throw data at the board but not n_tty (the normal one) - so for PPP you wouldn't see checks on room but the loader I assume is using the normal tty layer - if so something is up because the ldisc shouldn't have written more than there is room for. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 19 Jan 2010 09:30 > The log shows no call to usb_serial_generic_write_room() > Do you consider this a bug in the tty layer? Actually this all makes sense because of where it was hanging. A reply of 0 to the tty->ops->write will cause it to either return (O_NONBLOCK) or sleep in the n_tty write code waiting for a write_wait wakeup (tty_wakeup(tty)) So the fix does indeed look correct. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Johan Hovold on 19 Jan 2010 10:30
> > The log shows no call to usb_serial_generic_write_room() > > Do you consider this a bug in the tty layer? > > Actually this all makes sense because of where it was hanging. A reply of > 0 to the tty->ops->write will cause it to either return (O_NONBLOCK) or > sleep in the n_tty write code waiting for a write_wait wakeup > (tty_wakeup(tty)) > > So the fix does indeed look correct. Is it really a fix? If the fifo is already full the write urb should be in use and Oliver's patch would amount to only a minor optimisation as usb_serial_generic_write_start would return 0 anyway. /Johan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |