* [PATCH 1/7] tty: Remove tty_wait_until_sent_from_close()
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
2015-10-10 20:00 ` [PATCH 2/7] tty: Remove ASYNC_CLOSING checks in open()/hangup() methods Peter Hurley
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Karsten Keil, Peter Hurley, Arnd Bergmann, netdev, linuxppc-dev,
linux-kernel, David Laight, linux-serial, Jiri Slaby,
David Miller, Alan Cox
tty_wait_until_sent_from_close() drops the tty lock while waiting
for the tty driver to finish sending previously accepted data (ie.,
data remaining in its write buffer and transmit fifo).
tty_wait_until_sent_from_close() was added by commit a57a7bf3fc7e
("TTY: define tty_wait_until_sent_from_close") to prevent the entire
tty subsystem from being unable to open new ttys while waiting for
one tty to close while output drained.
However, since commit 0911261d4cb6 ("tty: Don't take tty_mutex for tty
count changes"), holding a tty lock while closing does not prevent other
ttys from being opened/closed/hung up, but only prevents lifetime event
changes for the tty under lock.
Holding the tty lock while waiting for output to drain does prevent
parallel non-blocking opens (O_NONBLOCK) from advancing or returning
while the tty lock is held. However, all parallel opens _already_
block even if the tty lock is dropped while closing and the parallel
open advances. Blocking in open has been in mainline since at least 2.6.29
(see tty_port_block_til_ready(); note the test for O_NONBLOCK is _after_
the wait while ASYNC_CLOSING).
IOW, before this patch a non-blocking open will sleep anyway for the
_entire_ duration of a parallel hardware shutdown, and when it wakes, the
error return will cause a release of its tty, and it will restart with
a fresh attempt to open. Similarly with a blocking open that is already
waiting; when it's woken, the hardware shutdown has already completed
to ASYNC_INITIALIZED is not set, which forces a release and restart as
well.
So, holding the tty lock across the _entire_ close (which is what this
patch does), even while waiting for output to drain, is equivalent to
the current outcome wrt parallel opens.
Cc: Alan Cox <alan@linux.intel.com>
Cc: David Laight <David.Laight@aculab.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Karsten Keil <isdn@linux-pingi.de>
CC: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/isdn/i4l/isdn_tty.c | 2 +-
drivers/tty/hvc/hvc_console.c | 2 +-
drivers/tty/hvc/hvcs.c | 2 +-
drivers/tty/tty_port.c | 11 ++---------
include/linux/tty.h | 18 ------------------
5 files changed, 5 insertions(+), 30 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index bc91261..2175225 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1582,7 +1582,7 @@ isdn_tty_close(struct tty_struct *tty, struct file *filp)
* line status register.
*/
if (port->flags & ASYNC_INITIALIZED) {
- tty_wait_until_sent_from_close(tty, 3000); /* 30 seconds timeout */
+ tty_wait_until_sent(tty, 3000); /* 30 seconds timeout */
/*
* Before we drop DTR, make sure the UART transmitter
* has completely drained; this is especially
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 9c30f67..e46d628 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -418,7 +418,7 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
* there is no buffered data otherwise sleeps on a wait queue
* waking periodically to check chars_in_buffer().
*/
- tty_wait_until_sent_from_close(tty, HVC_CLOSE_WAIT);
+ tty_wait_until_sent(tty, HVC_CLOSE_WAIT);
} else {
if (hp->port.count < 0)
printk(KERN_ERR "hvc_close %X: oops, count is %d\n",
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index f7ff97c..5997b17 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -1230,7 +1230,7 @@ static void hvcs_close(struct tty_struct *tty, struct file *filp)
irq = hvcsd->vdev->irq;
spin_unlock_irqrestore(&hvcsd->lock, flags);
- tty_wait_until_sent_from_close(tty, HVCS_CLOSE_WAIT);
+ tty_wait_until_sent(tty, HVCS_CLOSE_WAIT);
/*
* This line is important because it tells hvcs_open that this
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 40b3183..d7d9f9c 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -463,10 +463,7 @@ static void tty_port_drain_delay(struct tty_port *port, struct tty_struct *tty)
schedule_timeout_interruptible(timeout);
}
-/* Caller holds tty lock.
- * NB: may drop and reacquire tty lock (in tty_wait_until_sent_from_close())
- * so tty and tty port may have changed state (but not hung up or reopened).
- */
+/* Caller holds tty lock. */
int tty_port_close_start(struct tty_port *port,
struct tty_struct *tty, struct file *filp)
{
@@ -502,7 +499,7 @@ int tty_port_close_start(struct tty_port *port,
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);
+ tty_wait_until_sent(tty, port->closing_wait);
if (port->drain_delay)
tty_port_drain_delay(port, tty);
}
@@ -543,10 +540,6 @@ EXPORT_SYMBOL(tty_port_close_end);
* tty_port_close
*
* Caller holds tty lock
- *
- * NB: may drop and reacquire tty lock (in tty_port_close_start()->
- * tty_wait_until_sent_from_close()) so tty and tty_port may have changed
- * state (but not hung up or reopened).
*/
void tty_port_close(struct tty_port *port, struct tty_struct *tty,
struct file *filp)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d072ded..614c822 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -657,24 +657,6 @@ extern void __lockfunc tty_lock_slave(struct tty_struct *tty);
extern void __lockfunc tty_unlock_slave(struct tty_struct *tty);
extern void tty_set_lock_subclass(struct tty_struct *tty);
/*
- * this shall be called only from where BTM is held (like close)
- *
- * We need this to ensure nobody waits for us to finish while we are waiting.
- * Without this we were encountering system stalls.
- *
- * This should be indeed removed with BTM removal later.
- *
- * Locking: BTM required. Nobody is allowed to hold port->mutex.
- */
-static inline void tty_wait_until_sent_from_close(struct tty_struct *tty,
- long timeout)
-{
- tty_unlock(tty); /* tty->ops->close holds the BTM, drop it while waiting */
- tty_wait_until_sent(tty, timeout);
- tty_lock(tty);
-}
-
-/*
* wait_event_interruptible_tty -- wait for a condition with the tty lock held
*
* The condition we are waiting for might take a long time to
--
2.6.1
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/7] tty: Remove ASYNC_CLOSING checks in open()/hangup() methods
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
2015-10-10 20:00 ` [PATCH 1/7] tty: Remove tty_wait_until_sent_from_close() Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
2015-10-18 4:13 ` Greg Kroah-Hartman
2015-10-10 20:00 ` [PATCH 3/7] usb: gadget: gserial: Privatize close_wait Peter Hurley
` (4 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller, Peter Hurley
Since at least before 2.6.30, tty drivers that do not drop the tty lock
while closing cannot observe ASYNC_CLOSING set while holding the
tty lock; this includes the tty driver's open() and hangup() methods,
since the tty core calls these methods holding the tty lock.
For these drivers, waiting for ASYNC_CLOSING to clear while opening
is not required, since this condition cannot occur. Similarly, even
when the open() method drops and reacquires the tty lock after
blocking, ASYNC_CLOSING cannot be set (again, for drivers that
do not drop the tty lock while closing).
Now that tty port drivers no longer drop the tty lock while closing
(since 'tty: Remove tty_wait_until_sent_from_close()'), the same
conditions apply: waiting for ASYNC_CLOSING to clear while opening
is not required, nor is re-checking ASYNC_CLOSING after dropping and
reacquiring the tty lock while blocking (eg., in *_block_til_ready()).
Note: The ASYNC_CLOSING flag state is still maintained since several
bitrotting drivers use it for (dubious) other purposes.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/char/pcmcia/synclink_cs.c | 9 ---------
drivers/tty/cyclades.c | 9 ---------
drivers/tty/rocket.c | 12 ------------
drivers/tty/serial/crisv10.c | 33 +--------------------------------
drivers/tty/synclink.c | 18 ++++--------------
drivers/tty/synclink_gt.c | 14 ++------------
drivers/tty/synclinkmp.c | 14 ++------------
drivers/tty/tty_port.c | 13 +------------
net/irda/ircomm/ircomm_tty.c | 31 +------------------------------
9 files changed, 11 insertions(+), 142 deletions(-)
diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 7680d52..45df4bf 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -2507,15 +2507,6 @@ static int mgslpc_open(struct tty_struct *tty, struct file * filp)
printk("%s(%d):mgslpc_open(%s), old ref count = %d\n",
__FILE__, __LINE__, tty->driver->name, port->count);
- /* If port is closing, signal caller to try again */
- if (port->flags & ASYNC_CLOSING){
- wait_event_interruptible_tty(tty, port->close_wait,
- !(port->flags & ASYNC_CLOSING));
- retval = ((port->flags & ASYNC_HUP_NOTIFY) ?
- -EAGAIN : -ERESTARTSYS);
- goto cleanup;
- }
-
port->low_latency = (port->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
spin_lock_irqsave(&info->netlock, flags);
diff --git a/drivers/tty/cyclades.c b/drivers/tty/cyclades.c
index 87f6578..d4a1331 100644
--- a/drivers/tty/cyclades.c
+++ b/drivers/tty/cyclades.c
@@ -1577,15 +1577,6 @@ static int cy_open(struct tty_struct *tty, struct file *filp)
#endif
/*
- * If the port is the middle of closing, bail out now
- */
- if (info->port.flags & ASYNC_CLOSING) {
- wait_event_interruptible_tty(tty, info->port.close_wait,
- !(info->port.flags & ASYNC_CLOSING));
- return (info->port.flags & ASYNC_HUP_NOTIFY) ? -EAGAIN: -ERESTARTSYS;
- }
-
- /*
* Start up serial port
*/
retval = cy_startup(info, tty);
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index c8dd8dc..69c72d1 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -895,14 +895,6 @@ static int rp_open(struct tty_struct *tty, struct file *filp)
if (!page)
return -ENOMEM;
- if (port->flags & ASYNC_CLOSING) {
- retval = wait_for_completion_interruptible(&info->close_wait);
- free_page(page);
- if (retval)
- return retval;
- return ((port->flags & ASYNC_HUP_NOTIFY) ? -EAGAIN : -ERESTARTSYS);
- }
-
/*
* We must not sleep from here until the port is marked fully in use.
*/
@@ -1511,10 +1503,6 @@ static void rp_hangup(struct tty_struct *tty)
#endif
rp_flush_buffer(tty);
spin_lock_irqsave(&info->port.lock, flags);
- if (info->port.flags & ASYNC_CLOSING) {
- spin_unlock_irqrestore(&info->port.lock, flags);
- return;
- }
if (info->port.count)
atomic_dec(&rp_num_ports_open);
clear_bit((info->aiop * 8) + info->chan, (void *) &xmit_flags[info->board]);
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index 3e4470a..06f4fe9 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3759,23 +3759,6 @@ block_til_ready(struct tty_struct *tty, struct file * filp,
int do_clocal = 0;
/*
- * If the device is in the middle of being closed, then block
- * until it's done, and then try again.
- */
- if (info->port.flags & ASYNC_CLOSING) {
- wait_event_interruptible_tty(tty, info->port.close_wait,
- !(info->port.flags & ASYNC_CLOSING));
-#ifdef SERIAL_DO_RESTART
- if (info->port.flags & ASYNC_HUP_NOTIFY)
- return -EAGAIN;
- else
- return -ERESTARTSYS;
-#else
- return -EAGAIN;
-#endif
- }
-
- /*
* If non-blocking mode is set, or the port is not enabled,
* then make the check up front and then exit.
*/
@@ -3825,7 +3808,7 @@ block_til_ready(struct tty_struct *tty, struct file * filp,
#endif
break;
}
- if (!(info->port.flags & ASYNC_CLOSING) && do_clocal)
+ if (do_clocal)
/* && (do_clocal || DCD_IS_ASSERTED) */
break;
if (signal_pending(current)) {
@@ -3895,20 +3878,6 @@ rs_open(struct tty_struct *tty, struct file * filp)
info->port.low_latency = !!(info->port.flags & ASYNC_LOW_LATENCY);
/*
- * If the port is in the middle of closing, bail out now
- */
- if (info->port.flags & ASYNC_CLOSING) {
- wait_event_interruptible_tty(tty, info->port.close_wait,
- !(info->port.flags & ASYNC_CLOSING));
-#ifdef SERIAL_DO_RESTART
- return ((info->port.flags & ASYNC_HUP_NOTIFY) ?
- -EAGAIN : -ERESTARTSYS);
-#else
- return -EAGAIN;
-#endif
- }
-
- /*
* If DMA is enabled try to allocate the irq's.
*/
if (info->port.count == 1) {
diff --git a/drivers/tty/synclink.c b/drivers/tty/synclink.c
index 2fac712..11d322b 100644
--- a/drivers/tty/synclink.c
+++ b/drivers/tty/synclink.c
@@ -3314,12 +3314,11 @@ static int block_til_ready(struct tty_struct *tty, struct file * filp,
-EAGAIN : -ERESTARTSYS;
break;
}
-
+
dcd = tty_port_carrier_raised(&info->port);
-
- if (!(port->flags & ASYNC_CLOSING) && (do_clocal || dcd))
- break;
-
+ if (do_clocal || dcd)
+ break;
+
if (signal_pending(current)) {
retval = -ERESTARTSYS;
break;
@@ -3398,15 +3397,6 @@ static int mgsl_open(struct tty_struct *tty, struct file * filp)
printk("%s(%d):mgsl_open(%s), old ref count = %d\n",
__FILE__,__LINE__,tty->driver->name, info->port.count);
- /* If port is closing, signal caller to try again */
- if (info->port.flags & ASYNC_CLOSING){
- wait_event_interruptible_tty(tty, info->port.close_wait,
- !(info->port.flags & ASYNC_CLOSING));
- retval = ((info->port.flags & ASYNC_HUP_NOTIFY) ?
- -EAGAIN : -ERESTARTSYS);
- goto cleanup;
- }
-
info->port.low_latency = (info->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
spin_lock_irqsave(&info->netlock, flags);
diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 0ea8eee..6fc39fb 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -672,15 +672,6 @@ static int open(struct tty_struct *tty, struct file *filp)
DBGINFO(("%s open, old ref count = %d\n", info->device_name, info->port.count));
- /* If port is closing, signal caller to try again */
- if (info->port.flags & ASYNC_CLOSING){
- wait_event_interruptible_tty(tty, info->port.close_wait,
- !(info->port.flags & ASYNC_CLOSING));
- retval = ((info->port.flags & ASYNC_HUP_NOTIFY) ?
- -EAGAIN : -ERESTARTSYS);
- goto cleanup;
- }
-
mutex_lock(&info->port.mutex);
info->port.low_latency = (info->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
@@ -3320,9 +3311,8 @@ static int block_til_ready(struct tty_struct *tty, struct file *filp,
}
cd = tty_port_carrier_raised(port);
-
- if (!(port->flags & ASYNC_CLOSING) && (do_clocal || cd ))
- break;
+ if (do_clocal || cd)
+ break;
if (signal_pending(current)) {
retval = -ERESTARTSYS;
diff --git a/drivers/tty/synclinkmp.c b/drivers/tty/synclinkmp.c
index 08633a8..fb00a06 100644
--- a/drivers/tty/synclinkmp.c
+++ b/drivers/tty/synclinkmp.c
@@ -752,15 +752,6 @@ static int open(struct tty_struct *tty, struct file *filp)
printk("%s(%d):%s open(), old ref count = %d\n",
__FILE__,__LINE__,tty->driver->name, info->port.count);
- /* If port is closing, signal caller to try again */
- if (info->port.flags & ASYNC_CLOSING){
- wait_event_interruptible_tty(tty, info->port.close_wait,
- !(info->port.flags & ASYNC_CLOSING));
- retval = ((info->port.flags & ASYNC_HUP_NOTIFY) ?
- -EAGAIN : -ERESTARTSYS);
- goto cleanup;
- }
-
info->port.low_latency = (info->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
spin_lock_irqsave(&info->netlock, flags);
@@ -3341,9 +3332,8 @@ static int block_til_ready(struct tty_struct *tty, struct file *filp,
}
cd = tty_port_carrier_raised(port);
-
- if (!(port->flags & ASYNC_CLOSING) && (do_clocal || cd))
- break;
+ if (do_clocal || cd)
+ break;
if (signal_pending(current)) {
retval = -ERESTARTSYS;
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index d7d9f9c..0e1cf04 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -363,16 +363,6 @@ int tty_port_block_til_ready(struct tty_port *port,
unsigned long flags;
DEFINE_WAIT(wait);
- /* block if port is in the process of being closed */
- if (port->flags & ASYNC_CLOSING) {
- wait_event_interruptible_tty(tty, port->close_wait,
- !(port->flags & ASYNC_CLOSING));
- if (port->flags & ASYNC_HUP_NOTIFY)
- return -EAGAIN;
- else
- return -ERESTARTSYS;
- }
-
/* 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 (tty->flags & (1 << TTY_IO_ERROR)) {
@@ -423,8 +413,7 @@ int tty_port_block_til_ready(struct tty_port *port,
* Never ask drivers if CLOCAL is set, this causes troubles
* on some hardware.
*/
- if (!(port->flags & ASYNC_CLOSING) &&
- (do_clocal || tty_port_carrier_raised(port)))
+ if (do_clocal || tty_port_carrier_raised(port))
break;
if (signal_pending(current)) {
retval = -ERESTARTSYS;
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 683346d..a423770 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -335,8 +335,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
* specified, we cannot return before the IrCOMM link is
* ready
*/
- if (!test_bit(ASYNCB_CLOSING, &port->flags) &&
- (do_clocal || tty_port_carrier_raised(port)) &&
+ if ((do_clocal || tty_port_carrier_raised(port)) &&
self->state == IRCOMM_TTY_READY)
{
break;
@@ -443,34 +442,6 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
/* Not really used by us, but lets do it anyway */
self->port.low_latency = (self->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
- /*
- * If the port is the middle of closing, bail out now
- */
- if (test_bit(ASYNCB_CLOSING, &self->port.flags)) {
-
- /* Hm, why are we blocking on ASYNC_CLOSING if we
- * do return -EAGAIN/-ERESTARTSYS below anyway?
- * IMHO it's either not needed in the first place
- * or for some reason we need to make sure the async
- * closing has been finished - if so, wouldn't we
- * probably better sleep uninterruptible?
- */
-
- if (wait_event_interruptible(self->port.close_wait,
- !test_bit(ASYNCB_CLOSING, &self->port.flags))) {
- net_warn_ratelimited("%s - got signal while blocking on ASYNC_CLOSING!\n",
- __func__);
- return -ERESTARTSYS;
- }
-
-#ifdef SERIAL_DO_RESTART
- return (self->port.flags & ASYNC_HUP_NOTIFY) ?
- -EAGAIN : -ERESTARTSYS;
-#else
- return -EAGAIN;
-#endif
- }
-
/* Check if this is a "normal" ircomm device, or an irlpt device */
if (self->line < 0x10) {
self->service_type = IRCOMM_3_WIRE | IRCOMM_9_WIRE;
--
2.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/7] tty: Remove ASYNC_CLOSING checks in open()/hangup() methods
2015-10-10 20:00 ` [PATCH 2/7] tty: Remove ASYNC_CLOSING checks in open()/hangup() methods Peter Hurley
@ 2015-10-18 4:13 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2015-10-18 4:13 UTC (permalink / raw)
To: Peter Hurley
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller
On Sat, Oct 10, 2015 at 04:00:52PM -0400, Peter Hurley wrote:
>
> Note: The ASYNC_CLOSING flag state is still maintained since several
> bitrotting drivers use it for (dubious) other purposes.
I think we should drop it, it's pointless and no one ever actually does
anything with it.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/7] usb: gadget: gserial: Privatize close_wait
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
2015-10-10 20:00 ` [PATCH 1/7] tty: Remove tty_wait_until_sent_from_close() Peter Hurley
2015-10-10 20:00 ` [PATCH 2/7] tty: Remove ASYNC_CLOSING checks in open()/hangup() methods Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
2015-10-10 20:09 ` Peter Hurley
2015-10-10 20:00 ` [PATCH 4/7] tty: Remove tty_port::close_wait Peter Hurley
` (3 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller, Peter Hurley
close_wait is no longer needed or provided by the tty core.
Move close_wait to struct gs_port.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/usb/gadget/function/u_serial.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 7ee0579..42894f5 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -114,6 +114,7 @@ struct gs_port {
struct gs_buf port_write_buf;
wait_queue_head_t drain_wait; /* wait while writes drain */
bool write_busy;
+ wait_queue_head_t close_wait;
/* REVISIT this state ... */
struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */
@@ -884,7 +885,7 @@ static void gs_close(struct tty_struct *tty, struct file *file)
pr_debug("gs_close: ttyGS%d (%p,%p) done!\n",
port->port_num, tty, file);
- wake_up(&port->port.close_wait);
+ wake_up(&port->close_wait);
exit:
spin_unlock_irq(&port->port_lock);
}
@@ -1044,6 +1045,7 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
tty_port_init(&port->port);
spin_lock_init(&port->port_lock);
init_waitqueue_head(&port->drain_wait);
+ init_waitqueue_head(&port->close_wait);
tasklet_init(&port->push, gs_rx_push, (unsigned long) port);
@@ -1074,7 +1076,7 @@ static void gserial_free_port(struct gs_port *port)
{
tasklet_kill(&port->push);
/* wait for old opens to finish */
- wait_event(port->port.close_wait, gs_closed(port));
+ wait_event(port->close_wait, gs_closed(port));
WARN_ON(port->port_usb != NULL);
tty_port_destroy(&port->port);
kfree(port);
--
2.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/7] usb: gadget: gserial: Privatize close_wait
2015-10-10 20:00 ` [PATCH 3/7] usb: gadget: gserial: Privatize close_wait Peter Hurley
@ 2015-10-10 20:09 ` Peter Hurley
0 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:09 UTC (permalink / raw)
To: Greg Kroah-Hartman, Felipe Balbi
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller
[ forgot to addr Felipe here, sorry ]
On 10/10/2015 04:00 PM, Peter Hurley wrote:
> close_wait is no longer needed or provided by the tty core.
> Move close_wait to struct gs_port.
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> drivers/usb/gadget/function/u_serial.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> index 7ee0579..42894f5 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -114,6 +114,7 @@ struct gs_port {
> struct gs_buf port_write_buf;
> wait_queue_head_t drain_wait; /* wait while writes drain */
> bool write_busy;
> + wait_queue_head_t close_wait;
>
> /* REVISIT this state ... */
> struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */
> @@ -884,7 +885,7 @@ static void gs_close(struct tty_struct *tty, struct file *file)
> pr_debug("gs_close: ttyGS%d (%p,%p) done!\n",
> port->port_num, tty, file);
>
> - wake_up(&port->port.close_wait);
> + wake_up(&port->close_wait);
> exit:
> spin_unlock_irq(&port->port_lock);
> }
> @@ -1044,6 +1045,7 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
> tty_port_init(&port->port);
> spin_lock_init(&port->port_lock);
> init_waitqueue_head(&port->drain_wait);
> + init_waitqueue_head(&port->close_wait);
>
> tasklet_init(&port->push, gs_rx_push, (unsigned long) port);
>
> @@ -1074,7 +1076,7 @@ static void gserial_free_port(struct gs_port *port)
> {
> tasklet_kill(&port->push);
> /* wait for old opens to finish */
> - wait_event(port->port.close_wait, gs_closed(port));
> + wait_event(port->close_wait, gs_closed(port));
> WARN_ON(port->port_usb != NULL);
> tty_port_destroy(&port->port);
> kfree(port);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/7] tty: Remove tty_port::close_wait
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
` (2 preceding siblings ...)
2015-10-10 20:00 ` [PATCH 3/7] usb: gadget: gserial: Privatize close_wait Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
2015-10-10 20:00 ` [PATCH 5/7] tty: r3964: Use tty->read_wait waitqueue Peter Hurley
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller, Peter Hurley
With the removal of tty_wait_until_sent_from_close(), tty drivers
no longer wait during open for parallel closes to complete (instead,
the tty core waits before calling the driver open() method). Thus,
the close_wait waitqueue is no longer used for waiting.
Remove struct tty_port::close_wait.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/rocket.c | 1 -
drivers/tty/serial/68328serial.c | 1 -
drivers/tty/serial/crisv10.c | 1 -
drivers/tty/serial/serial_core.c | 1 -
drivers/tty/tty_port.c | 2 --
include/linux/tty.h | 1 -
6 files changed, 7 deletions(-)
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index 69c72d1..802eac7 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -1049,7 +1049,6 @@ static void rp_close(struct tty_struct *tty, struct file *filp)
mutex_unlock(&port->mutex);
tty_port_tty_set(port, NULL);
- wake_up_interruptible(&port->close_wait);
complete_all(&info->close_wait);
atomic_dec(&rp_num_ports_open);
diff --git a/drivers/tty/serial/68328serial.c b/drivers/tty/serial/68328serial.c
index 9ba0c93..0140ba4 100644
--- a/drivers/tty/serial/68328serial.c
+++ b/drivers/tty/serial/68328serial.c
@@ -1071,7 +1071,6 @@ static void rs_close(struct tty_struct *tty, struct file * filp)
wake_up_interruptible(&port->open_wait);
}
port->flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
- wake_up_interruptible(&port->close_wait);
local_irq_restore(flags);
}
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index 06f4fe9..f13f2eb 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3655,7 +3655,6 @@ rs_close(struct tty_struct *tty, struct file * filp)
wake_up_interruptible(&info->port.open_wait);
}
info->port.flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
- wake_up_interruptible(&info->port.close_wait);
local_irq_restore(flags);
/* port closed */
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index df4271a..def5199 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1437,7 +1437,6 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
clear_bit(ASYNCB_CLOSING, &port->flags);
spin_unlock_irq(&port->lock);
wake_up_interruptible(&port->open_wait);
- wake_up_interruptible(&port->close_wait);
mutex_unlock(&port->mutex);
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 0e1cf04..e04a8cf 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -22,7 +22,6 @@ void tty_port_init(struct tty_port *port)
memset(port, 0, sizeof(*port));
tty_buffer_init(port);
init_waitqueue_head(&port->open_wait);
- init_waitqueue_head(&port->close_wait);
init_waitqueue_head(&port->delta_msr_wait);
mutex_init(&port->mutex);
mutex_init(&port->buf_mutex);
@@ -520,7 +519,6 @@ void tty_port_close_end(struct tty_port *port, struct tty_struct *tty)
wake_up_interruptible(&port->open_wait);
}
port->flags &= ~(ASYNC_NORMAL_ACTIVE | ASYNC_CLOSING);
- wake_up_interruptible(&port->close_wait);
spin_unlock_irqrestore(&port->lock, flags);
}
EXPORT_SYMBOL(tty_port_close_end);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 614c822..090ce2a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -227,7 +227,6 @@ struct tty_port {
int blocked_open; /* Waiting to open */
int count; /* Usage count */
wait_queue_head_t open_wait; /* Open waiters */
- wait_queue_head_t close_wait; /* Close waiters */
wait_queue_head_t delta_msr_wait; /* Modem status change */
unsigned long flags; /* TTY flags ASY_*/
unsigned char console:1, /* port is a console */
--
2.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/7] tty: r3964: Use tty->read_wait waitqueue
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
` (3 preceding siblings ...)
2015-10-10 20:00 ` [PATCH 4/7] tty: Remove tty_port::close_wait Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
2015-10-10 20:00 ` [PATCH 6/7] tty: r3964: Replace/remove bogus tty lock use Peter Hurley
2015-10-10 20:00 ` [PATCH 7/7] tty: Remove wait_event_interruptible_tty() Peter Hurley
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller, Peter Hurley
The tty core provides read_wait waitqueue specifically for line
disciplines to wait readers; otherwise, the line discipline may
miss wakeups generated by the tty core.
NB: The tty core already provides serialization for the line discipline's
close() method, and guarantees no readers or writers will be using the
closing instance of the line discipline. Completely remove that wakeup.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_r3964.c | 10 ++++------
include/linux/n_r3964.h | 3 ---
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
index 8b157d6..6fdef92 100644
--- a/drivers/tty/n_r3964.c
+++ b/drivers/tty/n_r3964.c
@@ -276,7 +276,7 @@ static void remove_from_tx_queue(struct r3964_info *pInfo, int error_code)
add_msg(pHeader->owner, R3964_MSG_ACK, pHeader->length,
error_code, NULL);
}
- wake_up_interruptible(&pInfo->read_wait);
+ wake_up_interruptible(&pInfo->tty->read_wait);
}
spin_lock_irqsave(&pInfo->lock, flags);
@@ -542,7 +542,7 @@ static void on_receive_block(struct r3964_info *pInfo)
pBlock);
}
}
- wake_up_interruptible(&pInfo->read_wait);
+ wake_up_interruptible(&pInfo->tty->read_wait);
pInfo->state = R3964_IDLE;
@@ -979,7 +979,6 @@ static int r3964_open(struct tty_struct *tty)
spin_lock_init(&pInfo->lock);
pInfo->tty = tty;
- init_waitqueue_head(&pInfo->read_wait);
pInfo->priority = R3964_MASTER;
pInfo->rx_first = pInfo->rx_last = NULL;
pInfo->tx_first = pInfo->tx_last = NULL;
@@ -1045,7 +1044,6 @@ static void r3964_close(struct tty_struct *tty)
}
/* Free buffers: */
- wake_up_interruptible(&pInfo->read_wait);
kfree(pInfo->rx_buf);
TRACE_M("r3964_close - rx_buf kfree %p", pInfo->rx_buf);
kfree(pInfo->tx_buf);
@@ -1077,7 +1075,7 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
goto unlock;
}
/* block until there is a message: */
- wait_event_interruptible_tty(tty, pInfo->read_wait,
+ wait_event_interruptible_tty(tty, tty->read_wait,
(pMsg = remove_msg(pInfo, pClient)));
}
@@ -1227,7 +1225,7 @@ static unsigned int r3964_poll(struct tty_struct *tty, struct file *file,
pClient = findClient(pInfo, task_pid(current));
if (pClient) {
- poll_wait(file, &pInfo->read_wait, wait);
+ poll_wait(file, &tty->read_wait, wait);
spin_lock_irqsave(&pInfo->lock, flags);
pMsg = pClient->first_msg;
spin_unlock_irqrestore(&pInfo->lock, flags);
diff --git a/include/linux/n_r3964.h b/include/linux/n_r3964.h
index 5d0b2a1..e9adb42 100644
--- a/include/linux/n_r3964.h
+++ b/include/linux/n_r3964.h
@@ -152,9 +152,6 @@ struct r3964_info {
unsigned char *rx_buf; /* ring buffer */
unsigned char *tx_buf;
- wait_queue_head_t read_wait;
- //struct wait_queue *read_wait;
-
struct r3964_block_header *rx_first;
struct r3964_block_header *rx_last;
struct r3964_block_header *tx_first;
--
2.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/7] tty: r3964: Replace/remove bogus tty lock use
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
` (4 preceding siblings ...)
2015-10-10 20:00 ` [PATCH 5/7] tty: r3964: Use tty->read_wait waitqueue Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
2015-10-10 20:00 ` [PATCH 7/7] tty: Remove wait_event_interruptible_tty() Peter Hurley
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller, Peter Hurley
The tty lock is strictly for serializing tty lifetime events
(open/close/hangup), and not for line discipline serialization.
The tty core already provides serialization of concurrent writes
to the same tty, and line discipline lifetime management (by ldisc
references), so pinning the tty via tty_lock() is unnecessary and
counter-productive; remove tty lock use.
However, the line discipline is responsible for serializing reads
(if required by the line discipline); add read_lock mutex to
serialize calls of r3964_read().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_r3964.c | 20 +++++++++++++-------
include/linux/n_r3964.h | 5 +++--
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
index 6fdef92..3451114 100644
--- a/drivers/tty/n_r3964.c
+++ b/drivers/tty/n_r3964.c
@@ -978,6 +978,7 @@ static int r3964_open(struct tty_struct *tty)
}
spin_lock_init(&pInfo->lock);
+ mutex_init(&pInfo->read_lock);
pInfo->tty = tty;
pInfo->priority = R3964_MASTER;
pInfo->rx_first = pInfo->rx_last = NULL;
@@ -1063,7 +1064,16 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
TRACE_L("read()");
- tty_lock(tty);
+ /*
+ * Internal serialization of reads.
+ */
+ if (file->f_flags & O_NONBLOCK) {
+ if (!mutex_trylock(&pInfo->read_lock))
+ return -EAGAIN;
+ } else {
+ if (mutex_lock_interruptible(&pInfo->read_lock))
+ return -ERESTARTSYS;
+ }
pClient = findClient(pInfo, task_pid(current));
if (pClient) {
@@ -1075,7 +1085,7 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
goto unlock;
}
/* block until there is a message: */
- wait_event_interruptible_tty(tty, tty->read_wait,
+ wait_event_interruptible(tty->read_wait,
(pMsg = remove_msg(pInfo, pClient)));
}
@@ -1105,7 +1115,7 @@ static ssize_t r3964_read(struct tty_struct *tty, struct file *file,
}
ret = -EPERM;
unlock:
- tty_unlock(tty);
+ mutex_unlock(&pInfo->read_lock);
return ret;
}
@@ -1154,8 +1164,6 @@ static ssize_t r3964_write(struct tty_struct *tty, struct file *file,
pHeader->locks = 0;
pHeader->owner = NULL;
- tty_lock(tty);
-
pClient = findClient(pInfo, task_pid(current));
if (pClient) {
pHeader->owner = pClient;
@@ -1173,8 +1181,6 @@ static ssize_t r3964_write(struct tty_struct *tty, struct file *file,
add_tx_queue(pInfo, pHeader);
trigger_transmit(pInfo);
- tty_unlock(tty);
-
return 0;
}
diff --git a/include/linux/n_r3964.h b/include/linux/n_r3964.h
index e9adb42..90a803a 100644
--- a/include/linux/n_r3964.h
+++ b/include/linux/n_r3964.h
@@ -161,8 +161,9 @@ struct r3964_info {
unsigned char last_rx;
unsigned char bcc;
unsigned int blocks_in_rx_queue;
-
-
+
+ struct mutex read_lock; /* serialize r3964_read */
+
struct r3964_client_info *firstClient;
unsigned int state;
unsigned int flags;
--
2.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/7] tty: Remove wait_event_interruptible_tty()
2015-10-10 20:00 [PATCH 0/7] tty close cleanup Peter Hurley
` (5 preceding siblings ...)
2015-10-10 20:00 ` [PATCH 6/7] tty: r3964: Replace/remove bogus tty lock use Peter Hurley
@ 2015-10-10 20:00 ` Peter Hurley
6 siblings, 0 replies; 10+ messages in thread
From: Peter Hurley @ 2015-10-10 20:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Alan Cox, David Laight, Arnd Bergmann, linux-kernel,
linux-serial, netdev, David Miller, Peter Hurley
In-tree users of wait_event_interruptible_tty() have been removed;
remove.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
include/linux/tty.h | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 090ce2a..c2889f4 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -655,32 +655,6 @@ extern void __lockfunc tty_unlock(struct tty_struct *tty);
extern void __lockfunc tty_lock_slave(struct tty_struct *tty);
extern void __lockfunc tty_unlock_slave(struct tty_struct *tty);
extern void tty_set_lock_subclass(struct tty_struct *tty);
-/*
- * wait_event_interruptible_tty -- wait for a condition with the tty lock held
- *
- * The condition we are waiting for might take a long time to
- * become true, or might depend on another thread taking the
- * BTM. In either case, we need to drop the BTM to guarantee
- * forward progress. This is a leftover from the conversion
- * from the BKL and should eventually get removed as the BTM
- * falls out of use.
- *
- * Do not use in new code.
- */
-#define wait_event_interruptible_tty(tty, wq, condition) \
-({ \
- int __ret = 0; \
- if (!(condition)) \
- __ret = __wait_event_interruptible_tty(tty, wq, \
- condition); \
- __ret; \
-})
-
-#define __wait_event_interruptible_tty(tty, wq, condition) \
- ___wait_event(wq, condition, TASK_INTERRUPTIBLE, 0, 0, \
- tty_unlock(tty); \
- schedule(); \
- tty_lock(tty))
#ifdef CONFIG_PROC_FS
extern void proc_tty_register_driver(struct tty_driver *);
--
2.6.1
^ permalink raw reply related [flat|nested] 10+ messages in thread