Re: [PATCH v2] xhci: dbgtty: fix device unregister

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 11/24/25 08:42, Jiri Slaby wrote:
> Hmm, CCing TTY MAINTAINERS entry would not hurt.
> 
> On 19. 11. 25, 22:29, Łukasz Bartosik wrote:
>> From: Łukasz Bartosik <ukaszb@chromium.org>
>>
>> When DbC is disconnected then xhci_dbc_tty_unregister_device()
>> is called. However if there is any user space process blocked
>> on write to DbC terminal device then it will never be signalled
>> and thus stay blocked indifinitely.
> 
> indefinitely
> 
>> This fix adds a tty_vhangup() call in xhci_dbc_tty_unregister_device().
>> The tty_vhangup() wakes up any blocked writers and causes subsequent
>> write attempts to DbC terminal device to fail.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
>> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
>> ---
>> Changes in v2:
>> - Replaced tty_hangup() with tty_vhangup()
> 
> Why exactly?

I recommended using tty_vhangup(), well actually tty_port_tty_vhangup() to solve
issue '2' you pointed out.
To me it looks like tty_vhangup() is synchronous so it won't schedule hangup work
and should be safe to call right before we destroy the port.

> 
>> --- a/drivers/usb/host/xhci-dbgtty.c
>> +++ b/drivers/usb/host/xhci-dbgtty.c
>> @@ -535,6 +535,12 @@ static void xhci_dbc_tty_unregister_device(struct xhci_dbc *dbc)
>>       if (!port->registered)
>>           return;
>> +    /*
>> +     * Hang up the TTY. This wakes up any blocked
>> +     * writers and causes subsequent writes to fail.
>> +     */
>> +    tty_vhangup(port->port.tty);
> 
> This is wrong IMO:
> 1) what if there is no tty currently open? Ie. tty is NULL.

v1 had the NULL check, I stated that tty_port_tty_vhangup() does the check for us
so no need for it here.

https://lore.kernel.org/linux-usb/d25feb0d-2ede-4722-a499-095139870c96@linux.intel.com/

looks like v2 removed the check but used tty_vhangup() instead of
tty_port_tty_vhangup()

> 2) you schedule a tty hangup work and destroy the port right after:
>>       tty_unregister_device(dbc_tty_driver, port->minor);
>>       xhci_dbc_tty_exit_port(port);
>>       port->registered = false;
> You should to elaborate how this is supposed to work?

Does tty_port_tty_vhangup() work here? it
1. checks if tty is NULL
2. is synchronous and should be safe to call before tty_unregister_device()

tty internals are still a bit of mystery to me

Thanks
Mathias