From d945cb9cce20ac7143c2de8d88b187f62db99bdc Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Tue, 7 Jul 2009 16:39:41 +0100 Subject: pty: Rework the pty layer to use the normal buffering logic This fixes the ppp problems and various other issues with call locking caused by one side of a pty called in one locking context trying to match another with differing rules on the other side. We also get a big slack space to work with that means we can bury the flow control deadlock case for any conceivable real world situation. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/pty.c | 154 ++++++++++++++++++++--------------------------------- 1 file changed, 59 insertions(+), 95 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/pty.c b/drivers/char/pty.c index daebe1ba43d4..9d1b4f548f67 100644 --- a/drivers/char/pty.c +++ b/drivers/char/pty.c @@ -75,114 +75,88 @@ static void pty_close(struct tty_struct *tty, struct file *filp) */ static void pty_unthrottle(struct tty_struct *tty) { - struct tty_struct *o_tty = tty->link; - - if (!o_tty) - return; - - tty_wakeup(o_tty); + tty_wakeup(tty->link); set_bit(TTY_THROTTLED, &tty->flags); } -/* - * WSH 05/24/97: modified to - * (1) use space in tty->flip instead of a shared temp buffer - * The flip buffers aren't being used for a pty, so there's lots - * of space available. The buffer is protected by a per-pty - * semaphore that should almost never come under contention. - * (2) avoid redundant copying for cases where count >> receive_room - * N.B. Calls from user space may now return an error code instead of - * a count. +/** + * pty_space - report space left for writing + * @to: tty we are writing into * - * FIXME: Our pty_write method is called with our ldisc lock held but - * not our partners. We can't just wait on the other one blindly without - * risking deadlocks. At some point when everything has settled down we need - * to look into making pty_write at least able to sleep over an ldisc change. + * The tty buffers allow 64K but we sneak a peak and clip at 8K this + * allows a lot of overspill room for echo and other fun messes to + * be handled properly + */ + +static int pty_space(struct tty_struct *to) +{ + int n = 8192 - to->buf.memory_used; + if (n < 0) + return 0; + return n; +} + +/** + * pty_write - write to a pty + * @tty: the tty we write from + * @buf: kernel buffer of data + * @count: bytes to write * - * The return on no ldisc is a bit counter intuitive but the logic works - * like this. During an ldisc change the other end will flush its buffers. We - * thus return the full length which is identical to the case where we had - * proper locking and happened to queue the bytes just before the flush during - * the ldisc change. + * Our "hardware" write method. Data is coming from the ldisc which + * may be in a non sleeping state. We simply throw this at the other + * end of the link as if we were an IRQ handler receiving stuff for + * the other side of the pty/tty pair. */ + static int pty_write(struct tty_struct *tty, const unsigned char *buf, int count) { struct tty_struct *to = tty->link; - struct tty_ldisc *ld; - int c = count; + int c; - if (!to || tty->stopped) + if (tty->stopped) return 0; - ld = tty_ldisc_ref(to); - - if (ld) { - c = to->receive_room; - if (c > count) - c = count; - ld->ops->receive_buf(to, buf, NULL, c); - tty_ldisc_deref(ld); + + /* This isn't locked but our 8K is quite sloppy so no + big deal */ + + c = pty_space(to); + if (c > count) + c = count; + if (c > 0) { + /* Stuff the data into the input queue of the other end */ + c = tty_insert_flip_string(to, buf, c); + /* And shovel */ + tty_flip_buffer_push(to); + tty_wakeup(tty); } return c; } +/** + * pty_write_room - write space + * @tty: tty we are writing from + * + * Report how many bytes the ldisc can send into the queue for + * the other device. + */ + static int pty_write_room(struct tty_struct *tty) { - struct tty_struct *to = tty->link; - - if (!to || tty->stopped) - return 0; - - return to->receive_room; + return pty_space(tty->link); } -/* - * WSH 05/24/97: Modified for asymmetric MASTER/SLAVE behavior - * The chars_in_buffer() value is used by the ldisc select() function - * to hold off writing when chars_in_buffer > WAKEUP_CHARS (== 256). - * The pty driver chars_in_buffer() Master/Slave must behave differently: - * - * The Master side needs to allow typed-ahead commands to accumulate - * while being canonicalized, so we report "our buffer" as empty until - * some threshold is reached, and then report the count. (Any count > - * WAKEUP_CHARS is regarded by select() as "full".) To avoid deadlock - * the count returned must be 0 if no canonical data is available to be - * read. (The N_TTY ldisc.chars_in_buffer now knows this.) +/** + * pty_chars_in_buffer - characters currently in our tx queue + * @tty: our tty * - * The Slave side passes all characters in raw mode to the Master side's - * buffer where they can be read immediately, so in this case we can - * return the true count in the buffer. + * Report how much we have in the transmit queue. As everything is + * instantly at the other end this is easy to implement. */ + static int pty_chars_in_buffer(struct tty_struct *tty) { - struct tty_struct *to = tty->link; - struct tty_ldisc *ld; - int count = 0; - - /* We should get the line discipline lock for "tty->link" */ - if (!to) - return 0; - /* We cannot take a sleeping reference here without deadlocking with - an ldisc change - but it doesn't really matter */ - ld = tty_ldisc_ref(to); - if (ld == NULL) - return 0; - - /* The ldisc must report 0 if no characters available to be read */ - if (ld->ops->chars_in_buffer) - count = ld->ops->chars_in_buffer(to); - - tty_ldisc_deref(ld); - - if (tty->driver->subtype == PTY_TYPE_SLAVE) - return count; - - /* Master side driver ... if the other side's read buffer is less than - * half full, return 0 to allow writers to proceed; otherwise return - * the count. This leaves a comfortable margin to avoid overflow, - * and still allows half a buffer's worth of typed-ahead commands. - */ - return (count < N_TTY_BUF_SIZE/2) ? 0 : count; + return 0; } /* Set the lock flag on a pty */ @@ -202,20 +176,10 @@ static void pty_flush_buffer(struct tty_struct *tty) { struct tty_struct *to = tty->link; unsigned long flags; - struct tty_ldisc *ld; if (!to) return; - ld = tty_ldisc_ref(to); - - /* The other end is changing discipline */ - if (!ld) - return; - - if (ld->ops->flush_buffer) - to->ldisc->ops->flush_buffer(to); - tty_ldisc_deref(ld); - + /* tty_buffer_flush(to); FIXME */ if (to->packet) { spin_lock_irqsave(&tty->ctrl_lock, flags); tty->ctrl_status |= TIOCPKT_FLUSHWRITE; -- cgit v1.2.3 From ad361c9884e809340f6daca80d56a9e9c871690a Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Mon, 6 Jul 2009 13:05:40 -0700 Subject: Remove multiple KERN_ prefixes from printk formats Commit 5fd29d6ccbc98884569d6f3105aeca70858b3e0f ("printk: clean up handling of log-levels and newlines") changed printk semantics. printk lines with multiple KERN_ prefixes are no longer emitted as before the patch. is now included in the output on each additional use. Remove all uses of multiple KERN_s in formats. Signed-off-by: Joe Perches Signed-off-by: Linus Torvalds --- drivers/char/hw_random/intel-rng.c | 9 +++++---- drivers/char/isicom.c | 16 ++++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/hw_random/intel-rng.c b/drivers/char/hw_random/intel-rng.c index 5dcbe603eca2..91b53eb1c053 100644 --- a/drivers/char/hw_random/intel-rng.c +++ b/drivers/char/hw_random/intel-rng.c @@ -305,10 +305,11 @@ static int __init intel_init_hw_struct(struct intel_rng_hw *intel_rng_hw, (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK)) == BIOS_CNTL_LOCK_ENABLE_MASK) { static __initdata /*const*/ char warning[] = - KERN_WARNING PFX "Firmware space is locked read-only. If you can't or\n" - KERN_WARNING PFX "don't want to disable this in firmware setup, and if\n" - KERN_WARNING PFX "you are certain that your system has a functional\n" - KERN_WARNING PFX "RNG, try using the 'no_fwh_detect' option.\n"; + KERN_WARNING +PFX "Firmware space is locked read-only. If you can't or\n" +PFX "don't want to disable this in firmware setup, and if\n" +PFX "you are certain that your system has a functional\n" +PFX "RNG, try using the 'no_fwh_detect' option.\n"; if (no_fwh_detect) return -ENODEV; diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c index 4159292e35cf..621d1184673c 100644 --- a/drivers/char/isicom.c +++ b/drivers/char/isicom.c @@ -1478,10 +1478,10 @@ static int __devinit load_firmware(struct pci_dev *pdev, status = inw(base + 0x4); if (status != 0) { dev_warn(&pdev->dev, "Card%d rejected load header:\n" - KERN_WARNING "Address:0x%x\n" - KERN_WARNING "Count:0x%x\n" - KERN_WARNING "Status:0x%x\n", - index + 1, frame->addr, frame->count, status); + "Address:0x%x\n" + "Count:0x%x\n" + "Status:0x%x\n", + index + 1, frame->addr, frame->count, status); goto errrelfw; } outsw(base, frame->data, word_count); @@ -1526,10 +1526,10 @@ static int __devinit load_firmware(struct pci_dev *pdev, status = inw(base + 0x4); if (status != 0) { dev_warn(&pdev->dev, "Card%d rejected verify header:\n" - KERN_WARNING "Address:0x%x\n" - KERN_WARNING "Count:0x%x\n" - KERN_WARNING "Status: 0x%x\n", - index + 1, frame->addr, frame->count, status); + "Address:0x%x\n" + "Count:0x%x\n" + "Status: 0x%x\n", + index + 1, frame->addr, frame->count, status); goto errrelfw; } -- cgit v1.2.3 From 405f55712dfe464b3240d7816cc4fe4174831be2 Mon Sep 17 00:00:00 2001 From: Alexey Dobriyan Date: Sat, 11 Jul 2009 22:08:37 +0400 Subject: headers: smp_lock.h redux * Remove smp_lock.h from files which don't need it (including some headers!) * Add smp_lock.h to files which do need it * Make smp_lock.h include conditional in hardirq.h It's needed only for one kernel_locked() usage which is under CONFIG_PREEMPT This will make hardirq.h inclusion cheaper for every PREEMPT=n config (which includes allmodconfig/allyesconfig, BTW) Signed-off-by: Alexey Dobriyan Signed-off-by: Linus Torvalds --- drivers/char/amiserial.c | 1 + drivers/char/cyclades.c | 1 + drivers/char/epca.c | 1 + drivers/char/isicom.c | 1 + drivers/char/istallion.c | 1 + drivers/char/moxa.c | 1 + drivers/char/mxser.c | 1 + drivers/char/n_hdlc.c | 1 + drivers/char/n_r3964.c | 1 + drivers/char/pty.c | 1 + drivers/char/rio/rio_linux.c | 1 + drivers/char/riscom8.c | 1 + drivers/char/rocket.c | 1 + drivers/char/serial167.c | 1 + drivers/char/specialix.c | 1 + drivers/char/sx.c | 1 + drivers/char/synclink.c | 1 + drivers/char/synclink_gt.c | 1 + drivers/char/synclinkmp.c | 1 + drivers/char/tpm/tpm.c | 1 - drivers/char/tty_ioctl.c | 1 - drivers/char/tty_ldisc.c | 1 - drivers/char/vt.c | 1 + drivers/char/vt_ioctl.c | 1 + 24 files changed, 21 insertions(+), 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/amiserial.c b/drivers/char/amiserial.c index 72429b6b2fa8..6c32fbf07164 100644 --- a/drivers/char/amiserial.c +++ b/drivers/char/amiserial.c @@ -81,6 +81,7 @@ static char *serial_version = "4.30"; #include #include #include +#include #include #include diff --git a/drivers/char/cyclades.c b/drivers/char/cyclades.c index f3366d3f06cf..2dafc2da0648 100644 --- a/drivers/char/cyclades.c +++ b/drivers/char/cyclades.c @@ -633,6 +633,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/epca.c b/drivers/char/epca.c index abef1f7d84fe..ff647ca1c489 100644 --- a/drivers/char/epca.c +++ b/drivers/char/epca.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c index 621d1184673c..4f1f4cd670da 100644 --- a/drivers/char/isicom.c +++ b/drivers/char/isicom.c @@ -122,6 +122,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/istallion.c b/drivers/char/istallion.c index 0c999f5bb3db..ab2f3349c5c4 100644 --- a/drivers/char/istallion.c +++ b/drivers/char/istallion.c @@ -20,6 +20,7 @@ #include #include +#include #include #include #include diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c index 65b6ff2442c6..dd0083bbb64a 100644 --- a/drivers/char/moxa.c +++ b/drivers/char/moxa.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c index 52d953eb30c3..dbf8d52f31d0 100644 --- a/drivers/char/mxser.c +++ b/drivers/char/mxser.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/n_hdlc.c b/drivers/char/n_hdlc.c index 1c43c8cdee25..c68118efad84 100644 --- a/drivers/char/n_hdlc.c +++ b/drivers/char/n_hdlc.c @@ -97,6 +97,7 @@ #include #include #include +#include #include /* used in new tty drivers */ #include /* used in new tty drivers */ #include diff --git a/drivers/char/n_r3964.c b/drivers/char/n_r3964.c index 2e99158ebb8a..6934025a1ac1 100644 --- a/drivers/char/n_r3964.c +++ b/drivers/char/n_r3964.c @@ -58,6 +58,7 @@ #include #include #include +#include #include #include #include /* used in new tty drivers */ diff --git a/drivers/char/pty.c b/drivers/char/pty.c index 9d1b4f548f67..6e6942c45f5b 100644 --- a/drivers/char/pty.c +++ b/drivers/char/pty.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/rio/rio_linux.c b/drivers/char/rio/rio_linux.c index ce81da5b2da9..d58c2eb07f07 100644 --- a/drivers/char/rio/rio_linux.c +++ b/drivers/char/rio/rio_linux.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include diff --git a/drivers/char/riscom8.c b/drivers/char/riscom8.c index 217660451237..171711acf5cd 100644 --- a/drivers/char/riscom8.c +++ b/drivers/char/riscom8.c @@ -47,6 +47,7 @@ #include #include #include +#include #include #include diff --git a/drivers/char/rocket.c b/drivers/char/rocket.c index 63d5b628477a..0e29a23ec4c5 100644 --- a/drivers/char/rocket.c +++ b/drivers/char/rocket.c @@ -73,6 +73,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/serial167.c b/drivers/char/serial167.c index f1f24f0ee26f..51e7a46787be 100644 --- a/drivers/char/serial167.c +++ b/drivers/char/serial167.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/specialix.c b/drivers/char/specialix.c index e72be4190a44..bfe4cdb2febb 100644 --- a/drivers/char/specialix.c +++ b/drivers/char/specialix.c @@ -87,6 +87,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/sx.c b/drivers/char/sx.c index 518f2a25d91e..a81ec4fcf6ff 100644 --- a/drivers/char/sx.c +++ b/drivers/char/sx.c @@ -216,6 +216,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/synclink.c b/drivers/char/synclink.c index afded3a2379c..813552f14884 100644 --- a/drivers/char/synclink.c +++ b/drivers/char/synclink.c @@ -81,6 +81,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/synclink_gt.c b/drivers/char/synclink_gt.c index a2e67e6df3a1..91f20a92fddf 100644 --- a/drivers/char/synclink_gt.c +++ b/drivers/char/synclink_gt.c @@ -62,6 +62,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/synclinkmp.c b/drivers/char/synclinkmp.c index 6f727e3c53ad..8d4a2a8a0a70 100644 --- a/drivers/char/synclinkmp.c +++ b/drivers/char/synclinkmp.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c index ccdd828adcef..b0603b2e5684 100644 --- a/drivers/char/tpm/tpm.c +++ b/drivers/char/tpm/tpm.c @@ -26,7 +26,6 @@ #include #include #include -#include #include "tpm.h" diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c index b24f6c6a1ea3..ad6ba4ed2808 100644 --- a/drivers/char/tty_ioctl.c +++ b/drivers/char/tty_ioctl.c @@ -21,7 +21,6 @@ #include #include #include -#include #include #include diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index 913aa8d3f1c5..0ef0dc97ba20 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -21,7 +21,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/char/vt.c b/drivers/char/vt.c index d9113b4c76e3..7947bd1b4cf7 100644 --- a/drivers/char/vt.c +++ b/drivers/char/vt.c @@ -89,6 +89,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c index 7539bed0f7e0..95189f288f8c 100644 --- a/drivers/char/vt_ioctl.c +++ b/drivers/char/vt_ioctl.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include -- cgit v1.2.3 From c8d50041734534e0a4b0ea13df36ed5857fccd56 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Thu, 16 Jul 2009 16:05:08 +0100 Subject: tty: fix close/hangup race We can get a situation where a hangup occurs during or after a close. In that case the ldisc gets disposed of by the close and the hangup then explodes. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/tty_ldisc.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index 0ef0dc97ba20..acd76b767d4c 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -790,17 +790,20 @@ void tty_ldisc_hangup(struct tty_struct *tty) * N_TTY. */ if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) { - /* Avoid racing set_ldisc */ + /* Avoid racing set_ldisc or tty_ldisc_release */ mutex_lock(&tty->ldisc_mutex); - /* Switch back to N_TTY */ - tty_ldisc_halt(tty); - tty_ldisc_wait_idle(tty); - tty_ldisc_reinit(tty); - /* At this point we have a closed ldisc and we want to - reopen it. We could defer this to the next open but - it means auditing a lot of other paths so this is a FIXME */ - WARN_ON(tty_ldisc_open(tty, tty->ldisc)); - tty_ldisc_enable(tty); + if (tty->ldisc) { /* Not yet closed */ + /* Switch back to N_TTY */ + tty_ldisc_halt(tty); + tty_ldisc_wait_idle(tty); + tty_ldisc_reinit(tty); + /* At this point we have a closed ldisc and we want to + reopen it. We could defer this to the next open but + it means auditing a lot of other paths so this is + a FIXME */ + WARN_ON(tty_ldisc_open(tty, tty->ldisc)); + tty_ldisc_enable(tty); + } mutex_unlock(&tty->ldisc_mutex); tty_reset_termios(tty); } @@ -865,6 +868,7 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) tty_ldisc_wait_idle(tty); + mutex_lock(&tty->ldisc_mutex); /* * Now kill off the ldisc */ @@ -875,6 +879,7 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) /* Ensure the next open requests the N_TTY ldisc */ tty_set_termios_ldisc(tty, N_TTY); + mutex_unlock(&tty->ldisc_mutex); /* This will need doing differently if we need to lock */ if (o_tty) -- cgit v1.2.3 From 5c9228f0cfb09a098a8a380116b42ae099e967b6 Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Thu, 16 Jul 2009 16:06:09 +0100 Subject: vt: drop bootmem/slab memory distinction Bootmem is not used for the vt screen buffer anymore as slab is now available at the time the console is initialized. Get rid of the now superfluous distinction between slab and bootmem, it's always slab. This also fixes a kmalloc leak which Catalin described thusly: Commit a5f4f52e ("vt: use kzalloc() instead of the bootmem allocator") replaced the alloc_bootmem() with kzalloc() but didn't set vc_kmalloced to 1 and the memory block is later leaked. The corresponding kmemleak trace: unreferenced object 0xdf828000 (size 8192): comm "swapper", pid 0, jiffies 4294937296 backtrace: [] __save_stack_trace+0x17/0x1c [] log_early+0x55/0x84 [] kmemleak_alloc+0x33/0x3c [] __kmalloc+0xd7/0xe4 [] con_init+0xbf/0x1b8 [] console_init+0x11/0x20 [] start_kernel+0x137/0x1e4 Signed-off-by: Johannes Weiner Reviewed-by: Pekka Enberg Tested-by: Catalin Marinas Signed-off-by: Andrew Morton Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/vt.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/vt.c b/drivers/char/vt.c index 7947bd1b4cf7..404f4c1ee431 100644 --- a/drivers/char/vt.c +++ b/drivers/char/vt.c @@ -770,14 +770,12 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ visual_init(vc, currcons, 1); if (!*vc->vc_uni_pagedir_loc) con_set_default_unimap(vc); - if (!vc->vc_kmalloced) - vc->vc_screenbuf = kmalloc(vc->vc_screenbuf_size, GFP_KERNEL); + vc->vc_screenbuf = kmalloc(vc->vc_screenbuf_size, GFP_KERNEL); if (!vc->vc_screenbuf) { kfree(vc); vc_cons[currcons].d = NULL; return -ENOMEM; } - vc->vc_kmalloced = 1; vc_init(vc, vc->vc_rows, vc->vc_cols, 1); vcs_make_sysfs(currcons); atomic_notifier_call_chain(&vt_notifier_list, VT_ALLOCATE, ¶m); @@ -913,10 +911,8 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc, if (new_scr_end > new_origin) scr_memsetw((void *)new_origin, vc->vc_video_erase_char, new_scr_end - new_origin); - if (vc->vc_kmalloced) - kfree(vc->vc_screenbuf); + kfree(vc->vc_screenbuf); vc->vc_screenbuf = newscreen; - vc->vc_kmalloced = 1; vc->vc_screenbuf_size = new_screen_size; set_origin(vc); @@ -995,8 +991,7 @@ void vc_deallocate(unsigned int currcons) vc->vc_sw->con_deinit(vc); put_pid(vc->vt_pid); module_put(vc->vc_sw->owner); - if (vc->vc_kmalloced) - kfree(vc->vc_screenbuf); + kfree(vc->vc_screenbuf); if (currcons >= MIN_NR_CONSOLES) kfree(vc); vc_cons[currcons].d = NULL; @@ -2881,7 +2876,6 @@ static int __init con_init(void) INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK); visual_init(vc, currcons, 1); vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT); - vc->vc_kmalloced = 0; vc_init(vc, vc->vc_rows, vc->vc_cols, currcons || !vc->vc_sw->con_save_screen); } -- cgit v1.2.3 From 9237a81a1468d0aca1cc4e244bba2362d6f81b35 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Thu, 16 Jul 2009 16:06:18 +0100 Subject: tty: nozomi, fix tty refcounting bug Don't forget to drop a tty refererence on fail paths in receive_data(). Signed-off-by: Jiri Slaby Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/nozomi.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c index 574f1c79b6e6..280b41c507a7 100644 --- a/drivers/char/nozomi.c +++ b/drivers/char/nozomi.c @@ -828,7 +828,7 @@ static int receive_data(enum port_type index, struct nozomi *dc) struct port *port = &dc->port[index]; void __iomem *addr = port->dl_addr[port->toggle_dl]; struct tty_struct *tty = tty_port_tty_get(&port->port); - int i; + int i, ret; if (unlikely(!tty)) { DBG1("tty not open for port: %d?", index); @@ -844,12 +844,14 @@ static int receive_data(enum port_type index, struct nozomi *dc) /* disable interrupt in downlink... */ disable_transmit_dl(index, dc); - return 0; + ret = 0; + goto put; } if (unlikely(size == 0)) { dev_err(&dc->pdev->dev, "size == 0?\n"); - return 1; + ret = 1; + goto put; } tty_buffer_request_room(tty, size); @@ -871,8 +873,10 @@ static int receive_data(enum port_type index, struct nozomi *dc) } set_bit(index, &dc->flip); + ret = 1; +put: tty_kref_put(tty); - return 1; + return ret; } /* Debug for interrupts */ -- cgit v1.2.3 From 807708844979ba8c6d5717345a8608454992696d Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Thu, 16 Jul 2009 16:07:03 +0100 Subject: n_tty: Fix echo race If a tty in N_TTY mode with echo enabled manages to get itself into a state where - echo characters are pending - FASYNC is enabled - tty_write_wakeup is called from either - a device write path (pty) - an IRQ (serial) then it either deadlocks or explodes taking a mutex in the IRQ path. On the serial side it is almost impossible to reproduce because you have to go from a full serial port to a near empty one with echo characters pending. The pty case happens to have become possible to trigger using emacs and ptys, the pty changes having created a scenario which shows up this bug. The code path is n_tty:process_echoes() (takes mutex) tty_io:tty_put_char() pty:pty_write (or serial paths) tty_wakeup (from pty_write or serial IRQ) n_tty_write_wakeup() process_echoes() *KABOOM* Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/n_tty.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c index 94a5d5020abc..ff47907ff1bf 100644 --- a/drivers/char/n_tty.c +++ b/drivers/char/n_tty.c @@ -1331,9 +1331,6 @@ handle_newline: static void n_tty_write_wakeup(struct tty_struct *tty) { - /* Write out any echoed characters that are still pending */ - process_echoes(tty); - if (tty->fasync && test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) kill_fasync(&tty->fasync, SIGIO, POLL_OUT); } -- cgit v1.2.3 From ecc2e05e739c30870c8e4f252b63a0c4041f2724 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Fri, 17 Jul 2009 16:17:26 +0100 Subject: tty_port: Fix return on interrupted use Whoops.. fortunately not many people use this yet. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/tty_port.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/tty_port.c b/drivers/char/tty_port.c index 4e862a75f7ff..9769b1149f76 100644 --- a/drivers/char/tty_port.c +++ b/drivers/char/tty_port.c @@ -267,7 +267,7 @@ int tty_port_block_til_ready(struct tty_port *port, if (retval == 0) port->flags |= ASYNC_NORMAL_ACTIVE; spin_unlock_irqrestore(&port->lock, flags); - return 0; + return retval; } EXPORT_SYMBOL(tty_port_block_til_ready); -- cgit v1.2.3 From c46a7aec556ffdbdb7357db0b05904b176cb3375 Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Mon, 20 Jul 2009 16:04:55 +0100 Subject: vc: create vcs(a) devices for consoles The buffer for the consoles are unconditionally allocated at con_init() time, which miss the creation of the vcs(a) devices. Since 2.6.30 (commit 4995f8ef9d3aac72745e12419d7fbaa8d01b1d81, 'vcs: hook sysfs devices into object lifetime instead of "binding"' to be exact) these devices are no longer created at open() and removed on close(), but controlled by the lifetime of the buffers. Reported-by: Gerardo Exequiel Pozzi Tested-by: Gerardo Exequiel Pozzi Cc: stable@kernel.org Signed-off-by: Kay Sievers Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/vc_screen.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/vc_screen.c b/drivers/char/vc_screen.c index d94d25c12aa8..c1791a63d99d 100644 --- a/drivers/char/vc_screen.c +++ b/drivers/char/vc_screen.c @@ -495,11 +495,15 @@ void vcs_remove_sysfs(int index) int __init vcs_init(void) { + unsigned int i; + if (register_chrdev(VCS_MAJOR, "vcs", &vcs_fops)) panic("unable to get major %d for vcs device", VCS_MAJOR); vc_class = class_create(THIS_MODULE, "vc"); device_create(vc_class, NULL, MKDEV(VCS_MAJOR, 0), NULL, "vcs"); device_create(vc_class, NULL, MKDEV(VCS_MAJOR, 128), NULL, "vcsa"); + for (i = 0; i < MIN_NR_CONSOLES; i++) + vcs_make_sysfs(i); return 0; } -- cgit v1.2.3 From 254702568da63ce6f5ad68e77d83b427da693654 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Mon, 20 Jul 2009 16:05:07 +0100 Subject: specialix.c: convert nested spin_lock_irqsave to spin_lock If spin_lock_irqsave is called twice in a row with the same second argument, the interrupt state at the point of the second call overwrites the value saved by the first call. Indeed, the second call does not need to save the interrupt state, so it is changed to a simple spin_lock. The semantic match that finds this problem is as follows: (http://www.emn.fr/x-info/coccinelle/) // @@ expression lock1,lock2; expression flags; @@ *spin_lock_irqsave(lock1,flags) ... when != flags *spin_lock_irqsave(lock2,flags) // Signed-off-by: Julia Lawall Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/specialix.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/specialix.c b/drivers/char/specialix.c index bfe4cdb2febb..268e17f9ec3f 100644 --- a/drivers/char/specialix.c +++ b/drivers/char/specialix.c @@ -1809,10 +1809,10 @@ static int sx_tiocmset(struct tty_struct *tty, struct file *file, if (clear & TIOCM_DTR) port->MSVR &= ~MSVR_DTR; } - spin_lock_irqsave(&bp->lock, flags); + spin_lock(&bp->lock); sx_out(bp, CD186x_CAR, port_No(port)); sx_out(bp, CD186x_MSVR, port->MSVR); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); spin_unlock_irqrestore(&port->lock, flags); func_exit(); return 0; @@ -1833,11 +1833,11 @@ static int sx_send_break(struct tty_struct *tty, int length) port->break_length = SPECIALIX_TPS / HZ * length; port->COR2 |= COR2_ETC; port->IER |= IER_TXRDY; - spin_lock_irqsave(&bp->lock, flags); + spin_lock(&bp->lock); sx_out(bp, CD186x_CAR, port_No(port)); sx_out(bp, CD186x_COR2, port->COR2); sx_out(bp, CD186x_IER, port->IER); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); spin_unlock_irqrestore(&port->lock, flags); sx_wait_CCR(bp); spin_lock_irqsave(&bp->lock, flags); @@ -2023,9 +2023,9 @@ static void sx_unthrottle(struct tty_struct *tty) if (sx_crtscts(tty)) port->MSVR |= MSVR_DTR; /* Else clause: see remark in "sx_throttle"... */ - spin_lock_irqsave(&bp->lock, flags); + spin_lock(&bp->lock); sx_out(bp, CD186x_CAR, port_No(port)); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); if (I_IXOFF(tty)) { spin_unlock_irqrestore(&port->lock, flags); sx_wait_CCR(bp); @@ -2035,9 +2035,9 @@ static void sx_unthrottle(struct tty_struct *tty) sx_wait_CCR(bp); spin_lock_irqsave(&port->lock, flags); } - spin_lock_irqsave(&bp->lock, flags); + spin_lock(&bp->lock); sx_out(bp, CD186x_MSVR, port->MSVR); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); spin_unlock_irqrestore(&port->lock, flags); func_exit(); @@ -2061,10 +2061,10 @@ static void sx_stop(struct tty_struct *tty) spin_lock_irqsave(&port->lock, flags); port->IER &= ~IER_TXRDY; - spin_lock_irqsave(&bp->lock, flags); + spin_lock(&bp->lock); sx_out(bp, CD186x_CAR, port_No(port)); sx_out(bp, CD186x_IER, port->IER); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); spin_unlock_irqrestore(&port->lock, flags); func_exit(); @@ -2089,10 +2089,10 @@ static void sx_start(struct tty_struct *tty) spin_lock_irqsave(&port->lock, flags); if (port->xmit_cnt && port->xmit_buf && !(port->IER & IER_TXRDY)) { port->IER |= IER_TXRDY; - spin_lock_irqsave(&bp->lock, flags); + spin_lock(&bp->lock); sx_out(bp, CD186x_CAR, port_No(port)); sx_out(bp, CD186x_IER, port->IER); - spin_unlock_irqrestore(&bp->lock, flags); + spin_unlock(&bp->lock); } spin_unlock_irqrestore(&port->lock, flags); -- cgit v1.2.3 From 23198fda7182969b619613a555f8645fdc3dc334 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Mon, 20 Jul 2009 16:05:27 +0100 Subject: tty: fix chars_in_buffers This function does not have an error return and returning an error is instead interpreted as having a lot of pending bytes. Reported by Jeff Harris who provided a list of some of the remaining offenders. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/hvc_console.c | 2 +- drivers/char/nozomi.c | 4 +--- drivers/char/pcmcia/ipwireless/tty.c | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c index 94e7e3c8c05a..d97779ef72cb 100644 --- a/drivers/char/hvc_console.c +++ b/drivers/char/hvc_console.c @@ -552,7 +552,7 @@ static int hvc_chars_in_buffer(struct tty_struct *tty) struct hvc_struct *hp = tty->driver_data; if (!hp) - return -1; + return 0; return hp->n_outbuf; } diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c index 280b41c507a7..ec58d8c387ff 100644 --- a/drivers/char/nozomi.c +++ b/drivers/char/nozomi.c @@ -1866,16 +1866,14 @@ static s32 ntty_chars_in_buffer(struct tty_struct *tty) { struct port *port = tty->driver_data; struct nozomi *dc = get_dc_by_tty(tty); - s32 rval; + s32 rval = 0; if (unlikely(!dc || !port)) { - rval = -ENODEV; goto exit_in_buffer; } if (unlikely(!port->port.count)) { dev_err(&dc->pdev->dev, "No tty open?\n"); - rval = -ENODEV; goto exit_in_buffer; } diff --git a/drivers/char/pcmcia/ipwireless/tty.c b/drivers/char/pcmcia/ipwireless/tty.c index 569f2f7743a7..674b3ab3587d 100644 --- a/drivers/char/pcmcia/ipwireless/tty.c +++ b/drivers/char/pcmcia/ipwireless/tty.c @@ -320,10 +320,10 @@ static int ipw_chars_in_buffer(struct tty_struct *linux_tty) struct ipw_tty *tty = linux_tty->driver_data; if (!tty) - return -ENODEV; + return 0; if (!tty->open_count) - return -EINVAL; + return 0; return tty->tx_bytes_queued; } -- cgit v1.2.3 From 3a54297478e6578f96fd54bf4daa1751130aca86 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Mon, 27 Jul 2009 22:17:51 +0100 Subject: pty: quickfix for the pty ENXIO timing problems This also makes close stall in the normal case which is apparently needed to fix emacs Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/char/pty.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/pty.c b/drivers/char/pty.c index 6e6942c45f5b..3850a68f265a 100644 --- a/drivers/char/pty.c +++ b/drivers/char/pty.c @@ -52,6 +52,7 @@ static void pty_close(struct tty_struct *tty, struct file *filp) return; tty->link->packet = 0; set_bit(TTY_OTHER_CLOSED, &tty->link->flags); + tty_flip_buffer_push(tty->link); wake_up_interruptible(&tty->link->read_wait); wake_up_interruptible(&tty->link->write_wait); if (tty->driver->subtype == PTY_TYPE_MASTER) { @@ -207,6 +208,7 @@ static int pty_open(struct tty_struct *tty, struct file *filp) clear_bit(TTY_OTHER_CLOSED, &tty->link->flags); set_bit(TTY_THROTTLED, &tty->flags); retval = 0; + tty->low_latency = 1; out: return retval; } -- cgit v1.2.3 From 68dbcb726e372b3c8ef60b79b5aff4174dd2bdf0 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 28 Jul 2009 22:36:59 -0400 Subject: Remove zero-length file drivers/char/vr41xx_giu.c Signed-off-by: Jeff Garzik --- drivers/char/vr41xx_giu.c | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 drivers/char/vr41xx_giu.c (limited to 'drivers/char') diff --git a/drivers/char/vr41xx_giu.c b/drivers/char/vr41xx_giu.c deleted file mode 100644 index e69de29bb2d1..000000000000 -- cgit v1.2.3 From e043e42bdb66885b3ac10d27a01ccb9972e2b0a3 Mon Sep 17 00:00:00 2001 From: OGAWA Hirofumi Date: Wed, 29 Jul 2009 12:15:56 -0700 Subject: pty: avoid forcing 'low_latency' tty flag We really don't want to mark the pty as a low-latency device, because as Alan points out, the ->write method can be called from an IRQ (ppp?), and that means we can't use ->low_latency=1 as we take mutexes in the low_latency case. So rather than using low_latency to force the written data to be pushed to the ldisc handling at 'write()' time, just make the reader side (or the poll function) do the flush when it checks whether there is data to be had. This also fixes the problem with lost data in an emacs compile buffer (bugzilla 13815), and we can thus revert the low_latency pty hack (commit 3a54297478e6578f96fd54bf4daa1751130aca86: "pty: quickfix for the pty ENXIO timing problems"). Signed-off-by: OGAWA Hirofumi Tested-by: Aneesh Kumar K.V [ Modified to do the tty_flush_to_ldisc() inside input_available_p() so that it triggers for both read and poll() - Linus] Signed-off-by: Linus Torvalds --- drivers/char/n_tty.c | 1 + drivers/char/pty.c | 2 -- drivers/char/tty_buffer.c | 13 +++++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c index ff47907ff1bf..973be2f44195 100644 --- a/drivers/char/n_tty.c +++ b/drivers/char/n_tty.c @@ -1583,6 +1583,7 @@ static int n_tty_open(struct tty_struct *tty) static inline int input_available_p(struct tty_struct *tty, int amt) { + tty_flush_to_ldisc(tty); if (tty->icanon) { if (tty->canon_data) return 1; diff --git a/drivers/char/pty.c b/drivers/char/pty.c index 3850a68f265a..6e6942c45f5b 100644 --- a/drivers/char/pty.c +++ b/drivers/char/pty.c @@ -52,7 +52,6 @@ static void pty_close(struct tty_struct *tty, struct file *filp) return; tty->link->packet = 0; set_bit(TTY_OTHER_CLOSED, &tty->link->flags); - tty_flip_buffer_push(tty->link); wake_up_interruptible(&tty->link->read_wait); wake_up_interruptible(&tty->link->write_wait); if (tty->driver->subtype == PTY_TYPE_MASTER) { @@ -208,7 +207,6 @@ static int pty_open(struct tty_struct *tty, struct file *filp) clear_bit(TTY_OTHER_CLOSED, &tty->link->flags); set_bit(TTY_THROTTLED, &tty->flags); retval = 0; - tty->low_latency = 1; out: return retval; } diff --git a/drivers/char/tty_buffer.c b/drivers/char/tty_buffer.c index 810ee25d66a4..3108991c5c8b 100644 --- a/drivers/char/tty_buffer.c +++ b/drivers/char/tty_buffer.c @@ -461,6 +461,19 @@ static void flush_to_ldisc(struct work_struct *work) tty_ldisc_deref(disc); } +/** + * tty_flush_to_ldisc + * @tty: tty to push + * + * Push the terminal flip buffers to the line discipline. + * + * Must not be called from IRQ context. + */ +void tty_flush_to_ldisc(struct tty_struct *tty) +{ + flush_to_ldisc(&tty->buf.work.work); +} + /** * tty_flip_buffer_push - terminal * @tty: tty to push -- cgit v1.2.3 From cab8bd3410d448279e3bd0fbf96d31db0bf770fa Mon Sep 17 00:00:00 2001 From: Hidetoshi Seto Date: Wed, 29 Jul 2009 15:04:14 -0700 Subject: sysrq, kdump: make sysrq-c consistent commit d6580a9f15238b87e618310c862231ae3f352d2d ("kexec: sysrq: simplify sysrq-c handler") changed the behavior of sysrq-c to unconditional dereference of NULL pointer. So in cases with CONFIG_KEXEC, where crash_kexec() was directly called from sysrq-c before, now it can be said that a step of "real oops" was inserted before starting kdump. However, in contrast to oops via SysRq-c from keyboard which results in panic due to in_interrupt(), oops via "echo c > /proc/sysrq-trigger" will not become panic unless panic_on_oops=1. It means that even if dump is properly configured to be taken on panic, the sysrq-c from proc interface might not start crashdump while the sysrq-c from keyboard can start crashdump. This confuses traditional users of kdump, i.e. people who expect sysrq-c to do common behavior in both of the keyboard and proc interface. This patch brings the keyboard and proc interface behavior of sysrq-c in line, by forcing panic_on_oops=1 before oops in sysrq-c handler. And some updates in documentation are included, to clarify that there is no longer dependency with CONFIG_KEXEC, and that now the system can just crash by sysrq-c if no dump mechanism is configured. Signed-off-by: Hidetoshi Seto Cc: Lai Jiangshan Cc: Ken'ichi Ohmichi Acked-by: Neil Horman Acked-by: Vivek Goyal Cc: Brayan Arraes Cc: Eric W. Biederman Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/char/sysrq.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c index 0db35857e4d8..5d7a02f63e1c 100644 --- a/drivers/char/sysrq.c +++ b/drivers/char/sysrq.c @@ -35,7 +35,6 @@ #include #include #include -#include #include #include @@ -124,9 +123,12 @@ static struct sysrq_key_op sysrq_unraw_op = { static void sysrq_handle_crash(int key, struct tty_struct *tty) { char *killer = NULL; + + panic_on_oops = 1; /* force panic */ + wmb(); *killer = 1; } -static struct sysrq_key_op sysrq_crashdump_op = { +static struct sysrq_key_op sysrq_crash_op = { .handler = sysrq_handle_crash, .help_msg = "Crash", .action_msg = "Trigger a crash", @@ -401,7 +403,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = { */ NULL, /* a */ &sysrq_reboot_op, /* b */ - &sysrq_crashdump_op, /* c & ibm_emac driver debug */ + &sysrq_crash_op, /* c & ibm_emac driver debug */ &sysrq_showlocks_op, /* d */ &sysrq_term_op, /* e */ &sysrq_moom_op, /* f */ -- cgit v1.2.3 From c43962321e8af5309dd3ffcd78743c89581265e5 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Sun, 2 Aug 2009 15:35:43 +0200 Subject: parisc: parisc-agp.c - use correct page_mask function Fix those compiler warnings, which indeed point to a bug: drivers/char/agp/parisc-agp.c:228: warning: initialization from incompatible pointer type drivers/char/agp/parisc-agp.c:201: warning: 'parisc_agp_page_mask_memory' defined but not used Signed-off-by: Helge Deller --- drivers/char/agp/parisc-agp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c index f4bb43fb8016..e077701ae3d9 100644 --- a/drivers/char/agp/parisc-agp.c +++ b/drivers/char/agp/parisc-agp.c @@ -225,7 +225,7 @@ static const struct agp_bridge_driver parisc_agp_driver = { .configure = parisc_agp_configure, .fetch_size = parisc_agp_fetch_size, .tlb_flush = parisc_agp_tlbflush, - .mask_memory = parisc_agp_mask_memory, + .mask_memory = parisc_agp_page_mask_memory, .masks = parisc_agp_masks, .agp_enable = parisc_agp_enable, .cache_flush = global_cache_flush, -- cgit v1.2.3 From 18eac1cc100fa2afd5f39085aae6b694e417734b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 3 Aug 2009 10:58:29 -0700 Subject: tty-ldisc: make refcount be atomic_t 'users' count This is pure preparation of changing the ldisc reference counting to be a true refcount that defines the lifetime of the ldisc. But this is a purely syntactic change for now to make the next steps easier. This patch should make no semantic changes at all. But I wanted to make the ldisc refcount be an atomic (I will be touching it without locks soon enough), and I wanted to rename it so that there isn't quite as much confusion between 'ldo->refcount' (ldisk operations refcount) and 'ld->refcount' (ldisc refcount itself) in the same file. So it's now an atomic 'ld->users' count. It still starts at zero, despite having a reference from 'tty->ldisc', but that will change once we turn it into a _real_ refcount. Signed-off-by: Linus Torvalds Tested-by: OGAWA Hirofumi Tested-by: Sergey Senozhatsky Acked-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/char/tty_ldisc.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index acd76b767d4c..fd175e60aad5 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -142,7 +142,7 @@ static struct tty_ldisc *tty_ldisc_try_get(int disc) /* lock it */ ldops->refcount++; ld->ops = ldops; - ld->refcount = 0; + atomic_set(&ld->users, 0); err = 0; } } @@ -206,7 +206,7 @@ static void tty_ldisc_put(struct tty_ldisc *ld) ldo->refcount--; module_put(ldo->owner); spin_unlock_irqrestore(&tty_ldisc_lock, flags); - WARN_ON(ld->refcount); + WARN_ON(atomic_read(&ld->users)); kfree(ld); } @@ -297,7 +297,7 @@ static int tty_ldisc_try(struct tty_struct *tty) spin_lock_irqsave(&tty_ldisc_lock, flags); ld = tty->ldisc; if (test_bit(TTY_LDISC, &tty->flags)) { - ld->refcount++; + atomic_inc(&ld->users); ret = 1; } spin_unlock_irqrestore(&tty_ldisc_lock, flags); @@ -324,7 +324,7 @@ struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty) { /* wait_event is a macro */ wait_event(tty_ldisc_wait, tty_ldisc_try(tty)); - WARN_ON(tty->ldisc->refcount == 0); + WARN_ON(atomic_read(&tty->ldisc->users) == 0); return tty->ldisc; } EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait); @@ -365,11 +365,9 @@ void tty_ldisc_deref(struct tty_ldisc *ld) BUG_ON(ld == NULL); spin_lock_irqsave(&tty_ldisc_lock, flags); - if (ld->refcount == 0) + if (atomic_read(&ld->users) == 0) printk(KERN_ERR "tty_ldisc_deref: no references.\n"); - else - ld->refcount--; - if (ld->refcount == 0) + else if (atomic_dec_and_test(&ld->users)) wake_up(&tty_ldisc_wait); spin_unlock_irqrestore(&tty_ldisc_lock, flags); } @@ -536,10 +534,10 @@ static int tty_ldisc_wait_idle(struct tty_struct *tty) { unsigned long flags; spin_lock_irqsave(&tty_ldisc_lock, flags); - while (tty->ldisc->refcount) { + while (atomic_read(&tty->ldisc->users)) { spin_unlock_irqrestore(&tty_ldisc_lock, flags); if (wait_event_timeout(tty_ldisc_wait, - tty->ldisc->refcount == 0, 5 * HZ) == 0) + atomic_read(&tty->ldisc->users) == 0, 5 * HZ) == 0) return -EBUSY; spin_lock_irqsave(&tty_ldisc_lock, flags); } -- cgit v1.2.3 From 65b770468e98941e45e19780dff9283e663e6b8b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 3 Aug 2009 11:11:19 -0700 Subject: tty-ldisc: turn ldisc user count into a proper refcount By using the user count for the actual lifetime rules, we can get rid of the silly "wait_for_idle" logic, because any busy ldisc will automatically stay around until the last user releases it. This avoids a host of odd issues, and simplifies the code. So now, when the last ldisc reference is dropped, we just release the ldisc operations struct reference, and free the ldisc. It looks obvious enough, and it does work for me, but the counting _could_ be off. It probably isn't (bad counting in the new version would generally imply that the old code did something really bad, like free an ldisc with a non-zero count), but it does need some testing, and preferably somebody looking at it. With this change, both 'tty_ldisc_put()' and 'tty_ldisc_deref()' are just aliases for the new ref-counting 'put_ldisc()'. Both of them decrement the ldisc user count and free it if it goes down to zero. They're identical functions, in other words. But the reason they still exist as sepate functions is that one of them was exported (tty_ldisc_deref) and had a stupid name (so I don't want to use it as the main name), and the other one was used in multiple places (and I didn't want to make the patch larger just to rename the users). In addition to the refcounting, I did do some minimal cleanup. For example, now "tty_ldisc_try()" actually returns the ldisc it got under the lock, rather than returning true/false and then the caller would look up the ldisc again (now without the protection of the lock). That said, there's tons of dubious use of 'tty->ldisc' without obviously proper locking or refcounting left. I expressly did _not_ want to try to fix it all, keeping the patch minimal. There may or may not be bugs in that kind of code, but they wouldn't be _new_ bugs. That said, even if the bugs aren't new, the timing and lifetime will change. For example, some silly code may depend on the 'tty->ldisc' pointer not changing because they hold a refcount on the 'ldisc'. And that's no longer true - if you hold a ref on the ldisc, the 'ldisc' itself is safe, but tty->ldisc may change. So the proper locking (remains) to hold tty->ldisc_mutex if you expect tty->ldisc to be stable. That's not really a _new_ rule, but it's an example of something that the old code might have unintentionally depended on and hidden bugs. Whatever. The patch _looks_ sensible to me. The only users of ldisc->users are: - get_ldisc() - atomically increment the count - put_ldisc() - atomically decrements the count and releases if zero - tty_ldisc_try_get() - creates the ldisc, and sets the count to 1. The ldisc should then either be released, or be attached to a tty. Signed-off-by: Linus Torvalds Tested-by: OGAWA Hirofumi Tested-by: Sergey Senozhatsky Acked-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/char/tty_ldisc.c | 143 +++++++++++++++-------------------------------- 1 file changed, 46 insertions(+), 97 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index fd175e60aad5..be55dfcf59ac 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -48,6 +48,34 @@ static DECLARE_WAIT_QUEUE_HEAD(tty_ldisc_wait); /* Line disc dispatch table */ static struct tty_ldisc_ops *tty_ldiscs[NR_LDISCS]; +static inline struct tty_ldisc *get_ldisc(struct tty_ldisc *ld) +{ + if (ld) + atomic_inc(&ld->users); + return ld; +} + +static inline void put_ldisc(struct tty_ldisc *ld) +{ + if (WARN_ON_ONCE(!ld)) + return; + + /* + * If this is the last user, free the ldisc, and + * release the ldisc ops. + */ + if (atomic_dec_and_test(&ld->users)) { + unsigned long flags; + struct tty_ldisc_ops *ldo = ld->ops; + + kfree(ld); + spin_lock_irqsave(&tty_ldisc_lock, flags); + ldo->refcount--; + module_put(ldo->owner); + spin_unlock_irqrestore(&tty_ldisc_lock, flags); + } +} + /** * tty_register_ldisc - install a line discipline * @disc: ldisc number @@ -142,7 +170,7 @@ static struct tty_ldisc *tty_ldisc_try_get(int disc) /* lock it */ ldops->refcount++; ld->ops = ldops; - atomic_set(&ld->users, 0); + atomic_set(&ld->users, 1); err = 0; } } @@ -181,35 +209,6 @@ static struct tty_ldisc *tty_ldisc_get(int disc) return ld; } -/** - * tty_ldisc_put - drop ldisc reference - * @ld: ldisc - * - * Drop a reference to a line discipline. Manage refcounts and - * module usage counts. Free the ldisc once the recount hits zero. - * - * Locking: - * takes tty_ldisc_lock to guard against ldisc races - */ - -static void tty_ldisc_put(struct tty_ldisc *ld) -{ - unsigned long flags; - int disc = ld->ops->num; - struct tty_ldisc_ops *ldo; - - BUG_ON(disc < N_TTY || disc >= NR_LDISCS); - - spin_lock_irqsave(&tty_ldisc_lock, flags); - ldo = tty_ldiscs[disc]; - BUG_ON(ldo->refcount == 0); - ldo->refcount--; - module_put(ldo->owner); - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - WARN_ON(atomic_read(&ld->users)); - kfree(ld); -} - static void *tty_ldiscs_seq_start(struct seq_file *m, loff_t *pos) { return (*pos < NR_LDISCS) ? pos : NULL; @@ -234,7 +233,7 @@ static int tty_ldiscs_seq_show(struct seq_file *m, void *v) if (IS_ERR(ld)) return 0; seq_printf(m, "%-10s %2d\n", ld->ops->name ? ld->ops->name : "???", i); - tty_ldisc_put(ld); + put_ldisc(ld); return 0; } @@ -288,20 +287,17 @@ static void tty_ldisc_assign(struct tty_struct *tty, struct tty_ldisc *ld) * Locking: takes tty_ldisc_lock */ -static int tty_ldisc_try(struct tty_struct *tty) +static struct tty_ldisc *tty_ldisc_try(struct tty_struct *tty) { unsigned long flags; struct tty_ldisc *ld; - int ret = 0; spin_lock_irqsave(&tty_ldisc_lock, flags); - ld = tty->ldisc; - if (test_bit(TTY_LDISC, &tty->flags)) { - atomic_inc(&ld->users); - ret = 1; - } + ld = NULL; + if (test_bit(TTY_LDISC, &tty->flags)) + ld = get_ldisc(tty->ldisc); spin_unlock_irqrestore(&tty_ldisc_lock, flags); - return ret; + return ld; } /** @@ -322,10 +318,11 @@ static int tty_ldisc_try(struct tty_struct *tty) struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty) { + struct tty_ldisc *ld; + /* wait_event is a macro */ - wait_event(tty_ldisc_wait, tty_ldisc_try(tty)); - WARN_ON(atomic_read(&tty->ldisc->users) == 0); - return tty->ldisc; + wait_event(tty_ldisc_wait, (ld = tty_ldisc_try(tty)) != NULL); + return ld; } EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait); @@ -342,9 +339,7 @@ EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait); struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty) { - if (tty_ldisc_try(tty)) - return tty->ldisc; - return NULL; + return tty_ldisc_try(tty); } EXPORT_SYMBOL_GPL(tty_ldisc_ref); @@ -360,19 +355,15 @@ EXPORT_SYMBOL_GPL(tty_ldisc_ref); void tty_ldisc_deref(struct tty_ldisc *ld) { - unsigned long flags; - - BUG_ON(ld == NULL); - - spin_lock_irqsave(&tty_ldisc_lock, flags); - if (atomic_read(&ld->users) == 0) - printk(KERN_ERR "tty_ldisc_deref: no references.\n"); - else if (atomic_dec_and_test(&ld->users)) - wake_up(&tty_ldisc_wait); - spin_unlock_irqrestore(&tty_ldisc_lock, flags); + put_ldisc(ld); } EXPORT_SYMBOL_GPL(tty_ldisc_deref); +static inline void tty_ldisc_put(struct tty_ldisc *ld) +{ + put_ldisc(ld); +} + /** * tty_ldisc_enable - allow ldisc use * @tty: terminal to activate ldisc on @@ -520,31 +511,6 @@ static int tty_ldisc_halt(struct tty_struct *tty) return cancel_delayed_work(&tty->buf.work); } -/** - * tty_ldisc_wait_idle - wait for the ldisc to become idle - * @tty: tty to wait for - * - * Wait for the line discipline to become idle. The discipline must - * have been halted for this to guarantee it remains idle. - * - * tty_ldisc_lock protects the ref counts currently. - */ - -static int tty_ldisc_wait_idle(struct tty_struct *tty) -{ - unsigned long flags; - spin_lock_irqsave(&tty_ldisc_lock, flags); - while (atomic_read(&tty->ldisc->users)) { - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - if (wait_event_timeout(tty_ldisc_wait, - atomic_read(&tty->ldisc->users) == 0, 5 * HZ) == 0) - return -EBUSY; - spin_lock_irqsave(&tty_ldisc_lock, flags); - } - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - return 0; -} - /** * tty_set_ldisc - set line discipline * @tty: the terminal to set @@ -640,14 +606,6 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) flush_scheduled_work(); - /* Let any existing reference holders finish */ - retval = tty_ldisc_wait_idle(tty); - if (retval < 0) { - clear_bit(TTY_LDISC_CHANGING, &tty->flags); - tty_ldisc_put(new_ldisc); - return retval; - } - mutex_lock(&tty->ldisc_mutex); if (test_bit(TTY_HUPPED, &tty->flags)) { /* We were raced by the hangup method. It will have stomped @@ -793,7 +751,6 @@ void tty_ldisc_hangup(struct tty_struct *tty) if (tty->ldisc) { /* Not yet closed */ /* Switch back to N_TTY */ tty_ldisc_halt(tty); - tty_ldisc_wait_idle(tty); tty_ldisc_reinit(tty); /* At this point we have a closed ldisc and we want to reopen it. We could defer this to the next open but @@ -858,14 +815,6 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) tty_ldisc_halt(tty); flush_scheduled_work(); - /* - * Wait for any short term users (we know they are just driver - * side waiters as the file is closing so user count on the file - * side is zero. - */ - - tty_ldisc_wait_idle(tty); - mutex_lock(&tty->ldisc_mutex); /* * Now kill off the ldisc -- cgit v1.2.3 From cbe9352fa08f90aa03b4dbf1bbabfc95d196e562 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 3 Aug 2009 14:54:56 -0700 Subject: tty-ldisc: be more careful in 'put_ldisc' locking Use 'atomic_dec_and_lock()' to make sure that we always hold the tty_ldisc_lock when the ldisc count goes to zero. That way we can never race against 'tty_ldisc_try()' increasing the count again. Reported-by: OGAWA Hirofumi Signed-off-by: Linus Torvalds Tested-by: Sergey Senozhatsky Signed-off-by: Greg Kroah-Hartman --- drivers/char/tty_ldisc.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index be55dfcf59ac..1733d3439ad2 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -55,25 +55,32 @@ static inline struct tty_ldisc *get_ldisc(struct tty_ldisc *ld) return ld; } -static inline void put_ldisc(struct tty_ldisc *ld) +static void put_ldisc(struct tty_ldisc *ld) { + unsigned long flags; + if (WARN_ON_ONCE(!ld)) return; /* * If this is the last user, free the ldisc, and * release the ldisc ops. + * + * We really want an "atomic_dec_and_lock_irqsave()", + * but we don't have it, so this does it by hand. */ - if (atomic_dec_and_test(&ld->users)) { - unsigned long flags; + local_irq_save(flags); + if (atomic_dec_and_lock(&ld->users, &tty_ldisc_lock)) { struct tty_ldisc_ops *ldo = ld->ops; - kfree(ld); - spin_lock_irqsave(&tty_ldisc_lock, flags); ldo->refcount--; module_put(ldo->owner); spin_unlock_irqrestore(&tty_ldisc_lock, flags); + + kfree(ld); + return; } + local_irq_restore(flags); } /** -- cgit v1.2.3