On Mon, 12 Oct 2009 14:31:54 -0700
Greg KH <greg@xxxxxxxxx> wrote:
> On Mon, Oct 12, 2009 at 10:51:07PM +0200, Richard Zidlicky wrote:
> > On Mon, Oct 12, 2009 at 01:23:50PM -0700, Greg KH wrote:
> > > On Mon, Oct 12, 2009 at 09:56:29PM +0200, Richard Zidlicky wrote:
> > > > Hello,
> > > >
> > > > everything worked nicely untill I happened to hit this one - looked
> > > > scary enough that I rebooted on next occassion.
> > >
> > > What kernel version are you using?
> >
> > 2.6.31.3
> >
> > >
> > > Can you enable usb-serial debugging to see if you can reproduce this?
> >
> > working on it... hm I am see I have CONFIG_USB_SERIAL_DEBUG=m, is modprobe
> > enough to enable it?
>
> 'modprobe usb-serial debug=1' will do it.
>
> And don't compress the output, it's a pain when you do that...
Looks like a bogus test
The easiest sequence to create it is
enable echo
make the remote end spew data
close the port
at this point during the close down we get data received, which n_tty
echoes back which calls ->write_room() which seems port->count = 0 and
which then barfs.
As the tty is now refcounted I believe the WARN_ON() can simply go. Ditto
in serial_write although there the window is tiny because we do
if (port->serial->dev->state == USB_STATE_NOTATTACHED)
goto exit;
dbg("%s - port %d, %d byte(s)", __func__, port->number, count);
/* count is managed under the mutex lock for the tty so cannot
drop to zero until after the last close completes */
WARN_ON(!port->port.count);
/* pass on to the driver specific version of this function */
retval = port->serial->type->write(tty, port, buf, count);
One way to simplify all the logic here would be to apply the
tty_port_close patch I did based on Alan Stern's suggestions. At that
point shutdown/startup are within port->mutex which in turn means that we
can do this for the operations removing a ton of horrible cases from the
lower level drivers
mutex_lock(&port->mutex);
if (test_bit(ASYNCB_INITIALIZED, &port->flags))
port->ops->whatever(...);
else {
/* default if different */
}
mutex_unlock(&port->mutex);
although someone would have to sort out any interactions with the weird
and wonderful world of USB autopm/suspend/resume paths.
The uart layer does pretty much that, but using its own different locks
at this point in time.
(I don't btw think its unreasonable to argue that this chunk of sanity
belongs in the core tty layer if every interface that attaches to a
tty gets to include a tty port. It would dramatically simplify a lot of
the driver writing work if we did so)
--
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
