Re: tty: closing n_gsm line discipline always times out

[Alan Cox <gnomes@lxorguk.ukuu.org.uk>]

On Wed, 17 May 2017 15:44:56 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi All,
> 
> When the n_gsm line discipline is closed it wants to shutdown the line
> discipline properly and asks the link partner to end the mux protocol. This is
> done in gsm_cleanup_mux() around line 2050 in n_gsm.c:
> 
> 	if (dlci) {
> 		gc = gsm_control_send(gsm, CMD_CLD, NULL, 0);
> 		if (gc)
> 			gsm_control_wait(gsm, gc);
> 	}
> 
> The call stack is like this:
> 
> (struct tty_ldisc_ops).close
>   -> gsmld_close
>     -> gsmld_detach_gsm
>       -> gsm_cleanup_mux  
> 
> (struct tty_ldisc_ops).close is called at tty_release time or when the line
> discipline is changed.  At tty_release time the tty is already shutdown, so we
> cannot send anything anymore. In this case our link partner never closes the
> mux protocol, which means we can never re-open it again.  When instead the line
> discipline is changed back to N_TTY to close the protocol, gsm_control_send
> works fine, but the tty_ldisc is locked and thus the chars received by the UART
> never make it to the line discipline and gsm_control_wait times out.
> 
> There's obviously something wrong here. Any ideas how to fix that? When we are
> not allowed to send/receive at tty_release time, do we need a new n_gsm specific
> ioctl to shutdown the mux?

Probably - it used to work fine but other changes in the core tty code
broke it, and I think because 99% of the users of that code are Android
they've not yet caught up with this decade 8)

The other option would be to just remove the CLD sending and have the
caller do whatever cleanup handling it wants in N_TTY ldisc. You can just
write() the frame rather than having an ioctl.

That said there are power management cases where being able to turn it
on/off without ldisc changing might be useful. However you've then got to
deal with all the locking of active sessions against being in down state.

So I think just having the caller use a write() in N_TTY would be simpler
since we'd have to change user space anyway.

Alan