On Fri, 21 Aug 2009, Alan Cox wrote: > > serial_open() /* first user */ > > tty_port_block_til_ready() /* returns immediately */ > > serial->type->open() /* initializes the hardware */ > > > > serial_open() /* second user */ > > tty_port_block_til_ready() /* blocks */ > > If the first user succeeded then second should open immediately as the use > count is >= 1 already. It is re-opening the open device, unless the > hangup occurs first. What about protecting the use counter? In tty_port.c it's always protected by port->lock, but not in serial_open(). Is that a mistake? How are hangups synchronized with opens? Do you rely on the BKL? Suppose a hangup occurs, and do_tty_hangup() marks all the existing file references with hung_up_tty_fops. But before it gets around to calling tty->ops->hangup(), another open occurs. I can't imagine the BKL will prevent this; do_tty_hangup() is so complex it must sleep somewhere. Thus it's possible for __tty_open() to call tty->ops->open() before tty->ops->hangup() is called, which means the open will succeed. The when the driver's hangup routine finally gets around to calling tty_port_hangup(), port->count will be set back to 0. So now we've got an uncounted open file. > Most drivers tend to look like > > open > test ASYNC_INITIALIZED > init hardware > [either in full or clean up partial] > set ASYNC_INITIALIZED) > any other alloc/counter magic > tty->private_data = my stuff tty->driver_data, right? > block_til_ready > return ok/error > > close > if (hung_up) > return > if (tty->driver_data == NULL) > return > counts Is "counts" shorthand for: if (tty_port_close_start(...) == 0) return ? > clean up resources > if (last && test_clear INITIALIZED) How do you check for "last"? Doesn't the fact that we are here mean that there are no remaining open references? > deinit-hardware > return ok/error > > hangup > if (initialized & test_clear INITIALIZED) { What is "initialized" supposed to be? Isn't INITIALIZED enough? > deinit hardware > } > > which is where I was trying to get the USB code. It doesn't look as though we're too far away. Thanks for the detailed explanations. Alan Stern P.S.: Consider this code in tty_port_block_til_ready(): /* if non-blocking mode is set we can pass directly to open unless the port has just hung up or is in another error state */ if ((filp->f_flags & O_NONBLOCK) || (tty->flags & (1 << TTY_IO_ERROR))) { port->flags |= ASYNC_NORMAL_ACTIVE; return 0; } The comment doesn't agree with the logic of the test. Which is wrong? -- 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