* [PATCH v2 0/4] TTY: port hangup and close fixes
@ 2013-02-26 11:14 Johan Hovold
2013-02-26 11:14 ` [PATCH v2 1/4] TTY: clean up port shutdown Johan Hovold
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Johan Hovold @ 2013-02-26 11:14 UTC (permalink / raw)
To: Greg KH; +Cc: Alan Stern, linux-usb, linux-serial, Peter Hurley, Johan Hovold
These patches against tty-next fix a few issues with tty-port hangup and
close.
The first and third patch are essentially clean ups.
The second patch makes sure DTR is dropped also on hangup and that DTR
is only dropped for initialised ports (where is could have been raised
in the first place).
The fourth and final patch, make sure no tty callbacks are made from
tty_port_close_start when the port has not been initialised (successfully
opened). This was previously only done for wait_until_sent but there's
no reason to call flush_buffer or to honour port drain delay either.
The latter could cause a failed open to stall for up to two seconds.
As a side-effect, these patches also fix an issue in USB-serial where we could
get tty-port callbacks on an uninitialised port after having hung up and
unregistered a device after disconnect.
Johan
v2:
- reuse tty reference from hangup and close in shutdown. Both call sites
guarantee tty is either NULL or has a kref.
Changes since RFC-series:
- fix up the two driver relying on tty_port_close_start directly but
that did not manage DTR themselves
Johan Hovold (4):
TTY: clean up port shutdown
TTY: fix DTR not being dropped on hang up
TTY: clean up port drain-delay handling
TTY: fix close of uninitialised ports
drivers/tty/mxser.c | 4 +++
drivers/tty/n_gsm.c | 4 +++
drivers/tty/tty_port.c | 72 +++++++++++++++++++++++++++++---------------------
3 files changed, 50 insertions(+), 30 deletions(-)
--
1.8.1.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/4] TTY: clean up port shutdown 2013-02-26 11:14 [PATCH v2 0/4] TTY: port hangup and close fixes Johan Hovold @ 2013-02-26 11:14 ` Johan Hovold [not found] ` <1361877272-6074-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Johan Hovold @ 2013-02-26 11:14 UTC (permalink / raw) To: Greg KH; +Cc: Alan Stern, linux-usb, linux-serial, Peter Hurley, Johan Hovold Untangle port-shutdown logic and make sure the initialised flag is always cleared for non-console ports. Signed-off-by: Johan Hovold <jhovold@gmail.com> --- drivers/tty/tty_port.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index b7ff59d..57a061e 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -199,9 +199,14 @@ EXPORT_SYMBOL(tty_port_tty_set); static void tty_port_shutdown(struct tty_port *port) { mutex_lock(&port->mutex); - if (port->ops->shutdown && !port->console && - test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags)) + if (port->console) + goto out; + + if (test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags)) { + if (port->ops->shutdown) port->ops->shutdown(port); + } +out: mutex_unlock(&port->mutex); } -- 1.8.1.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <1361877272-6074-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH v2 2/4] TTY: fix DTR not being dropped on hang up [not found] ` <1361877272-6074-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-02-26 11:14 ` Johan Hovold 2013-02-26 11:14 ` [PATCH v2 3/4] TTY: clean up port drain-delay handling Johan Hovold 1 sibling, 0 replies; 6+ messages in thread From: Johan Hovold @ 2013-02-26 11:14 UTC (permalink / raw) To: Greg KH Cc: Alan Stern, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-serial-u79uwXL29TY76Z2rM5mHXA, Peter Hurley, Johan Hovold Move HUPCL handling to port shutdown so that DTR is dropped also on hang up (tty_port_close is a noop for hung-up ports). Also do not try to drop DTR for uninitialised ports where it has never been raised (e.g. after a failed open). Nine drivers currently call tty_port_close_start directly (rather than through tty_port_close) and seven of them lower DTR as part of their close (if the port has been initialised). Fixup the remaining two drivers so that it continues to be lowered also on normal (non-HUP) close. [ Note that most of those other seven drivers did not expect DTR to have been dropped by tty_port_close_start in the first place. ] Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/tty/mxser.c | 4 ++++ drivers/tty/n_gsm.c | 4 ++++ drivers/tty/tty_port.c | 24 ++++++++++++------------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c index 484b6a3..c547887 100644 --- a/drivers/tty/mxser.c +++ b/drivers/tty/mxser.c @@ -1084,6 +1084,10 @@ static void mxser_close(struct tty_struct *tty, struct file *filp) mutex_lock(&port->mutex); mxser_close_port(port); mxser_flush_buffer(tty); + if (test_bit(ASYNCB_INITIALIZED, &port->flags)) { + if (tty->termios.c_cflag & HUPCL) + tty_port_lower_dtr_rts(port); + } mxser_shutdown_port(port); clear_bit(ASYNCB_INITIALIZED, &port->flags); mutex_unlock(&port->mutex); diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 4a43ef5d7..049013e 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2968,6 +2968,10 @@ static void gsmtty_close(struct tty_struct *tty, struct file *filp) if (tty_port_close_start(&dlci->port, tty, filp) == 0) goto out; gsm_dlci_begin_close(dlci); + if (test_bit(ASYNCB_INITIALIZED, &dlci->port.flags)) { + if (tty->termios.c_cflag & HUPCL) + tty_port_lower_dtr_rts(&dlci->port); + } tty_port_close_end(&dlci->port, tty); tty_port_tty_set(&dlci->port, NULL); out: diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index 57a061e..2c4fea6 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -196,13 +196,20 @@ void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty) } EXPORT_SYMBOL(tty_port_tty_set); -static void tty_port_shutdown(struct tty_port *port) +static void tty_port_shutdown(struct tty_port *port, struct tty_struct *tty) { mutex_lock(&port->mutex); if (port->console) goto out; if (test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags)) { + /* + * Drop DTR/RTS if HUPCL is set. This causes any attached + * modem to hang up the line. + */ + if (!tty || tty->termios.c_cflag & HUPCL) + tty_port_lower_dtr_rts(port); + if (port->ops->shutdown) port->ops->shutdown(port); } @@ -225,15 +232,13 @@ void tty_port_hangup(struct tty_port *port) spin_lock_irqsave(&port->lock, flags); port->count = 0; port->flags &= ~ASYNC_NORMAL_ACTIVE; - if (port->tty) { + if (port->tty) set_bit(TTY_IO_ERROR, &port->tty->flags); - tty_kref_put(port->tty); - } - port->tty = NULL; spin_unlock_irqrestore(&port->lock, flags); + tty_port_shutdown(port, port->tty); + tty_port_tty_set(port, NULL); wake_up_interruptible(&port->open_wait); wake_up_interruptible(&port->delta_msr_wait); - tty_port_shutdown(port); } EXPORT_SYMBOL(tty_port_hangup); @@ -452,11 +457,6 @@ int tty_port_close_start(struct tty_port *port, /* Flush the ldisc buffering */ tty_ldisc_flush(tty); - /* Drop DTR/RTS if HUPCL is set. This causes any attached modem to - hang up the line */ - if (tty->termios.c_cflag & HUPCL) - tty_port_lower_dtr_rts(port); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] TTY: clean up port drain-delay handling [not found] ` <1361877272-6074-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-02-26 11:14 ` [PATCH v2 2/4] TTY: fix DTR not being dropped on hang up Johan Hovold @ 2013-02-26 11:14 ` Johan Hovold 1 sibling, 0 replies; 6+ messages in thread From: Johan Hovold @ 2013-02-26 11:14 UTC (permalink / raw) To: Greg KH Cc: Alan Stern, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-serial-u79uwXL29TY76Z2rM5mHXA, Peter Hurley, Johan Hovold Move port drain-delay handling to a separate function. Signed-off-by: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/tty/tty_port.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index 2c4fea6..ce4b591 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -405,6 +405,20 @@ int tty_port_block_til_ready(struct tty_port *port, } EXPORT_SYMBOL(tty_port_block_til_ready); +static void tty_port_drain_delay(struct tty_port *port, struct tty_struct *tty) +{ + unsigned int bps = tty_get_baud_rate(tty); + long timeout; + + if (bps > 1200) { + timeout = (HZ * 10 * port->drain_delay) / bps; + timeout = max_t(long, timeout, HZ / 10); + } else { + timeout = 2 * HZ; + } + schedule_timeout_interruptible(timeout); +} + int tty_port_close_start(struct tty_port *port, struct tty_struct *tty, struct file *filp) { @@ -443,17 +457,8 @@ int tty_port_close_start(struct tty_port *port, if (test_bit(ASYNCB_INITIALIZED, &port->flags) && port->closing_wait != ASYNC_CLOSING_WAIT_NONE) tty_wait_until_sent_from_close(tty, port->closing_wait); - if (port->drain_delay) { - unsigned int bps = tty_get_baud_rate(tty); - long timeout; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] TTY: fix close of uninitialised ports 2013-02-26 11:14 [PATCH v2 0/4] TTY: port hangup and close fixes Johan Hovold 2013-02-26 11:14 ` [PATCH v2 1/4] TTY: clean up port shutdown Johan Hovold [not found] ` <1361877272-6074-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-02-26 11:14 ` Johan Hovold 2013-02-26 11:56 ` [PATCH v2 0/4] TTY: port hangup and close fixes Peter Hurley 3 siblings, 0 replies; 6+ messages in thread From: Johan Hovold @ 2013-02-26 11:14 UTC (permalink / raw) To: Greg KH; +Cc: Alan Stern, linux-usb, linux-serial, Peter Hurley, Johan Hovold Make sure we do not make tty-driver callbacks or wait for port to drain on uninitialised ports (e.g. when open failed) in tty_port_close_start(). No callback, such as flush_buffer or wait_until_sent, needs to be made on a port that has never been opened. Neither does it make much sense to add drain delay for an uninitialised port. Currently a drain delay of up to two seconds could be added when a tty fails to open. Signed-off-by: Johan Hovold <jhovold@gmail.com> --- drivers/tty/tty_port.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index ce4b591..e73ac90 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -451,14 +451,16 @@ int tty_port_close_start(struct tty_port *port, set_bit(ASYNCB_CLOSING, &port->flags); tty->closing = 1; spin_unlock_irqrestore(&port->lock, flags); - /* Don't block on a stalled port, just pull the chain */ - if (tty->flow_stopped) - tty_driver_flush_buffer(tty); - if (test_bit(ASYNCB_INITIALIZED, &port->flags) && - port->closing_wait != ASYNC_CLOSING_WAIT_NONE) - tty_wait_until_sent_from_close(tty, port->closing_wait); - if (port->drain_delay) - tty_port_drain_delay(port, tty); + + if (test_bit(ASYNCB_INITIALIZED, &port->flags)) { + /* Don't block on a stalled port, just pull the chain */ + if (tty->flow_stopped) + tty_driver_flush_buffer(tty); + if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE) + tty_wait_until_sent_from_close(tty, port->closing_wait); + if (port->drain_delay) + tty_port_drain_delay(port, tty); + } /* Flush the ldisc buffering */ tty_ldisc_flush(tty); -- 1.8.1.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/4] TTY: port hangup and close fixes 2013-02-26 11:14 [PATCH v2 0/4] TTY: port hangup and close fixes Johan Hovold ` (2 preceding siblings ...) 2013-02-26 11:14 ` [PATCH v2 4/4] TTY: fix close of uninitialised ports Johan Hovold @ 2013-02-26 11:56 ` Peter Hurley 3 siblings, 0 replies; 6+ messages in thread From: Peter Hurley @ 2013-02-26 11:56 UTC (permalink / raw) To: Johan Hovold; +Cc: Greg KH, Alan Stern, linux-usb, linux-serial On Tue, 2013-02-26 at 12:14 +0100, Johan Hovold wrote: > These patches against tty-next fix a few issues with tty-port hangup and > close. > > The first and third patch are essentially clean ups. > > The second patch makes sure DTR is dropped also on hangup and that DTR > is only dropped for initialised ports (where is could have been raised > in the first place). > > The fourth and final patch, make sure no tty callbacks are made from > tty_port_close_start when the port has not been initialised (successfully > opened). This was previously only done for wait_until_sent but there's > no reason to call flush_buffer or to honour port drain delay either. > The latter could cause a failed open to stall for up to two seconds. > > As a side-effect, these patches also fix an issue in USB-serial where we could > get tty-port callbacks on an uninitialised port after having hung up and > unregistered a device after disconnect. > > Johan > > > v2: > - reuse tty reference from hangup and close in shutdown. Both call sites > guarantee tty is either NULL or has a kref. Great, thank you. Peter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-26 11:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-26 11:14 [PATCH v2 0/4] TTY: port hangup and close fixes Johan Hovold
2013-02-26 11:14 ` [PATCH v2 1/4] TTY: clean up port shutdown Johan Hovold
[not found] ` <1361877272-6074-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-02-26 11:14 ` [PATCH v2 2/4] TTY: fix DTR not being dropped on hang up Johan Hovold
2013-02-26 11:14 ` [PATCH v2 3/4] TTY: clean up port drain-delay handling Johan Hovold
2013-02-26 11:14 ` [PATCH v2 4/4] TTY: fix close of uninitialised ports Johan Hovold
2013-02-26 11:56 ` [PATCH v2 0/4] TTY: port hangup and close fixes Peter Hurley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).