All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PATCH] TTY/serial fixes for .37-rc4
@ 2010-12-02 20:48 Greg KH
  2010-12-02 21:08 ` [PATCH 1/6] serial8250: Mark console as CON_ANYTIME Greg Kroah-Hartman
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Greg KH @ 2010-12-02 20:48 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, Alan Cox

Here are a number of tty and serial fixes for your .37-rc4 tree.

Jiri did a lot of work tracking down the annoying WARN_ON messages that
have been showing up since .36, and he's fixes some issues that are even
older than that here that have been reported by a lot of users.

Please pull from:
	master.kernel.org:/pub/scm/linux/kernel/git/gregkh/tty-2.6.git/ tty-linus

All of these have been in the -next and -mm releases for a while.

Patches will be sent to the linux-kernel mailing list, if anyone wants
to see them.

thanks,

greg k-h

------------

 drivers/net/wan/x25_asy.c |    2 +-
 drivers/serial/8250.c     |    2 +-
 drivers/serial/mfd.c      |   24 ++++++++----------------
 drivers/tty/tty_io.c      |   13 +++++++++++--
 drivers/tty/tty_ldisc.c   |    2 ++
 include/linux/tty.h       |    1 +
 6 files changed, 24 insertions(+), 20 deletions(-)

---------------

Feng Tang (1):
      serial: mfd: adjust the baud rate setting

Jiri Slaby (4):
      TTY: ldisc, fix open flag handling
      NET: wan/x25, fix ldisc->open retval
      TTY: don't allow reopen when ldisc is changing
      TTY: open/hangup race fixup

Peter Zijlstra (1):
      serial8250: Mark console as CON_ANYTIME


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] serial8250: Mark console as CON_ANYTIME
  2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
@ 2010-12-02 21:08 ` Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 2/6] TTY: ldisc, fix open flag handling Greg Kroah-Hartman
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2010-12-02 21:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Peter Zijlstra, Greg Kroah-Hartman

From: Peter Zijlstra <peterz@infradead.org>

While trying to debug a cpu-hotplug issue I noticed printk() stopped
working once the cpu got marked offline, since the 8250 serial console
doesn't have any per-cpu resources the CON_ANYTIME bit is the safe and
documented way to make it work again.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/serial/8250.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 4d8e14b..09a5508 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2872,7 +2872,7 @@ static struct console serial8250_console = {
 	.device		= uart_console_device,
 	.setup		= serial8250_console_setup,
 	.early_setup	= serial8250_console_early_setup,
-	.flags		= CON_PRINTBUFFER,
+	.flags		= CON_PRINTBUFFER | CON_ANYTIME,
 	.index		= -1,
 	.data		= &serial8250_reg,
 };
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] TTY: ldisc, fix open flag handling
  2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
  2010-12-02 21:08 ` [PATCH 1/6] serial8250: Mark console as CON_ANYTIME Greg Kroah-Hartman
@ 2010-12-02 21:08 ` Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 3/6] NET: wan/x25, fix ldisc->open retval Greg Kroah-Hartman
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2010-12-02 21:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Slaby, Alan Cox, stable, Greg Kroah-Hartman

From: Jiri Slaby <jslaby@suse.cz>

When a concrete ldisc open fails in tty_ldisc_open, we forget to clear
TTY_LDISC_OPEN. This causes a false warning on the next ldisc open:
WARNING: at drivers/char/tty_ldisc.c:445 tty_ldisc_open+0x26/0x38()
Hardware name: System Product Name
Modules linked in: ...
Pid: 5251, comm: a.out Tainted: G        W  2.6.32-5-686 #1
Call Trace:
 [<c1030321>] ? warn_slowpath_common+0x5e/0x8a
 [<c1030357>] ? warn_slowpath_null+0xa/0xc
 [<c119311c>] ? tty_ldisc_open+0x26/0x38
 [<c11936c5>] ? tty_set_ldisc+0x218/0x304
...

So clear the bit when failing...

Introduced in c65c9bc3efa (tty: rewrite the ldisc locking) back in
2.6.31-rc1.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@linux.intel.com>
Reported-by: Sergey Lapin <slapin@ossfans.org>
Tested-by: Sergey Lapin <slapin@ossfans.org>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/tty_ldisc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index d8e96b0..4214d58 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -454,6 +454,8 @@ static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
                 /* BTM here locks versus a hangup event */
 		WARN_ON(!tty_locked());
 		ret = ld->ops->open(tty);
+		if (ret)
+			clear_bit(TTY_LDISC_OPEN, &tty->flags);
 		return ret;
 	}
 	return 0;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] NET: wan/x25, fix ldisc->open retval
  2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
  2010-12-02 21:08 ` [PATCH 1/6] serial8250: Mark console as CON_ANYTIME Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 2/6] TTY: ldisc, fix open flag handling Greg Kroah-Hartman
@ 2010-12-02 21:08 ` Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 4/6] TTY: don't allow reopen when ldisc is changing Greg Kroah-Hartman
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2010-12-02 21:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Slaby, Alan Cox, Andrew Hendry, linux-x25, Greg Kroah-Hartman

From: Jiri Slaby <jslaby@suse.cz>

We should never return positive values from ldisc->open, tty layer
doesn't (and never did) expect that.

If we do that, weird things like warnings in tty_ldisc_close happen.

Not sure if dev->base_addr is used somehow now.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: linux-x25@vger.kernel.org
Tested-by: Sergey Lapin <slapin@ossfans.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/net/wan/x25_asy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
index d81ad83..66cda25 100644
--- a/drivers/net/wan/x25_asy.c
+++ b/drivers/net/wan/x25_asy.c
@@ -582,7 +582,7 @@ static int x25_asy_open_tty(struct tty_struct *tty)
 	if (err)
 		return err;
 	/* Done.  We have linked the TTY line to a channel. */
-	return sl->dev->base_addr;
+	return 0;
 }
 
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] TTY: don't allow reopen when ldisc is changing
  2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
                   ` (2 preceding siblings ...)
  2010-12-02 21:08 ` [PATCH 3/6] NET: wan/x25, fix ldisc->open retval Greg Kroah-Hartman
@ 2010-12-02 21:08 ` Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 5/6] TTY: open/hangup race fixup Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 6/6] serial: mfd: adjust the baud rate setting Greg Kroah-Hartman
  5 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2010-12-02 21:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Slaby, Kyle McMartin, Alan Cox, stable, Greg Kroah-Hartman

From: Jiri Slaby <jslaby@suse.cz>

There are many WARNINGs like the following reported nowadays:
WARNING: at drivers/tty/tty_io.c:1331 tty_open+0x2a2/0x49a()
Hardware name: Latitude E6500
Modules linked in:
Pid: 1207, comm: plymouthd Not tainted 2.6.37-rc3-mmotm1123 #3
Call Trace:
 [<ffffffff8103b189>] warn_slowpath_common+0x80/0x98
 [<ffffffff8103b1b6>] warn_slowpath_null+0x15/0x17
 [<ffffffff8128a3ab>] tty_open+0x2a2/0x49a
 [<ffffffff810fd53f>] chrdev_open+0x11d/0x146
...

This means tty_reopen is called without TTY_LDISC set. For further
considerations, note tty_lock is held in tty_open. TTY_LDISC is cleared in:
1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this
section tty_lock is held. However tty_lock is temporarily dropped in
the middle of the function by tty_ldisc_hangup.

2) tty_release via tty_ldisc_release till the end of tty existence. If
tty->count <= 1, tty_lock is taken, TTY_CLOSING bit set and then
tty_ldisc_release called. tty_reopen checks TTY_CLOSING before checking
TTY_LDISC.

3) tty_set_ldisc from tty_ldisc_halt to tty_ldisc_enable. We:
   * take tty_lock, set TTY_LDISC_CHANGING, put tty_lock
   * call tty_ldisc_halt (clear TTY_LDISC), tty_lock is _not_ held
   * do some other work
   * take tty_lock, call tty_ldisc_enable (set TTY_LDISC), put
     tty_lock

I cannot see how 2) can be a problem, as there I see no race. OTOH, 1)
and 3) can happen without problems. This patch the case 3) by checking
TTY_LDISC_CHANGING along with TTY_CLOSING in tty_reopen. 1) will be
fixed in the following patch.

Nicely reproducible with two processes:
while (1) {
	fd = open("/dev/ttyS1", O_RDWR);
	if (fd < 0) {
		warn("open");
		continue;
	}
	close(fd);
}
--------
while (1) {
        fd = open("/dev/ttyS1", O_RDWR);
        ld1 = 0; ld2 = 2;
        while (1) {
                ioctl(fd, TIOCSETD, &ld1);
                ioctl(fd, TIOCSETD, &ld2);
        }
        close(fd);
}

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: <Valdis.Kletnieks@vt.edu>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/tty_io.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c05c5af..878f6d6 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1310,7 +1310,8 @@ static int tty_reopen(struct tty_struct *tty)
 {
 	struct tty_driver *driver = tty->driver;
 
-	if (test_bit(TTY_CLOSING, &tty->flags))
+	if (test_bit(TTY_CLOSING, &tty->flags) ||
+			test_bit(TTY_LDISC_CHANGING, &tty->flags))
 		return -EIO;
 
 	if (driver->type == TTY_DRIVER_TYPE_PTY &&
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] TTY: open/hangup race fixup
  2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
                   ` (3 preceding siblings ...)
  2010-12-02 21:08 ` [PATCH 4/6] TTY: don't allow reopen when ldisc is changing Greg Kroah-Hartman
@ 2010-12-02 21:08 ` Greg Kroah-Hartman
  2010-12-02 21:08 ` [PATCH 6/6] serial: mfd: adjust the baud rate setting Greg Kroah-Hartman
  5 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2010-12-02 21:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Slaby, Alan Cox, stable, Greg Kroah-Hartman

From: Jiri Slaby <jslaby@suse.cz>

Like in the "TTY: don't allow reopen when ldisc is changing" patch,
this one fixes a TTY WARNING as described in the option 1) there:
1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this
section tty_lock is held. However tty_lock is temporarily dropped in
the middle of the function by tty_ldisc_hangup.

The fix is to introduce a new flag which we set during the unlocked
window and check it in tty_reopen too. The flag is TTY_HUPPING and is
cleared after TTY_HUPPED is set.

While at it, remove duplicate TTY_HUPPED set_bit. The one after
calling ops->hangup seems to be more correct. But anyway, we hold
tty_lock, so there should be no difference.

Also document the function it does that kind of crap.

Nicely reproducible with two forked children:
static void do_work(const char *tty)
{
	if (signal(SIGHUP, SIG_IGN) == SIG_ERR) exit(1);
	setsid();
	while (1) {
		int fd = open(tty, O_RDWR|O_NOCTTY);
		if (fd < 0) continue;
		if (ioctl(fd, TIOCSCTTY)) continue;
		if (vhangup()) continue;
		close(fd);
	}
	exit(0);
}

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: <Valdis.Kletnieks@vt.edu>
Reported-by: Kyle McMartin <kyle@mcmartin.ca>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/tty_io.c |   10 +++++++++-
 include/linux/tty.h  |    1 +
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 878f6d6..35480dd 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -559,6 +559,9 @@ void __tty_hangup(struct tty_struct *tty)
 
 	tty_lock();
 
+	/* some functions below drop BTM, so we need this bit */
+	set_bit(TTY_HUPPING, &tty->flags);
+
 	/* inuse_filps is protected by the single tty lock,
 	   this really needs to change if we want to flush the
 	   workqueue with the lock held */
@@ -578,6 +581,10 @@ void __tty_hangup(struct tty_struct *tty)
 	}
 	spin_unlock(&tty_files_lock);
 
+	/*
+	 * it drops BTM and thus races with reopen
+	 * we protect the race by TTY_HUPPING
+	 */
 	tty_ldisc_hangup(tty);
 
 	read_lock(&tasklist_lock);
@@ -615,7 +622,6 @@ void __tty_hangup(struct tty_struct *tty)
 	tty->session = NULL;
 	tty->pgrp = NULL;
 	tty->ctrl_status = 0;
-	set_bit(TTY_HUPPED, &tty->flags);
 	spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 
 	/* Account for the p->signal references we killed */
@@ -641,6 +647,7 @@ void __tty_hangup(struct tty_struct *tty)
 	 * can't yet guarantee all that.
 	 */
 	set_bit(TTY_HUPPED, &tty->flags);
+	clear_bit(TTY_HUPPING, &tty->flags);
 	tty_ldisc_enable(tty);
 
 	tty_unlock();
@@ -1311,6 +1318,7 @@ static int tty_reopen(struct tty_struct *tty)
 	struct tty_driver *driver = tty->driver;
 
 	if (test_bit(TTY_CLOSING, &tty->flags) ||
+			test_bit(TTY_HUPPING, &tty->flags) ||
 			test_bit(TTY_LDISC_CHANGING, &tty->flags))
 		return -EIO;
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index c7ea9bc..021bfd2 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -367,6 +367,7 @@ struct tty_file_private {
 #define TTY_HUPPED 		18	/* Post driver->hangup() */
 #define TTY_FLUSHING		19	/* Flushing to ldisc in progress */
 #define TTY_FLUSHPENDING	20	/* Queued buffer flush pending */
+#define TTY_HUPPING 		21	/* ->hangup() in progress */
 
 #define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] serial: mfd: adjust the baud rate setting
  2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
                   ` (4 preceding siblings ...)
  2010-12-02 21:08 ` [PATCH 5/6] TTY: open/hangup race fixup Greg Kroah-Hartman
@ 2010-12-02 21:08 ` Greg Kroah-Hartman
  5 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2010-12-02 21:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Feng Tang, stable, Greg Kroah-Hartman

From: Feng Tang <feng.tang@intel.com>

Previous baud rate setting code only has been tested with 3.5M/9600/
115200/230400/460800 bps, and recently we got a 3M bps device to test,
which needs to modify current MUL register setting, and with this
patch 2.5M/2M/1.5M/1M/0.5M should also work as they just use a MUL
value scale down from 3M's.

Also got some reference register setting from silicon guys for
different baud rates, which tries to keep the pre-scalar register value
to 16.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/serial/mfd.c |   24 ++++++++----------------
 1 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/serial/mfd.c b/drivers/serial/mfd.c
index 5fc699e..d40010a 100644
--- a/drivers/serial/mfd.c
+++ b/drivers/serial/mfd.c
@@ -900,8 +900,7 @@ serial_hsu_set_termios(struct uart_port *port, struct ktermios *termios,
 	unsigned char cval, fcr = 0;
 	unsigned long flags;
 	unsigned int baud, quot;
-	u32 mul = 0x3600;
-	u32 ps = 0x10;
+	u32 ps, mul;
 
 	switch (termios->c_cflag & CSIZE) {
 	case CS5:
@@ -943,31 +942,24 @@ serial_hsu_set_termios(struct uart_port *port, struct ktermios *termios,
 	baud = uart_get_baud_rate(port, termios, old, 0, 4000000);
 
 	quot = 1;
+	ps = 0x10;
+	mul = 0x3600;
 	switch (baud) {
 	case 3500000:
 		mul = 0x3345;
 		ps = 0xC;
 		break;
-	case 3000000:
-		mul = 0x2EE0;
-		break;
-	case 2500000:
-		mul = 0x2710;
-		break;
-	case 2000000:
-		mul = 0x1F40;
-		break;
 	case 1843200:
 		mul = 0x2400;
 		break;
+	case 3000000:
+	case 2500000:
+	case 2000000:
 	case 1500000:
-		mul = 0x1770;
-		break;
 	case 1000000:
-		mul = 0xFA0;
-		break;
 	case 500000:
-		mul = 0x7D0;
+		/* mul/ps/quot = 0x9C4/0x10/0x1 will make a 500000 bps */
+		mul = baud / 500000 * 0x9C4;
 		break;
 	default:
 		/* Use uart_get_divisor to get quot for other baud rates */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-12-02 21:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-02 20:48 [GIT PATCH] TTY/serial fixes for .37-rc4 Greg KH
2010-12-02 21:08 ` [PATCH 1/6] serial8250: Mark console as CON_ANYTIME Greg Kroah-Hartman
2010-12-02 21:08 ` [PATCH 2/6] TTY: ldisc, fix open flag handling Greg Kroah-Hartman
2010-12-02 21:08 ` [PATCH 3/6] NET: wan/x25, fix ldisc->open retval Greg Kroah-Hartman
2010-12-02 21:08 ` [PATCH 4/6] TTY: don't allow reopen when ldisc is changing Greg Kroah-Hartman
2010-12-02 21:08 ` [PATCH 5/6] TTY: open/hangup race fixup Greg Kroah-Hartman
2010-12-02 21:08 ` [PATCH 6/6] serial: mfd: adjust the baud rate setting Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.