--- On Mon, 6/14/10, Jon Povey <jon.povey@xxxxxxxxxxxxxxx> wrote: > From: Jon Povey <jon.povey@xxxxxxxxxxxxxxx> Acked-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx> > Subject: [PATCH] USB: g_serial: don't set low_latency flag > To: linux-usb@xxxxxxxxxxxxxxx > Cc: "Jon Povey" <jon.povey@xxxxxxxxxxxxxxx>, chrisv@xxxxxxxxxxxxxxxxxx, david-b@xxxxxxxxxxx > Date: Monday, June 14, 2010, 3:41 AM > No longer set low_latency flag as it > causes this warning backtrace: > > WARNING: at kernel/mutex.c:207 > __mutex_lock_slowpath+0x6c/0x288() > > Fix associated locking and wakeups. > > Signed-off-by: Jon Povey <jon.povey@xxxxxxxxxxxxxxx> > --- > I just saw another patch has been submitted for this by > Maulik Mankad <x0082077@xxxxxx>. ... and another from Chris Verges <chrisv@xxxxxxxxxxxxxxxxxx> It never rains but it pours, as they say; this bug has been known for a while, and finally it seems to be the time to submit patches... we have a choice of three versions! :) > > This patch goes further with updated comments and wakeup > changes. FWIW I'm acking yours because of that wakeup change; it may be that bug fixes in the TTY layer have removed the need for that wakeup; ISTR the addition of that wakeup as being a bugfix, but it would of course be more natural to handle that in the TTY layer. Also, because you changed the one comment that described a locking issue rather than deleting the whole thing and its mention of that issue. > > Tested on TI Davinci DM355 (MUSB OTG) > > Assumptions: > > 1. Any callbacks are now done from a workqueue, so don't > need to > drop the lock any more > 2. As noted by Cliff Cai http://www.spinics.net/lists/linux-usb/msg26424.html > it is not appropriate to wake readers > here (tty code will do it if needed?) > > DB, please review.. > > Also found a bug in the cleanup routine which I will send > another patch for. > > drivers/usb/gadget/u_serial.c | 15 > ++------------- > 1 files changed, 2 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/gadget/u_serial.c > b/drivers/usb/gadget/u_serial.c > index ff10d66..3e8dcb5 100644 > --- a/drivers/usb/gadget/u_serial.c > +++ b/drivers/usb/gadget/u_serial.c > @@ -536,17 +536,11 @@ recycle: > > list_move(&req->list, &port->read_pool); > } > > - /* Push from tty to ldisc; this is > immediate with low_latency, and > - * may trigger callbacks to > this driver ... so drop the spinlock. > + /* Push from tty to ldisc; without > low_latency set this is handled by > + * a workqueue, so we won't > get callbacks and can hold port_lock > */ > if (tty && do_push) { > - > spin_unlock_irq(&port->port_lock); > > tty_flip_buffer_push(tty); > - > wake_up_interruptible(&tty->read_wait); > - > spin_lock_irq(&port->port_lock); > - > - /* tty may have been > closed */ > - tty = > port->port_tty; > } > > > @@ -784,11 +778,6 @@ static int gs_open(struct tty_struct > *tty, struct file *file) > port->open_count = 1; > port->openclose = false; > > - /* low_latency means ldiscs work in > tasklet context, without > - * needing a workqueue > schedule ... easier to keep up. > - */ > - tty->low_latency = 1; > - > /* if connected, start the I/O stream > */ > if (port->port_usb) { > struct > gserial *gser = port->port_usb; > -- > 1.6.3.3 > > -- > To unsubscribe from this list: send the line "unsubscribe > linux-usb" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
