On Thu, Mar 14, 2013 at 01:39:39PM -0400, Peter Hurley wrote: > On Thu, 2013-03-14 at 16:23 +0100, Johan Hovold wrote: > > Make sure to unregister the tty-device before calling subdriver > > port_remove. > > > > This way remove will reverse probe, and specifically any port data > > released in port_remove will be available throughout tty unregister. > > > > Note that the order currently does not matter as the tty-layer can make > > callbacks also after the device has been unregistered. This is > > handled in usb-serial core using the disconnected flag, which is > > already set when usb-serial bus device remove is called. > > > > Cc: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> > > Reported-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> > > Signed-off-by: Johan Hovold <jhovold@xxxxxxxxx> > > Looks fine to me. > > The thing I did notice is that the ftdi usb serial mini-driver has its > own ioctl() handler for TIOCMIWAIT which can be holding the tty > reference open after the unregister. Yes, this is expected and perfectly fine. User-space may hold any tty-ref indefinitely, but we must unregister the tty-device at disconnect and then deal with it using the disconnected flag. > Its port_remove() method, ftdi_sio_port_remove(), does the wake up to > kick it off the loop. Yes, all is good (except for another use-after free bug in that very implementation that I fixed elsewhere). We hang up the port at disconnect, unregister the device from the bus, which calls ftdi port remove and wakes up any processes waiting on the delta_msr_wait queue, which then return immediately based on the disconnected flag (this last bit wasn't the case before the fix mentioned above but has nothing to do with tty unregister). > Now that the call order is reversed, that will need to be fixed. No, the call order in usb-serial bus remove does not change anything really. Any process doing TIOCMIWAIT will only be woken up slightly later (and again there is nothing wrong in keeping a tty ref after tty unregister). > A quick review of the usage seems like the whole ioctl() should be > torn out along with the private delta_msr_wait. Most usb-serial TIOCIMWAIT-implementations, including the ftdi one, are replaced by a generic implementation by this very series. Thanks, Johan -- 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
