* Peter Hurley | 2013-02-05 15:20:15 [-0500]:
> Please re-test with your dummy_hcd/g_nokia testcase, although
> I'm not convinced that usb gadget is using tty_hangup() appropriately.
> tty drivers use this for async carrier loss coming from an IRQ
> which will be disabled if the tty has been shutdown. Does gserial
> prevent async hangup to a dead tty in a similar fashion?
Not sure I understood. tty_hangup() is only called from within
gserial_disconnect() which calls right after usb_ep_disable(). After
usb_ep_disable() no further serial packets can be received until the
endpoints are re-enabled. This happens in gserial_connect().
Sebastian
[--cc Alan Cox]
On Tue, 2013-03-05 at 21:50 +0100, Sebastian Andrzej Siewior wrote:
> * Peter Hurley | 2013-02-05 15:20:15 [-0500]:
>
> > Please re-test with your dummy_hcd/g_nokia testcase, although
> > I'm not convinced that usb gadget is using tty_hangup() appropriately.
> > tty drivers use this for async carrier loss coming from an IRQ
> > which will be disabled if the tty has been shutdown. Does gserial
> > prevent async hangup to a dead tty in a similar fashion?
>
> Not sure I understood. tty_hangup() is only called from within
> gserial_disconnect() which calls right after usb_ep_disable(). After
> usb_ep_disable() no further serial packets can be received until the
> endpoints are re-enabled. This happens in gserial_connect().
That's why I asked. There are two potential issues:
First, tty_hangup() is asynchronous -- ie., it returns immediately. It
does not wait for the tty device to actually perform the hangup. So if
the gadget layers start cleanup immediately after, expecting that they
won't get a flurry of tty calls, that would be bad.
tty_vhangup() is synchronous -- ie., you wait while it cleans up. This
is what the usb serial core does on it's disconnect() method. But I
didn't research further if the circumstances were the same.
Second, when the hangup actually does run -- in __tty_hangup() -- it
expects the tty to exist. I didn't go looking through the gadget layers
to see if the tty was disposed some other way, which might race the
asynchronous tty hangup.
Thanks,
Peter Hurley
On 03/05/2013 11:20 PM, Peter Hurley wrote:
> [--cc Alan Cox]
>
> On Tue, 2013-03-05 at 21:50 +0100, Sebastian Andrzej Siewior wrote:
>> * Peter Hurley | 2013-02-05 15:20:15 [-0500]:
>>
>>> Please re-test with your dummy_hcd/g_nokia testcase, although
>>> I'm not convinced that usb gadget is using tty_hangup() appropriately.
>>> tty drivers use this for async carrier loss coming from an IRQ
>>> which will be disabled if the tty has been shutdown. Does gserial
>>> prevent async hangup to a dead tty in a similar fashion?
>>
>> Not sure I understood. tty_hangup() is only called from within
>> gserial_disconnect() which calls right after usb_ep_disable(). After
>> usb_ep_disable() no further serial packets can be received until the
>> endpoints are re-enabled. This happens in gserial_connect().
>
> That's why I asked. There are two potential issues:
>
> First, tty_hangup() is asynchronous -- ie., it returns immediately. It
> does not wait for the tty device to actually perform the hangup. So if
> the gadget layers start cleanup immediately after, expecting that they
> won't get a flurry of tty calls, that would be bad.
Sorry, I missed what driver is this?
> tty_vhangup() is synchronous -- ie., you wait while it cleans up. This
> is what the usb serial core does on it's disconnect() method. But I
> didn't research further if the circumstances were the same.
Even when tty_vhangup returns, it does not guarantee a closed tty. And
it also does not guarantee that any of tty->ops won't be called. The
latter is true only for devices that can be consoles. (For those,
file->ops are not redirected.) In that case one needs to wait for
port->count to become 0.
--
js
suse labs
[now this discussion has turned to usb gadget
+cc Felipe Balbi, linux-usb, -cc Dave Jones, Ilya Zykov]
On Tue, 2013-03-05 at 23:39 +0100, Jiri Slaby wrote:
> On 03/05/2013 11:20 PM, Peter Hurley wrote:
> > [--cc Alan Cox]
> >
> > On Tue, 2013-03-05 at 21:50 +0100, Sebastian Andrzej Siewior wrote:
> >> * Peter Hurley | 2013-02-05 15:20:15 [-0500]:
> >>
> >>> Please re-test with your dummy_hcd/g_nokia testcase, although
> >>> I'm not convinced that usb gadget is using tty_hangup() appropriately.
> >>> tty drivers use this for async carrier loss coming from an IRQ
> >>> which will be disabled if the tty has been shutdown. Does gserial
> >>> prevent async hangup to a dead tty in a similar fashion?
> >>
> >> Not sure I understood. tty_hangup() is only called from within
> >> gserial_disconnect() which calls right after usb_ep_disable(). After
> >> usb_ep_disable() no further serial packets can be received until the
> >> endpoints are re-enabled. This happens in gserial_connect().
> >
> > That's why I asked. There are two potential issues:
> >
> > First, tty_hangup() is asynchronous -- ie., it returns immediately. It
> > does not wait for the tty device to actually perform the hangup. So if
> > the gadget layers start cleanup immediately after, expecting that they
> > won't get a flurry of tty calls, that would be bad.
>
> Sorry, I missed what driver is this?
g_serial.
drivers/usb/gadget/u_serial.c
> > tty_vhangup() is synchronous -- ie., you wait while it cleans up. This
> > is what the usb serial core does on it's disconnect() method. But I
> > didn't research further if the circumstances were the same.
>
> Even when tty_vhangup returns, it does not guarantee a closed tty. And
> it also does not guarantee that any of tty->ops won't be called. The
> latter is true only for devices that can be consoles. (For those,
> file->ops are not redirected.) In that case one needs to wait for
> port->count to become 0.
Perhaps I was oversimplifying.
But my point was I doubt usb gadget is conducting its teardown safely
wrt tty.
Regards,
Peter Hurley
* Peter Hurley | 2013-03-05 17:20:29 [-0500]:
>> Not sure I understood. tty_hangup() is only called from within
>> gserial_disconnect() which calls right after usb_ep_disable(). After
>> usb_ep_disable() no further serial packets can be received until the
>> endpoints are re-enabled. This happens in gserial_connect().
>
>That's why I asked. There are two potential issues:
>
>First, tty_hangup() is asynchronous -- ie., it returns immediately. It
>does not wait for the tty device to actually perform the hangup. So if
>the gadget layers start cleanup immediately after, expecting that they
>won't get a flurry of tty calls, that would be bad.
tty_hangup() is called from interrupt context usually after the USB
cable has been unplugged. It is also possible that if the host requests
the function twice, it will (on the second invocation without a reset)
call tty_hangup() followed by tty_wakeup() to start the new session.
>
>tty_vhangup() is synchronous -- ie., you wait while it cleans up. This
>is what the usb serial core does on it's disconnect() method. But I
>didn't research further if the circumstances were the same.
Okay.
>Second, when the hangup actually does run -- in __tty_hangup() -- it
>expects the tty to exist. I didn't go looking through the gadget layers
>to see if the tty was disposed some other way, which might race the
>asynchronous tty hangup.
It calls wake_up_interruptible() expecting the user to leave. The
node is removed on module removal.
Is there any change of API you suggest?
>Thanks,
>Peter Hurley
Sebastian