Message-ID: <20080813110316.1d9af655@lxorguk.ukuu.org.uk>
Date:	Wed, 13 Aug 2008 11:03:16 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux 2.6.27-rc3

On Tue, 12 Aug 2008 19:33:16 -0700 (PDT)
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> 
> Things really _have_ calmed down, and hopefully we've also resolved a lot 
> of the regressions in -rc3.

Must admit it still seemed pretty broken to me. The firmware changes
still seem to break if you cross build and then nfs make modules_install
and a readonly mount "make modules_install" now fails. UDF mount appears
to blow up on the spot.

The tty resize race turns out to be fun. The reason people are now seeing
it bisects down to a scheduler change showing up the lack of locking
going back forever.

This is a bit of a big patch for -rc3 time so I don't know what you feel
about it. Basically we turn it inside out - we make resize a tty method
so that the driver takes its own locks first as you need to in this
situation then can call back into the default helper.

(If it looks ok let me know .. it'll also apply for testing versus -rc but
 wants a final polish in that form)

--

tty: remove resize window special case

From: Alan Cox <alan@...hat.com>

This moves it to being a tty operation. That removes special cases and now
also means that resize can be picked up by um and other non vt consoles
which may have a resize operation.

Signed-off-by: Alan Cox <alan@...hat.com>
---

 drivers/char/tty_io.c      |   76 +++++++++++++++++++++--------------------
 drivers/char/vt.c          |   82 +++++++++++++++++++++++++++++++++-----------
 drivers/char/vt_ioctl.c    |    7 +++-
 include/linux/tty.h        |    2 +
 include/linux/tty_driver.h |   14 ++++++++
 include/linux/vt_kern.h    |    1 -
 6 files changed, 124 insertions(+), 58 deletions(-)


diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 0e6866f..ddf5562 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2496,45 +2496,25 @@ static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg)
 }
 
 /**
- *	tiocswinsz		-	implement window size set ioctl
- *	@tty; tty
- *	@arg: user buffer for result
+ *	tty_do_resize		-	resize event
+ *	@tty: tty being resized
+ *	@real_tty: real tty (if using a pty/tty pair)
+ *	@rows: rows (character)
+ *	@cols: cols (character)
  *
- *	Copies the user idea of the window size to the kernel. Traditionally
- *	this is just advisory information but for the Linux console it
- *	actually has driver level meaning and triggers a VC resize.
- *
- *	Locking:
- *		Called function use the console_sem is used to ensure we do
- *	not try and resize the console twice at once.
- *		The tty->termios_mutex is used to ensure we don't double
- *	resize and get confused. Lock order - tty->termios_mutex before
- *	console sem
+ *	Update the termios variables and send the neccessary signals to
+ *	peform a terminal resize correctly
  */
 
-static int tiocswinsz(struct tty_struct *tty, struct tty_struct *real_tty,
-	struct winsize __user *arg)
+int tty_do_resize(struct tty_struct *tty, struct tty_struct *real_tty,
+					struct winsize *ws)
 {
-	struct winsize tmp_ws;
 	struct pid *pgrp, *rpgrp;
 	unsigned long flags;
 
-	if (copy_from_user(&tmp_ws, arg, sizeof(*arg)))
-		return -EFAULT;
-
 	mutex_lock(&tty->termios_mutex);
-	if (!memcmp(&tmp_ws, &tty->winsize, sizeof(*arg)))
+	if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
 		goto done;
-
-#ifdef CONFIG_VT
-	if (tty->driver->type == TTY_DRIVER_TYPE_CONSOLE) {
-		if (vc_lock_resize(tty->driver_data, tmp_ws.ws_col,
-					tmp_ws.ws_row)) {
-			mutex_unlock(&tty->termios_mutex);
-			return -ENXIO;
-		}
-	}
-#endif
 	/* Get the PID values and reference them so we can
 	   avoid holding the tty ctrl lock while sending signals */
 	spin_lock_irqsave(&tty->ctrl_lock, flags);
@@ -2550,14 +2530,42 @@ static int tiocswinsz(struct tty_struct *tty, struct tty_struct *real_tty,
 	put_pid(pgrp);
 	put_pid(rpgrp);
 
-	tty->winsize = tmp_ws;
-	real_tty->winsize = tmp_ws;
+	tty->winsize = *ws;
+	real_tty->winsize = *ws;
 done:
 	mutex_unlock(&tty->termios_mutex);
 	return 0;
 }
 
 /**
+ *	tiocswinsz		-	implement window size set ioctl
+ *	@tty; tty
+ *	@arg: user buffer for result
+ *
+ *	Copies the user idea of the window size to the kernel. Traditionally
+ *	this is just advisory information but for the Linux console it
+ *	actually has driver level meaning and triggers a VC resize.
+ *
+ *	Locking:
+ *		Driver dependant. The default do_resize method takes the
+ *	tty termios mutex and ctrl_lock. The console takes its own lock
+ *	then calls into the default method.
+ */
+
+static int tiocswinsz(struct tty_struct *tty, struct tty_struct *real_tty,
+	struct winsize __user *arg)
+{
+	struct winsize tmp_ws;
+	if (copy_from_user(&tmp_ws, arg, sizeof(*arg)))
+		return -EFAULT;
+
+	if (tty->ops->resize)
+		return tty->ops->resize(tty, real_tty, &tmp_ws);
+	else
+		return tty_do_resize(tty, real_tty, &tmp_ws);
+}
+
+/**
  *	tioccons	-	allow admin to move logical console
  *	@file: the file to become console
  *
diff --git a/drivers/char/vt.c b/drivers/char/vt.c
index c09170c..05ca1c5 100644
--- a/drivers/char/vt.c
+++ b/drivers/char/vt.c
@@ -803,7 +803,25 @@ static inline int resize_screen(struct vc_data *vc, int width, int height,
  */
 #define VC_RESIZE_MAXCOL (32767)
 #define VC_RESIZE_MAXROW (32767)
-int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int lines)
+
+/**
+ *	vc_do_resize	-	resizing method for the tty
+ *	@tty: tty being resized
+ *	@real_tty: real tty (different to tty if a pty/tty pair)
+ *	@vc: virtual console private data
+ *	@cols: columns
+ *	@lines: lines
+ *
+ *	Resize a virtual console, clipping according to the actual constraints.
+ *	If the caller passes a tty structure then update the termios winsize
+ *	information and perform any neccessary signal handling.
+ *
+ *	Caller must hold the console semaphore. Takes the termios mutex and
+ *	ctrl_lock of the tty IFF a tty is passed.
+ */
+
+static int vc_do_resize(struct tty_struct *tty, struct tty_struct *real_tty,
+		struct vc_data *vc, unsigned int cols, unsigned int lines)
 {
 	unsigned long old_origin, new_origin, new_scr_end, rlth, rrem, err = 0;
 	unsigned int old_cols, old_rows, old_row_size, old_screen_size;
@@ -907,24 +925,15 @@ int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int lines)
 	gotoxy(vc, vc->vc_x, vc->vc_y);
 	save_cur(vc);
 
-	if (vc->vc_tty) {
-		struct winsize ws, *cws = &vc->vc_tty->winsize;
-		struct pid *pgrp = NULL;
-
+	if (tty) {
+		/* Rewrite the requested winsize data with the actual
+		   resulting sizes */
+		struct winsize ws;
 		memset(&ws, 0, sizeof(ws));
 		ws.ws_row = vc->vc_rows;
 		ws.ws_col = vc->vc_cols;
 		ws.ws_ypixel = vc->vc_scan_lines;
-
-		spin_lock_irq(&vc->vc_tty->ctrl_lock);
-		if ((ws.ws_row != cws->ws_row || ws.ws_col != cws->ws_col))
-			pgrp = get_pid(vc->vc_tty->pgrp);
-		spin_unlock_irq(&vc->vc_tty->ctrl_lock);
-		if (pgrp) {
-			kill_pgrp(vc->vc_tty->pgrp, SIGWINCH, 1);
-			put_pid(pgrp);
-		}
-		*cws = ws;
+		tty_do_resize(tty, real_tty, &ws);
 	}
 
 	if (CON_IS_VISIBLE(vc))
@@ -932,14 +941,47 @@ int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int lines)
 	return err;
 }
 
-int vc_lock_resize(struct vc_data *vc, unsigned int cols, unsigned int lines)
+/**
+ *	vc_resize		-	resize a VT
+ *	@vc: virtual console
+ *	@cols: columns
+ *	@rows: rows
+ *
+ *	Resize a virtual console as seen from the console end of things. We
+ *	use the common vc_do_resize methods to update the structures. The
+ *	caller must hold the console sem to protect console internals and
+ *	vc->vc_tty
+ */
+
+int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int rows)
+{
+	return vc_do_resize(vc->vc_tty, vc->vc_tty, vc, cols, rows);
+}
+
+/**
+ *	vt_resize		-	resize a VT
+ *	@tty: tty to resize
+ *	@real_tty: tty if a pty/tty pair
+ *	@ws: winsize attributes
+ *
+ *	Resize a virtual terminal. This is called by the tty layer as we
+ *	register our own handler for resizing. The mutual helper does all
+ *	the actual work.
+ *
+ *	Takes the console sem and the called methods then take the tty
+ *	termios_mutex and the tty ctrl_lock in that order.
+ */
+
+int vt_resize(struct tty_struct *tty, struct tty_struct *real_tty,
+	struct winsize *ws)
 {
-	int rc;
+	struct vc_data *vc = tty->driver_data;
+	int ret;
 
 	acquire_console_sem();
-	rc = vc_resize(vc, cols, lines);
+	ret = vc_do_resize(tty, real_tty, vc, ws->ws_col, ws->ws_row);
 	release_console_sem();
-	return rc;
+	return ret;
 }
 
 void vc_deallocate(unsigned int currcons)
@@ -2905,6 +2947,7 @@ static const struct tty_operations con_ops = {
 	.start = con_start,
 	.throttle = con_throttle,
 	.unthrottle = con_unthrottle,
+	.resize = vt_resize,
 };
 
 int __init vty_init(void)
@@ -4059,7 +4102,6 @@ EXPORT_SYMBOL(default_blu);
 EXPORT_SYMBOL(update_region);
 EXPORT_SYMBOL(redraw_screen);
 EXPORT_SYMBOL(vc_resize);
-EXPORT_SYMBOL(vc_lock_resize);
 EXPORT_SYMBOL(fg_console);
 EXPORT_SYMBOL(console_blank_hook);
 EXPORT_SYMBOL(console_blanked);
diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
index 86f8421..6fd078b 100644
--- a/drivers/char/vt_ioctl.c
+++ b/drivers/char/vt_ioctl.c
@@ -374,6 +374,7 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
 	void __user *up = (void __user *)arg;
 	int i, perm;
 	int ret = 0;
+	struct winsize ws;
 
 	console = vc->vc_num;
 
@@ -949,14 +950,18 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
 		    get_user(cc, &vtsizes->v_cols))
 			ret = -EFAULT;
 		else {
+			acquire_console_sem();
 			for (i = 0; i < MAX_NR_CONSOLES; i++) {
 				vc = vc_cons[i].d;
 
 				if (vc) {
 					vc->vc_resize_user = 1;
-					vc_lock_resize(vc_cons[i].d, cc, ll);
+					ws.ws_col = cc;
+					ws.ws_row = ll;
+					vc_resize(vc_cons[i].d, cc, ll);
 				}
 			}
+			release_console_sem();
 		}
 		break;
 	}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index e3579cb..0cbec74 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -331,6 +331,8 @@ extern int tty_write_room(struct tty_struct *tty);
 extern void tty_driver_flush_buffer(struct tty_struct *tty);
 extern void tty_throttle(struct tty_struct *tty);
 extern void tty_unthrottle(struct tty_struct *tty);
+extern int tty_do_resize(struct tty_struct *tty, struct tty_struct *real_tty,
+						struct winsize *ws);
 
 extern int is_current_pgrp_orphaned(void);
 extern struct pid *tty_get_pgrp(struct tty_struct *tty);
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index e1065ac..16d2794 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -168,6 +168,18 @@
  *
  *	Optional: If not provided then the write method is called under
  *	the atomic write lock to keep it serialized with the ldisc.
+ *
+ * int (*resize)(struct tty_struct *tty, struct tty_struct *real_tty,
+ *				unsigned int rows, unsigned int cols);
+ *
+ *	Called when a termios request is issued which changes the
+ *	requested terminal geometry.
+ *
+ *	Optional: the default action is to update the termios structure
+ *	without error. This is usually the correct behaviour. Drivers should
+ *	not force errors here if they are not resizable objects (eg a serial
+ *	line). See tty_do_resize() if you need to wrap the standard method
+ *	in your own logic - the usual case.
  */
 
 #include <linux/fs.h>
@@ -206,6 +218,8 @@ struct tty_operations {
 	int (*tiocmget)(struct tty_struct *tty, struct file *file);
 	int (*tiocmset)(struct tty_struct *tty, struct file *file,
 			unsigned int set, unsigned int clear);
+	int (*resize)(struct tty_struct *tty, struct tty_struct *real_tty,
+				struct winsize *ws);
 #ifdef CONFIG_CONSOLE_POLL
 	int (*poll_init)(struct tty_driver *driver, int line, char *options);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index 1c78d56..1cbd0a7 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -35,7 +35,6 @@ extern int fg_console, last_console, want_console;
 int vc_allocate(unsigned int console);
 int vc_cons_allocated(unsigned int console);
 int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int lines);
-int vc_lock_resize(struct vc_data *vc, unsigned int cols, unsigned int lines);
 void vc_deallocate(unsigned int console);
 void reset_palette(struct vc_data *vc);
 void do_blank_screen(int entering_gfx);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists