[RFC PATCH 0/5] patches for a network-based hvc backend (s390)

Hello,

I work on a network-based hvc console backend for s390 that allows
to get "full-screen" terminal access to z/VM guest machines.
The solution consists of a HVC backend that provides the terminal interface;
and a tool to connect to the terminal via a z/VM specific communication
protocol.

The network-based backend differs in a few aspects from the notifier-based
model of the HVC console; because it has to deal with (dis)connects and must
take care of virtual tty hangups. Therefore, I would like to introduce a third
notifier for hangups (see patch 1).

Further I found out that if put_char() returns 0 in particular cases, the hvc
console starts to loop. I tried to address this problem with patch 3 and I hope
that it will work for all backends.

Finally, I would like to add a function that allows to resize the terminal
window of a HVC terminal (patch 4).


Here is an overview about the complete patch series:
 Patch 1 adds a hangup notifier
 Patch 2 adds tty driver flag TTY_DRIVER_RESET_TERMIOS
 Patch 3 fixes a loop if put_char() returns 0
 Patch 4 adds a function to resize the tty window
 Patch 5 removes __devexit of hvc_remove() for use in __(dev)init sections


Any review feedback would be greatly appreciated.
Thank you in advance.

Regards,
Hendrik
-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Hendrik Brueckner <redacted>

Removed __devexit annotation of hvc_remove() to avoid a section mismatch
if the backend initialization fails and hvc_remove() must be used to
clean up allocated hvc structs (called in section __init or __devinit).

Acked-by: Christian Borntraeger <redacted>
Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/hvc_console.c |    2 +-
 drivers/char/hvc_console.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c

@@ -806,7 +806,7 @@ struct hvc_struct __devinit *hvc_alloc(u
 }
 EXPORT_SYMBOL_GPL(hvc_alloc);
 
-int __devexit hvc_remove(struct hvc_struct *hp)
+int hvc_remove(struct hvc_struct *hp)
 {
 	unsigned long flags;
 	struct tty_struct *tty;
--- a/drivers/char/hvc_console.h
+++ b/drivers/char/hvc_console.h

@@ -81,7 +81,7 @@ extern int hvc_instantiate(uint32_t vter
 extern struct hvc_struct * __devinit hvc_alloc(uint32_t vtermno, int data,
 				struct hv_ops *ops, int outbuf_size);
 /* remove a vterm from hvc tty operation (module_exit or hotplug remove) */
-extern int __devexit hvc_remove(struct hvc_struct *hp);
+extern int hvc_remove(struct hvc_struct *hp);
 
 /* data available */
 int hvc_poll(struct hvc_struct *hp);

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Hendrik Brueckner <redacted>

If put_char() routine of a hvc console backend returns 0, then the
hvc console starts looping in the following scenarios:

1. hvc_console_print()
	If put_char() returns 0 then the while loop may loop forever.
	I have added the missing check for 0 to throw away console messages.

2. khvcd may loop:
	The thread calls hvc_poll() --> hvc_push()... if there are still
	buffered data then the HVC_POLL_WRITE bit is set and causes the
	khvcd thread to loop (if yield() returns immediately).

	However, instead of looping, the khvcd thread could sleep for
	MIN_TIMEOUT (doing the same as for get_chars()).
	The MIN_TIMEOUT is set if hvc_push() was not able to write
	data to the backend. If data has been written, the timeout is
	set to 0 to immediately re-schedule hvc_poll().

Reviewed-by: Christian Borntraeger <redacted>
Tested-by: Christian Borntraeger <redacted> (virtio_console)
Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/hvc_console.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c

@@ -161,7 +161,7 @@ static void hvc_console_print(struct con
 			}
 		} else {
 			r = cons_ops[index]->put_chars(vtermnos[index], c, i);
-			if (r < 0) {
+			if (r <= 0) {
 				/* throw away chars on error */
 				i = 0;
 			} else if (r > 0) {

@@ -431,7 +431,7 @@ static void hvc_hangup(struct tty_struct
  * Push buffered characters whether they were just recently buffered or waiting
  * on a blocked hypervisor.  Call this function with hp->lock held.
  */
-static void hvc_push(struct hvc_struct *hp)
+static int hvc_push(struct hvc_struct *hp)
 {
 	int n;
 

@@ -439,7 +439,7 @@ static void hvc_push(struct hvc_struct *
 	if (n <= 0) {
 		if (n == 0) {
 			hp->do_wakeup = 1;
-			return;
+			return 0;
 		}
 		/* throw away output on error; this happens when
 		   there is no session connected to the vterm. */

@@ -450,6 +450,8 @@ static void hvc_push(struct hvc_struct *
 		memmove(hp->outbuf, hp->outbuf + n, hp->n_outbuf);
 	else
 		hp->do_wakeup = 1;
+
+	return n;
 }
 
 static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count)

@@ -538,16 +540,20 @@ int hvc_poll(struct hvc_struct *hp)
 	char buf[N_INBUF] __ALIGNED__;
 	unsigned long flags;
 	int read_total = 0;
+	int written_total = 0;
 
 	spin_lock_irqsave(&hp->lock, flags);
 
 	/* Push pending writes */
 	if (hp->n_outbuf > 0)
-		hvc_push(hp);
+		written_total = hvc_push(hp);
 
 	/* Reschedule us if still some write pending */
-	if (hp->n_outbuf > 0)
+	if (hp->n_outbuf > 0) {
 		poll_mask |= HVC_POLL_WRITE;
+		/* If hvc_push() was not able to write, sleep a few msecs */
+		timeout = (written_total) ? 0 : MIN_TIMEOUT;
+	}
 
 	/* No tty attached, just skip */
 	tty = hp->tty;

@@ -659,10 +665,6 @@ static int khvcd(void *unused)
 			poll_mask |= HVC_POLL_READ;
 		if (hvc_kicked)
 			continue;
-		if (poll_mask & HVC_POLL_WRITE) {
-			yield();
-			continue;
-		}
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (!hvc_kicked) {
 			if (poll_mask == 0)

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Hendrik Brueckner <redacted>

After a tty hangup() or close() operation, processes might not reset the
termio settings to a sane state. In order to reset the termios to its
default settings the tty driver flag TTY_DRIVER_RESET_TERMIOS has been added.

TTY driver flag description from include/linux/tty_driver.h:
	TTY_DRIVER_RESET_TERMIOS --- requests the tty layer to reset the
	  termios setting when the last process has closed the device.
	  Used for PTY's, in particular.


Acked-by: Christian Borntraeger <redacted>
Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/hvc_console.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c

@@ -796,7 +796,7 @@ static int hvc_init(void)
 	drv->minor_start = HVC_MINOR;
 	drv->type = TTY_DRIVER_TYPE_SYSTEM;
 	drv->init_termios = tty_std_termios;
-	drv->flags = TTY_DRIVER_REAL_RAW;
+	drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_RESET_TERMIOS;
 	tty_set_operations(drv, &hvc_ops);
 
 	/* Always start the kthread because there can be hotplug vty adapters

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Hendrik Brueckner <redacted>

I have added a hangup notifier that can be used by hvc console
backends to handle a tty hangup. The default irq hangup notifier
calls the notifier_del_irq() for compatibility.

Acked-by: Christian Borntraeger <redacted>
Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/hvc_console.c    |    4 ++--
 drivers/char/hvc_console.h    |    4 +++-
 drivers/char/hvc_irq.c        |    5 +++++
 drivers/char/hvc_iseries.c    |    1 +
 drivers/char/hvc_vio.c        |    1 +
 drivers/char/hvc_xen.c        |    1 +
 drivers/char/virtio_console.c |    1 +
 7 files changed, 14 insertions(+), 3 deletions(-)

--- a/drivers/char/hvc_console.c	2008-10-08 16:08:40.000000000 +0200
+++ b/drivers/char/hvc_console.c	2008-10-09 10:30:13.000000000 +0200

@@ -416,8 +416,8 @@ static void hvc_hangup(struct tty_struct
 	hp->n_outbuf = 0;
 	hp->tty = NULL;
 
-	if (hp->ops->notifier_del)
-			hp->ops->notifier_del(hp, hp->data);
+	if (hp->ops->notifier_hangup)
+			hp->ops->notifier_hangup(hp, hp->data);
 
 	spin_unlock_irqrestore(&hp->lock, flags);
 
--- a/drivers/char/hvc_console.h	2008-10-08 16:08:40.000000000 +0200
+++ b/drivers/char/hvc_console.h	2008-10-09 10:30:13.000000000 +0200

@@ -65,9 +65,10 @@ struct hv_ops {
 	int (*get_chars)(uint32_t vtermno, char *buf, int count);
 	int (*put_chars)(uint32_t vtermno, const char *buf, int count);
 
-	/* Callbacks for notification. Called in open and close */
+	/* Callbacks for notification. Called in open, close and hangup */
 	int (*notifier_add)(struct hvc_struct *hp, int irq);
 	void (*notifier_del)(struct hvc_struct *hp, int irq);
+	void (*notifier_hangup)(struct hvc_struct *hp, int irq);
 };
 
 /* Register a vterm and a slot index for use as a console (console_init) */

@@ -86,6 +87,7 @@ void hvc_kick(void);
 /* default notifier for irq based notification */
 extern int notifier_add_irq(struct hvc_struct *hp, int data);
 extern void notifier_del_irq(struct hvc_struct *hp, int data);
+extern void notifier_hangup_irq(struct hvc_struct *hp, int data);
 
 
 #if defined(CONFIG_XMON) && defined(CONFIG_SMP)
--- a/drivers/char/hvc_irq.c	2008-10-08 16:08:40.000000000 +0200
+++ b/drivers/char/hvc_irq.c	2008-10-09 10:30:13.000000000 +0200

@@ -42,3 +42,8 @@ void notifier_del_irq(struct hvc_struct 
 	free_irq(irq, hp);
 	hp->irq_requested = 0;
 }
+
+void notifier_hangup_irq(struct hvc_struct *hp, int irq)
+{
+	notifier_del_irq(hp, irq);
+}
--- a/drivers/char/hvc_iseries.c	2008-10-08 16:08:40.000000000 +0200
+++ b/drivers/char/hvc_iseries.c	2008-10-09 10:30:13.000000000 +0200

@@ -202,6 +202,7 @@ static struct hv_ops hvc_get_put_ops = {
 	.put_chars = put_chars,
 	.notifier_add = notifier_add_irq,
 	.notifier_del = notifier_del_irq,
+	.notifier_hangup = notifier_hangup_irq,
 };
 
 static int __devinit hvc_vio_probe(struct vio_dev *vdev,
--- a/drivers/char/hvc_xen.c	2008-10-08 16:08:40.000000000 +0200
+++ b/drivers/char/hvc_xen.c	2008-10-09 10:30:13.000000000 +0200

@@ -102,6 +102,7 @@ static struct hv_ops hvc_ops = {
 	.put_chars = write_console,
 	.notifier_add = notifier_add_irq,
 	.notifier_del = notifier_del_irq,
+	.notifier_hangup = notifier_hangup_irq,
 };
 
 static int __init xen_init(void)
--- a/drivers/char/hvc_vio.c	2008-10-08 16:08:40.000000000 +0200
+++ b/drivers/char/hvc_vio.c	2008-10-09 10:30:13.000000000 +0200

@@ -82,6 +82,7 @@ static struct hv_ops hvc_get_put_ops = {
 	.put_chars = hvc_put_chars,
 	.notifier_add = notifier_add_irq,
 	.notifier_del = notifier_del_irq,
+	.notifier_hangup = notifier_hangup_irq,
 };
 
 static int __devinit hvc_vio_probe(struct vio_dev *vdev,
--- a/drivers/char/virtio_console.c	2008-10-09 10:29:41.000000000 +0200
+++ b/drivers/char/virtio_console.c	2008-10-09 10:30:23.000000000 +0200

@@ -198,6 +198,7 @@ static int __devinit virtcons_probe(stru
 	virtio_cons.put_chars = put_chars;
 	virtio_cons.notifier_add = notifier_add_vio;
 	virtio_cons.notifier_del = notifier_del_vio;
+	virtio_cons.notifier_hangup = notifier_del_vio;
 
 	/* The first argument of hvc_alloc() is the virtual console number, so
 	 * we use zero.  The second argument is the parameter for the

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Hendrik Brueckner <redacted>

The patch provides the hvc_resize() function to update the terminal
window dimensions (struct winsize) for a specified hvc console.
The function stores the new window size and schedules a function
that finally updates the tty winsize and signals the change to
user space (SIGWINCH).
Because the winsize update must acquire a mutex and might sleep,
the function is scheduled instead of being called from hvc_poll()
or khvcd.

Acked-by: Christian Borntraeger <redacted>
Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/hvc_console.c |   61 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/char/hvc_console.h |    6 ++++
 2 files changed, 67 insertions(+)

--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c

@@ -494,6 +494,48 @@ static int hvc_write(struct tty_struct *
 	return written;
 }
 
+/**
+ * hvc_set_winsz() - Resize the hvc tty terminal window.
+ * @work:	work structure.
+ *
+ * The routine shall not be called within an atomic context because it
+ * acquires the tty termios mutex and might sleep.
+ *
+ * Locking:	hp->lock, tty->termios_mutex and tty->ctrl_lock
+ */
+static void hvc_set_winsz(struct work_struct *work)
+{
+	struct hvc_struct *hp;
+	struct pid *pgrp;
+	unsigned long hvc_flags;
+	unsigned long ctrl_flags;
+
+	hp = container_of(work, struct hvc_struct, tty_resize);
+	if (!hp || !hp->tty)
+		return;
+
+	mutex_lock(&hp->tty->termios_mutex);
+	spin_lock_irqsave(&hp->lock, hvc_flags);
+
+	if ((hp->ws.ws_row == hp->tty->winsize.ws_row) &&
+	    (hp->ws.ws_col == hp->tty->winsize.ws_col))
+		goto out_mutex_unlock;
+
+	spin_lock_irqsave(&hp->tty->ctrl_lock, ctrl_flags);
+	pgrp = get_pid(hp->tty->pgrp);
+	spin_unlock_irqrestore(&hp->tty->ctrl_lock, ctrl_flags);
+
+	if (pgrp)
+		kill_pgrp(pgrp, SIGWINCH, 1);
+	put_pid(pgrp);
+
+	hp->tty->winsize = hp->ws;
+
+out_mutex_unlock:
+	spin_unlock_irqrestore(&hp->lock, hvc_flags);
+	mutex_unlock(&hp->tty->termios_mutex);
+}
+
 /*
  * This is actually a contract between the driver and the tty layer outlining
  * how much write room the driver can guarantee will be sent OR BUFFERED.  This

@@ -638,6 +680,24 @@ int hvc_poll(struct hvc_struct *hp)
 }
 EXPORT_SYMBOL_GPL(hvc_poll);
 
+/**
+ * hvc_resize() - Update terminal window size information.
+ * @hp:		HVC console pointer
+ * @ws:		Terminal window size structure
+ *
+ * Stores the specified window size information in the hvc structure of @hp.
+ * The function schedule the tty resize update.
+ *
+ * Locking:	Locking free; the function MUST be called holding hp->lock
+ */
+void hvc_resize(struct hvc_struct *hp, struct winsize ws)
+{
+	if ((hp->ws.ws_row != ws.ws_row) || (hp->ws.ws_col != ws.ws_col)) {
+		hp->ws = ws;
+		schedule_work(&hp->tty_resize);
+	}
+}
+
 /*
  * This kthread is either polling or interrupt driven.  This is determined by
  * calling hvc_poll() who determines whether a console adapter support

@@ -720,6 +780,7 @@ struct hvc_struct __devinit *hvc_alloc(u
 
 	kref_init(&hp->kref);
 
+	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
 	spin_lock_init(&hp->lock);
 	spin_lock(&hvc_structs_lock);
 
--- a/drivers/char/hvc_console.h
+++ b/drivers/char/hvc_console.h

@@ -27,6 +27,7 @@
 #ifndef HVC_CONSOLE_H
 #define HVC_CONSOLE_H
 #include <linux/kref.h>
+#include <linux/tty.h>
 
 /*
  * This is the max number of console adapters that can/will be found as

@@ -56,6 +57,8 @@ struct hvc_struct {
 	struct hv_ops *ops;
 	int irq_requested;
 	int data;
+	struct winsize ws;
+	struct work_struct tty_resize;
 	struct list_head next;
 	struct kref kref; /* ref count & hvc_struct lifetime */
 };

@@ -84,6 +87,9 @@ extern int __devexit hvc_remove(struct h
 int hvc_poll(struct hvc_struct *hp);
 void hvc_kick(void);
 
+/* Resize hvc tty terminal window */
+extern void hvc_resize(struct hvc_struct *hp, struct winsize ws);
+
 /* default notifier for irq based notification */
 extern int notifier_add_irq(struct hvc_struct *hp, int data);
 extern void notifier_del_irq(struct hvc_struct *hp, int data);

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

On Tue, 14 Oct 2008 11:12:49 +0200
Hendrik Brueckner [off-list ref] wrote:

From: Hendrik Brueckner <redacted>

After a tty hangup() or close() operation, processes might not reset the
termio settings to a sane state.

That is the job of the getty task normally. pty is special as the reissue
of the same pty is done as a new device (with new state).

Setting this on the hvc would parallel the PC vt console behaviour but
differ from most other ports.

Anyway its a policy question for PPC64 so if thats how you want it to work

Acked-by: Alan Cox <redacted>

+	hp = container_of(work, struct hvc_struct, tty_resize);
+	if (!hp || !hp->tty)
+		return;

What locks hp->tty here, it can go NULL after the test on a hangup it
seems ?

+ * hvc_resize() - Update terminal window size information.
+ * @hp:		HVC console pointer
+ * @ws:		Terminal window size structure

See tty_do_resize() for all of this stuff in the latest git. If you can't
use tty_do_resize from your work queue then please let me know why as I'd
like everyone to be using one abstraction.

We also now have a "resize" operation for devices that want to react to a
resize from TIOCSWINSZ

On Tue, Oct 14, 2008 at 10:44:28AM +0100, Alan Cox wrote:

quoted

+	hp = container_of(work, struct hvc_struct, tty_resize);
+	if (!hp || !hp->tty)
+		return;

What locks hp->tty here, it can go NULL after the test on a hangup it
seems ?

You are right. Thanks.

See tty_do_resize() for all of this stuff in the latest git. If you can't
use tty_do_resize from your work queue then please let me know why as I'd
like everyone to be using one abstraction.

I have looked at it  and I have corrected my patch to use tty_do_resize().
Since tty_do_resize() cannot be called holding the hp spinlock; the code uses
now tty_kref_get/put to keep track of the tty object. I am not sure if the
use of the kref's is correct here, so please let me know if there is a better
solution.

Thanks.

Regards,
Hendrik

[RFC PATCH 4/5 v2] hvc_console: Add tty window resizing using tty_do_resize()

From: Hendrik Brueckner <redacted>

The patch provides the hvc_resize() function to update the terminal
window dimensions (struct winsize) for a specified hvc console.
The function stores the new window size and schedules a function
that finally updates the tty winsize and signals the change to
user space (SIGWINCH).
Because the winsize update must acquire a mutex and might sleep,
the function is scheduled instead of being called from hvc_poll()
or khvcd.

This patch uses the tty_do_resize() routine from the tty layer.
A pending resize work is canceled in hvc_close() and hvc_hangup().

Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/hvc_console.c |   58 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/char/hvc_console.h |    6 ++++
 2 files changed, 64 insertions(+)

--- a/drivers/char/hvc_console.c	2008-10-14 16:13:12.000000000 +0200
+++ b/drivers/char/hvc_console.c	2008-10-14 16:43:03.000000000 +0200

@@ -374,6 +374,9 @@ static void hvc_close(struct tty_struct 
 		hp->tty = NULL;
 		spin_unlock_irqrestore(&hp->lock, flags);
 
+		/* cancel pending tty resize work */
+		cancel_work_sync(&hp->tty_resize);
+
 		/*
 		 * Chain calls chars_in_buffer() and returns immediately if
 		 * there is no buffered data otherwise sleeps on a wait queue

@@ -399,6 +402,9 @@ static void hvc_hangup(struct tty_struct
 	if (!hp)
 		return;
 
+	/* cancel pending tty resize work */
+	cancel_work_sync(&hp->tty_resize);
+
 	spin_lock_irqsave(&hp->lock, flags);
 
 	/*

@@ -494,6 +500,39 @@ static int hvc_write(struct tty_struct *
 	return written;
 }
 
+/**
+ * hvc_set_winsz() - Resize the hvc tty terminal window.
+ * @work:	work structure.
+ *
+ * The routine shall not be called within an atomic context because it
+ * might sleep.
+ *
+ * Locking:	hp->lock
+ */
+static void hvc_set_winsz(struct work_struct *work)
+{
+	struct hvc_struct *hp;
+	unsigned long hvc_flags;
+	struct tty_struct *tty;
+	struct winsize ws;
+
+	hp = container_of(work, struct hvc_struct, tty_resize);
+	if (!hp)
+		return;
+
+	spin_lock_irqsave(&hp->lock, hvc_flags);
+	if (!hp->tty) {
+		spin_unlock_irqrestore(&hp->lock, hvc_flags);
+		return;
+	}
+	ws  = hp->ws;
+	tty = tty_kref_get(hp->tty);
+	spin_unlock_irqrestore(&hp->lock, hvc_flags);
+
+	tty_do_resize(tty, tty, &ws);
+	tty_kref_put(tty);
+}
+
 /*
  * This is actually a contract between the driver and the tty layer outlining
  * how much write room the driver can guarantee will be sent OR BUFFERED.  This

@@ -638,6 +677,24 @@ int hvc_poll(struct hvc_struct *hp)
 }
 EXPORT_SYMBOL_GPL(hvc_poll);
 
+/**
+ * hvc_resize() - Update terminal window size information.
+ * @hp:		HVC console pointer
+ * @ws:		Terminal window size structure
+ *
+ * Stores the specified window size information in the hvc structure of @hp.
+ * The function schedule the tty resize update.
+ *
+ * Locking:	Locking free; the function MUST be called holding hp->lock
+ */
+void hvc_resize(struct hvc_struct *hp, struct winsize ws)
+{
+	if ((hp->ws.ws_row != ws.ws_row) || (hp->ws.ws_col != ws.ws_col)) {
+		hp->ws = ws;
+		schedule_work(&hp->tty_resize);
+	}
+}
+
 /*
  * This kthread is either polling or interrupt driven.  This is determined by
  * calling hvc_poll() who determines whether a console adapter support

@@ -720,6 +777,7 @@ struct hvc_struct __devinit *hvc_alloc(u
 
 	kref_init(&hp->kref);
 
+	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
 	spin_lock_init(&hp->lock);
 	spin_lock(&hvc_structs_lock);
 
--- a/drivers/char/hvc_console.h	2008-10-14 16:13:11.000000000 +0200
+++ b/drivers/char/hvc_console.h	2008-10-14 16:13:13.000000000 +0200

@@ -27,6 +27,7 @@
 #ifndef HVC_CONSOLE_H
 #define HVC_CONSOLE_H
 #include <linux/kref.h>
+#include <linux/tty.h>
 
 /*
  * This is the max number of console adapters that can/will be found as

@@ -56,6 +57,8 @@ struct hvc_struct {
 	struct hv_ops *ops;
 	int irq_requested;
 	int data;
+	struct winsize ws;
+	struct work_struct tty_resize;
 	struct list_head next;
 	struct kref kref; /* ref count & hvc_struct lifetime */
 };

@@ -84,6 +87,9 @@ extern int __devexit hvc_remove(struct h
 int hvc_poll(struct hvc_struct *hp);
 void hvc_kick(void);
 
+/* Resize hvc tty terminal window */
+extern void hvc_resize(struct hvc_struct *hp, struct winsize ws);
+
 /* default notifier for irq based notification */
 extern int notifier_add_irq(struct hvc_struct *hp, int data);
 extern void notifier_del_irq(struct hvc_struct *hp, int data);

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Since tty_do_resize() cannot be called holding the hp spinlock; the code uses
now tty_kref_get/put to keep track of the tty object. I am not sure if the
use of the kref's is correct here, so please let me know if there is a better
solution.

That looks right to me, hp->tty can go NULL but the tty object itself
will still have a reference even if the asynchronous events kick off late

Hello,

for the sake of completion, here are few more details why I have suggest to
add the TTY_DRIVER_RESET_TERMIOS flag:

On Tue, Oct 14, 2008 at 10:40:25AM +0100, Alan Cox wrote:

On Tue, 14 Oct 2008 11:12:49 +0200
Hendrik Brueckner [off-list ref] wrote:

quoted

After a tty hangup() or close() operation, processes might not reset the
termio settings to a sane state.

That is the job of the getty task normally. pty is special as the reissue
of the same pty is done as a new device (with new state).

During some testing, I have experienced that the bash alters few termios
settings before showing the bash prompt:
-
ioctl(0, TCGETS, {B38400 opost isig icanon echo ...}) = 0
ioctl(0, TCSETSW, {B38400 opost isig -icanon -echo ...}) = 0
write(2, "root@t6345050:~# ", 17) = 17
-
If the tty gets a hangup, the bash gets terminated but the settings still
remains after init has respawned the getty process.
For my network-based hvc backend, a tty_hangup() is caused by a disconnect.

I looked into the tty_io.c source and found out that the termios settings
are stored in an array of the tty driver struct and they remains unchanged
if a tty device is released and initialized again.
At tty device initialization, the tty_init_termios() set tty->termios to the
tty->driver->termios[tty->index].

The idea is to ensure that when a tty is initialized it has the
default (initial) termio settings; and that is actually done if
TTY_DRIVER_RESET_TERMIOS is set.

Anyhow the other possibility might be to always set the initial termios
when the tty is initialized (see patch below).
But I am not sure if that case is the general behavior of ttys.
Please let me know if there is a reason not to re-initialize the termios
of a new tty.

Thanks.

Best regards
Hendrik

Signed-off-by: Hendrik Brueckner <redacted>
---
 drivers/char/tty_io.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/char/tty_io.c	2008-10-16 09:48:00.000000000 +0200
+++ b/drivers/char/tty_io.c	2008-10-16 12:55:52.000000000 +0200

@@ -1243,10 +1243,9 @@ int tty_init_termios(struct tty_struct *
 		tp = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL);
 		if (tp == NULL)
 			return -ENOMEM;
-		memcpy(tp, &tty->driver->init_termios,
-						sizeof(struct ktermios));
 		tty->driver->termios[idx] = tp;
 	}
+	memcpy(tp, &tty->driver->init_termios, sizeof(struct ktermios));
 	tty->termios = tp;
 	tty->termios_locked = tp + 1;
 

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

On Tuesday 14 October 2008 20:12:51 Hendrik Brueckner wrote:

From: Hendrik Brueckner <redacted>

The patch provides the hvc_resize() function to update the terminal
window dimensions (struct winsize) for a specified hvc console.
The function stores the new window size and schedules a function
that finally updates the tty winsize and signals the change to
user space (SIGWINCH).
Because the winsize update must acquire a mutex and might sleep,
the function is scheduled instead of being called from hvc_poll()
or khvcd.

I want this functionality for lguest, too.  But I don't see anything which 
uses this call yet?   Are we going to use the config_change notifier from 
virtio?  (Which IIRC neither S/390 nor lguest have plumbed in).

Cheers,
Rusty.

Am Donnerstag, 16. Oktober 2008 schrieb Rusty Russell:

quoted

The patch provides the hvc_resize() function to update the terminal
window dimensions (struct winsize) for a specified hvc console.
The function stores the new window size and schedules a function
that finally updates the tty winsize and signals the change to
user space (SIGWINCH).
Because the winsize update must acquire a mutex and might sleep,
the function is scheduled instead of being called from hvc_poll()
or khvcd.

I want this functionality for lguest, too.  But I don't see anything which 
uses this call yet?   Are we going to use the config_change notifier from 
virtio?  (Which IIRC neither S/390 nor lguest have plumbed in).

I have some prototype patches for virtio_console, but did not find the time to 
finish them.

Christian

For my network-based hvc backend, a tty_hangup() is caused by a disconnect.

If it is network backed then you probably do want
TTY_DRIVER_TERMIOS_RESET. For a normal serial type port getty is supposed
to sort the terminal settings out. So in your case it sounds like
resetting it is the right thing to do.

On Tue, 2008-10-14 at 11:12 +0200, Hendrik Brueckner wrote:

Hello,

I work on a network-based hvc console backend for s390 that allows
to get "full-screen" terminal access to z/VM guest machines.
The solution consists of a HVC backend that provides the terminal interface;
and a tool to connect to the terminal via a z/VM specific communication
protocol.

 .../...

Hi !

What's the status with that patch serie ? Is this 2.6.28 material ?
Alan, are you ok with those ? Hendrik, have there been any respin other
than v2 of patch 4/5 ?

I can merge it via powerpc but it's getting late in the merge window...

Cheers,
Ben.

On Tue, Oct 21, 2008 at 10:23:37AM +1100, Benjamin Herrenschmidt wrote:

What's the status with that patch serie ? Is this 2.6.28 material ?

It is 2.6.28 material.

Alan, are you ok with those ? Hendrik, have there been any respin other
than v2 of patch 4/5 ?

The other patches have not been changed.

fyi: patch 1/5 (hangup notifier) applies successfully with an offset
of 2 lines due to commit eef2622a9fcfa964073333ea72c7c9cd20ad45e6
(hvc_console: Fix free_irq in spinlocked section).

I can merge it via powerpc but it's getting late in the merge window...

Thanks... that would be really great!

Regards
Hendrik

-- 
Hendrik Brueckner
D/3303 Linux on System z Development
Tel: +49 7031 16-1073
Fax: +49 7031 16-3456
eMail: brueckner@linux.vnet.ibm.com
IBM Deutschland Research & Development GmbH, Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Erich Baier
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294