tty close cleanup
Subsystems (6)
cover tty close cleanup
Hi Greg,
This series is a redux of cleanup I originally submitted back in 2014; the point is to remove the cruft which stems from dropping the tty lock while waiting for output to drain before closing the tty.
Dropping the tty lock during close was added by "TTY: define tty_wait_until_sent_from_close" to fix stalls on other parallel tty operations while a tty was draining output while closing.
Since commit 89c8d91e31f2 ("tty: localise the lock") and commit aa3cb814a8ef ("tty: Drop tty_mutex before tty reopen"), parallel tty open/close/hangup on _other_ ttys no longer stall waiting for a tty close to complete.
Continuing to hold the tty lock for the tty which is closing significantly simplifies the state handling when opening a tty, since the tty cannot have been closed concurrently. [Ideally, I would have liked to entirely remove the TTY_CLOSING state flag, but unfortunately some older bit-rotting drivers have co-opted it for dubious purposes].
In the previous series, David Laight had raised concerns about non-blocking opens on the _same_ tty which is closing. However, as I pointed out in this reply http://www.spinics.net/lists/linux-serial/msg14216.html, the outcome of a parallel open while closing the same tty has not changed with this series, since the existing code has blocked while ASYNC_CLOSING (and has done since at least before 2.6.29).
I cc'd the commenters from the original series, Felipe Balbi for the gserial usb gadget changes, and David Miller for the ISDN/IRDA changes.
Regards,
Peter Hurley (7):
tty: Remove tty_wait_until_sent_from_close()
tty: Remove ASYNC_CLOSING checks in open()/hangup() methods
usb: gadget: gserial: Privatize close_wait
tty: Remove tty_port::close_wait
tty: r3964: Use tty->read_wait waitqueue
tty: r3964: Replace/remove bogus tty lock use
tty: Remove wait_event_interruptible_tty()
drivers/char/pcmcia/synclink_cs.c | 9 ------- drivers/isdn/i4l/isdn_tty.c | 2 +- drivers/tty/cyclades.c | 9 ------- drivers/tty/hvc/hvc_console.c | 2 +- drivers/tty/hvc/hvcs.c | 2 +- drivers/tty/n_r3964.c | 28 ++++++++++++--------- drivers/tty/rocket.c | 13 ---------- drivers/tty/serial/68328serial.c | 1 - drivers/tty/serial/crisv10.c | 34 +------------------------ drivers/tty/serial/serial_core.c | 1 - drivers/tty/synclink.c | 18 +++----------- drivers/tty/synclink_gt.c | 14 ++--------- drivers/tty/synclinkmp.c | 14 ++--------- drivers/tty/tty_port.c | 26 +++----------------- drivers/usb/gadget/function/u_serial.c | 6 +++-- include/linux/n_r3964.h | 8 +++--- include/linux/tty.h | 45 ---------------------------------- net/irda/ircomm/ircomm_tty.c | 31 +---------------------- 18 files changed, 39 insertions(+), 224 deletions(-)
Signature · 1 line
2.6.1
PATCH 1/7 tty: Remove tty_wait_until_sent_from_close()
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.
--- 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(-)
drivers/isdn/i4l/isdn_tty.c +1 -1
@@ -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
drivers/tty/hvc/hvc_console.c +1 -1
@@ -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",
drivers/tty/hvc/hvcs.c +1 -1
@@ -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
drivers/tty/tty_port.c +2 -9
@@ -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)
include/linux/tty.h +0 -18
@@ -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
Signature · 6 lines
2.6.1 _______________________________________________ Linuxppc-dev mailing list [email protected] https://lists.ozlabs.org/listinfo/linuxppc-dev
PATCH 2/7 tty: Remove ASYNC_CLOSING checks in open()/hangup() methods
In reply to Peter HurleySince 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.
--- 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(-)
drivers/char/pcmcia/synclink_cs.c +0 -9
@@ -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);
drivers/tty/cyclades.c +0 -9
@@ -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);
drivers/tty/rocket.c +0 -12
@@ -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]);
drivers/tty/serial/crisv10.c +1 -32
@@ -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 ) {
drivers/tty/synclink.c +4 -14
@@ -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);
drivers/tty/synclink_gt.c +2 -12
@@ -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 ;
drivers/tty/synclinkmp.c +2 -12
@@ -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 ;
drivers/tty/tty_port.c +1 -12
@@ -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 ;
net/irda/ircomm/ircomm_tty.c +1 -30
@@ -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 ;
Signature · 1 line
2.6.1
Greg Kroah-Hartman
maintainerRe: [PATCH 2/7] tty: Remove ASYNC_CLOSING checks in open()/hangup() methods
Quoted 3 lines · Peter Hurley
Note: The
ASYNC_CLOSINGflag 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
PATCH 3/7 usb: gadget: gserial: Privatize close_wait
In reply to Peter Hurleyclose_wait is no longer needed or provided by the tty core. Move close_wait to struct gs_port.
--- drivers/usb/gadget/function/u_serial.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/u_serial.c +4 -2
@@ -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);
Signature · 1 line
2.6.1
Re: [PATCH 3/7] usb: gadget: gserial: Privatize close_wait
[ forgot to addr Felipe here, sorry ]
Quoted 47 lines · Peter Hurley
close_waitis no longer needed or provided by the tty core. Moveclose_waitto structgs_port.Signed-off-by Peter Hurley <[email protected]>--- drivers/usb/gadget/function/u_serial.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/usb/gadget/function/u_serial.c +4
-2@@ -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);
PATCH 4/7 tty: Remove tty_port::close_wait
In reply to Peter HurleyWith 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.
--- 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(-)
drivers/tty/rocket.c +0 -1
@@ -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);
drivers/tty/serial/68328serial.c +0 -1
@@ -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); }
drivers/tty/serial/crisv10.c +0 -1
@@ -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 */
drivers/tty/serial/serial_core.c +0 -1
@@ -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);
drivers/tty/tty_port.c +0 -2
@@ -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);
include/linux/tty.h +0 -1
@@ -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 */
Signature · 1 line
2.6.1
PATCH 5/7 tty: r3964: Use tty->read_wait waitqueue
In reply to Peter HurleyThe 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.
--- drivers/tty/n_r3964.c | 10 ++++------ include/linux/n_r3964.h | 3 --- 2 files changed, 4 insertions(+), 9 deletions(-)
drivers/tty/n_r3964.c +4 -6
@@ -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);
include/linux/n_r3964.h +0 -3
@@ -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;
Signature · 1 line
2.6.1
PATCH 6/7 tty: r3964: Replace/remove bogus tty lock use
In reply to Peter HurleyThe 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().
--- drivers/tty/n_r3964.c | 20 +++++++++++++------- include/linux/n_r3964.h | 5 +++-- 2 files changed, 16 insertions(+), 9 deletions(-)
drivers/tty/n_r3964.c +13 -7
@@ -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 ; }
include/linux/n_r3964.h +3 -2
@@ -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;
Signature · 1 line
2.6.1
PATCH 7/7 tty: Remove wait_event_interruptible_tty()
In reply to Peter HurleyIn-tree users of wait_event_interruptible_tty() have been removed; remove.
--- include/linux/tty.h | 26 -------------------------- 1 file changed, 26 deletions(-)
include/linux/tty.h +0 -26
@@ -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 *);
Signature · 1 line
2.6.1