hvc_console & interrupts: reworked patch

Benjamin Herrenschmidt benh at kernel.crashing.org
Tue Apr 13 18:21:56 EST 2004


Hi !

This patch is _TOTALLY UNTESTED_, I'll do that tomorrow, but at least
the chosen approach can be discussed here in the meantime. So I rewrote
Ryan's patch in a different way that, of course, I personally prefer ;)

Note that I know I'm kicking the thread a bit too often on normal writes,
I've been toying with the idea of adding a parameter to kick,
basically, NOW vs. LATER, the later one only kicking if blocked into
schedule(), not into schedule_timeout(), but I was too lazy to add
the necessary locking infrastructure for that. Same goes with the
interrupt which may trigger the thread spurriously if throttled,
though that's easily fixable if it's considered as a real issue.

Ryan: I suppose the interrupt acts like an "edge" trigger that gets
re-armed when HV returns 0 bytes on a read, right ?

Patch is against current ameslab 2.5

Ben.

===== arch/ppc64/kernel/hvconsole.c 1.10 vs edited =====
--- 1.10/arch/ppc64/kernel/hvconsole.c	Sat Apr 10 08:11:48 2004
+++ edited/arch/ppc64/kernel/hvconsole.c	Tue Apr 13 18:00:08 2004
@@ -30,6 +30,7 @@
 #include <linux/list.h>
 #include <linux/time.h>
 #include <linux/ctype.h>
+#include <linux/interrupt.h>
 #include <asm/delay.h>
 #include <asm/hvcall.h>
 #include <asm/prom.h>
@@ -50,6 +51,7 @@
 	int (*tiocmset)(struct vtty_struct *vtty, uint16_t set, uint16_t clear);
 	uint16_t seqno; /* HVSI packet sequence number */
 	uint16_t mctrl;
+	int irq;
 };
 static struct vtty_struct vttys[MAX_NR_HVC_CONSOLES];

@@ -718,6 +720,7 @@
 {
 	struct device_node *vty;
 	int count = 0;
+	unsigned int *irq_p;

 	for (vty = of_find_node_by_name(NULL, "vty"); vty != NULL;
 			vty = of_find_node_by_name(vty, "vty")) {
@@ -732,12 +735,20 @@
 			break;

 		vtty = &vttys[count];
+		vtty->irq = NO_IRQ;
 		if (device_is_compatible(vty, "hvterm1")) {
 			vtty->vtermno = *vtermno;
 			vtty->get_chars = hvc_hvterm_get_chars;
 			vtty->put_chars = hvc_hvterm_put_chars;
 			vtty->tiocmget = NULL;
 			vtty->tiocmset = NULL;
+			vtty->irq = NO_IRQ;
+			irq_p = (unsigned int *)get_property(vty, "interrupts", 0);
+			if (irq_p) {
+				int virq = virt_irq_create_mapping(*irq_p);
+				if (virq != NO_IRQ)
+					vtty->irq = irq_offset_up(virq);
+			}
 			hvc_instantiate();
 			count++;
 		} else if (device_is_compatible(vty, "hvterm-protocol")) {
@@ -758,6 +769,14 @@

 	return count;
 }
+
+int hvc_interruptible(int index)
+{
+	struct vtty_struct *vtty = &vttys[index];
+
+	/* If not interruptible then it'll return NO_IRQ */
+	return vtty->irq;
+}

 /* Convert arch specific return codes into relevant errnos.  The hvcs
  * functions aren't performance sensitive, so this conversion isn't an
===== drivers/char/hvc_console.c 1.25 vs edited =====
--- 1.25/drivers/char/hvc_console.c	Sat Apr 10 08:11:48 2004
+++ edited/drivers/char/hvc_console.c	Tue Apr 13 18:14:54 2004
@@ -38,12 +38,15 @@
 #define TIMEOUT		((HZ + 99) / 100)

 static struct tty_driver *hvc_driver;
-static int count;
+static int hvc_count;
+static int hvc_kicked;
+static wait_queue_head_t hvc_wait_queue;
 #ifdef CONFIG_MAGIC_SYSRQ
 static int sysrq_pressed;
 #endif

 #define N_OUTBUF	16
+#define N_INBUF		16

 #define __ALIGNED__	__attribute__((__aligned__(8)))

@@ -59,15 +62,34 @@
 	int do_wakeup;
 	char outbuf[N_OUTBUF] __ALIGNED__;
 	int n_outbuf;
+	int irq_requested;
 };

 struct hvc_struct hvc_struct[MAX_NR_HVC_CONSOLES];

+static void hvc_kick(void)
+{
+	hvc_kicked = 1;
+	wake_up_interruptible(&hvc_wait_queue);
+}
+
+static irqreturn_t hvc_handle_interrupt(int irq, void *dev_instance, struct pt_regs *regs)
+{
+	hvc_kick();
+	return IRQ_HANDLED;
+}
+
+static void hvc_unthrottle(struct tty_struct *tty)
+{
+	hvc_kick();
+}
+
 static int hvc_open(struct tty_struct *tty, struct file * filp)
 {
 	int line = tty->index;
 	struct hvc_struct *hp;
 	unsigned long flags;
+	int irq = NO_IRQ;

 	if (line < 0 || line >= MAX_NR_HVC_CONSOLES)
 		return -ENODEV;
@@ -77,7 +99,18 @@
 	spin_lock_irqsave(&hp->lock, flags);
 	hp->tty = tty;
 	hp->count++;
+	if (hp->count == 1) {
+		irq = hvc_interruptible(hp->index);
+		if (irq != NO_IRQ)
+			hp->irq_requested = 1;
+	}
 	spin_unlock_irqrestore(&hp->lock, flags);
+	/* XX check error, fallback to non-irq ? */
+	if (irq != NO_IRQ)
+		request_irq(irq, hvc_handle_interrupt, SA_INTERRUPT, "hvc_console", hp);
+
+	/* Force wakeup of the polling thread */
+	hvc_kick();

 	return 0;
 }
@@ -86,16 +119,28 @@
 {
 	struct hvc_struct *hp = tty->driver_data;
 	unsigned long flags;
+	int irq = NO_IRQ;

-	if (tty_hung_up_p(filp))
-		return;
 	spin_lock_irqsave(&hp->lock, flags);
-	if (--hp->count == 0)
+	if (tty_hung_up_p(filp)) {
+		if (hp->irq_requested)
+			irq = hvc_interruptible(hp->index);
+		hp->irq_requested = 0;
+		goto bail;
+	}
+
+	if (--hp->count == 0) {
 		hp->tty = NULL;
-	else if (hp->count < 0)
+		if (hp->irq_requested)
+			irq = hvc_interruptible(hp->index);
+		hp->irq_requested = 0;
+	} else if (hp->count < 0)
 		printk(KERN_ERR "hvc_close %lu: oops, count is %d\n",
 		       hp - hvc_struct, hp->count);
+ bail:
 	spin_unlock_irqrestore(&hp->lock, flags);
+	if (irq != NO_IRQ)
+		free_irq(irq, hp);
 }

 static void hvc_hangup(struct tty_struct *tty)
@@ -126,70 +171,94 @@
 		hp->do_wakeup = 1;
 }

-static int hvc_write(struct tty_struct *tty, int from_user,
-		     const unsigned char *buf, int count)
+static inline int __hvc_write_user(struct hvc_struct *hp,
+				   const unsigned char *buf, int count)
 {
-	struct hvc_struct *hp = tty->driver_data;
 	char *tbuf, *p;
 	int tbsize, rsize, written = 0;
 	unsigned long flags;

-	if (from_user) {
-		tbsize = min(count, (int)PAGE_SIZE);
-		if (!(tbuf = kmalloc(tbsize, GFP_KERNEL)))
-			return -ENOMEM;
-
-		while ((rsize = count - written) > 0) {
-			int wsize;
-			if (rsize > tbsize)
-				rsize = tbsize;
-
-			p = tbuf;
-			rsize -= copy_from_user(p, buf, rsize);
-			if (!rsize) {
-				if (written == 0)
-					written = -EFAULT;
-				break;
-			}
-			buf += rsize;
-			written += rsize;
-
-			spin_lock_irqsave(&hp->lock, flags);
-			for (wsize = N_OUTBUF - hp->n_outbuf; rsize && wsize;
-					wsize = N_OUTBUF - hp->n_outbuf) {
-				if (wsize > rsize)
-					wsize = rsize;
-				memcpy(hp->outbuf + hp->n_outbuf, p, wsize);
-				hp->n_outbuf += wsize;
-				hvc_push(hp);
-				rsize -= wsize;
-				p += wsize;
-			}
-			spin_unlock_irqrestore(&hp->lock, flags);
-
-			if (rsize)
-				break;
+	tbsize = min(count, (int)PAGE_SIZE);
+	if (!(tbuf = kmalloc(tbsize, GFP_KERNEL)))
+		return -ENOMEM;

-			if (count < tbsize)
-				tbsize = count;
+	while ((rsize = count - written) > 0) {
+		int wsize;
+		if (rsize > tbsize)
+			rsize = tbsize;
+
+		p = tbuf;
+		rsize -= copy_from_user(p, buf, rsize);
+		if (!rsize) {
+			if (written == 0)
+				written = -EFAULT;
+			break;
 		}
+		buf += rsize;

-		kfree(tbuf);
-	} else {
 		spin_lock_irqsave(&hp->lock, flags);
-		while (count > 0 && (rsize = N_OUTBUF - hp->n_outbuf) > 0) {
-			if (rsize > count)
-				rsize = count;
-			memcpy(hp->outbuf + hp->n_outbuf, buf, rsize);
-			count -= rsize;
-			buf += rsize;
-			hp->n_outbuf += rsize;
-			written += rsize;
+		for (wsize = N_OUTBUF - hp->n_outbuf; rsize && wsize;
+		     wsize = N_OUTBUF - hp->n_outbuf) {
+			if (wsize > rsize)
+				wsize = rsize;
+			memcpy(hp->outbuf + hp->n_outbuf, p, wsize);
+			hp->n_outbuf += wsize;
 			hvc_push(hp);
+			rsize -= wsize;
+			p += wsize;
+			written += wsize;
 		}
 		spin_unlock_irqrestore(&hp->lock, flags);
+
+		if (rsize)
+			break;
+
+		if (count < tbsize)
+			tbsize = count;
 	}

+	kfree(tbuf);
+
+	return written;
+}
+
+static inline int __hvc_write_kernel(struct hvc_struct *hp,
+				   const unsigned char *buf, int count)
+{
+	unsigned long flags;
+	int rsize, written = 0;
+
+	spin_lock_irqsave(&hp->lock, flags);
+	while (count > 0 && (rsize = N_OUTBUF - hp->n_outbuf) > 0) {
+		if (rsize > count)
+			rsize = count;
+		memcpy(hp->outbuf + hp->n_outbuf, buf, rsize);
+		count -= rsize;
+		buf += rsize;
+		hp->n_outbuf += rsize;
+		written += rsize;
+		hvc_push(hp);
+	}
+	spin_unlock_irqrestore(&hp->lock, flags);
+
+	return written;
+}
+
+static int hvc_write(struct tty_struct *tty, int from_user,
+		     const unsigned char *buf, int count)
+{
+	struct hvc_struct *hp = tty->driver_data;
+	int written;
+
+	if (from_user)
+		written = __hvc_write_user(hp, buf, count);
+	else
+		written = __hvc_write_kernel(hp, buf, count);
+
+	/* Racy, but harmless, kick thread if there are still pending data */
+	if (hp->n_outbuf)
+		hvc_kick();
+
 	return written;
 }

@@ -207,59 +276,103 @@
 	return hp->n_outbuf;
 }

-static void hvc_poll(int index)
+#define HVC_POLL_READ	0x00000001
+#define HVC_POLL_WRITE	0x00000002
+#define HVC_POLL_QUICK	0x00000004
+
+static int hvc_poll(int index)
 {
 	struct hvc_struct *hp = &hvc_struct[index];
 	struct tty_struct *tty;
-	int i, n;
-	char buf[16] __ALIGNED__;
+	int i, n, poll_mask = 0;
+	char buf[N_INBUF] __ALIGNED__;
 	unsigned long flags;
+	int read_total = 0;

 	spin_lock_irqsave(&hp->lock, flags);

+	/* Push pending writes */
 	if (hp->n_outbuf > 0)
 		hvc_push(hp);
+	/* Reschedule us if still some write pending */
+	if (hp->n_outbuf > 0)
+		poll_mask |= HVC_POLL_WRITE;

+	/* No tty attached, just skip */
 	tty = hp->tty;
-	if (tty) {
-		for (;;) {
-			if (TTY_FLIPBUF_SIZE - tty->flip.count < sizeof(buf))
-				break;
-			n = hvc_arch_get_chars(index, buf, sizeof(buf));
-			if (n <= 0) {
-				if (n == -EPIPE) {
-					printk("tty_hangup\n");
-					tty_hangup(tty);
-				}
-				break;
-			}
-			for (i = 0; i < n; ++i) {
-#ifdef CONFIG_MAGIC_SYSRQ		/* Handle the SysRq Hack */
-				if (buf[i] == '\x0f') {	/* ^O -- should support a sequence */
-					sysrq_pressed = 1;
-					continue;
-				} else if (sysrq_pressed) {
-					handle_sysrq(buf[i], NULL, tty);
-					sysrq_pressed = 0;
-					continue;
-				}
-#endif
-				tty_insert_flip_char(tty, buf[i], 0);
+	if (tty == NULL)
+		goto bail;
+
+	/* Now check if we can get data (are we throttled ?) */
+	if (test_bit(TTY_THROTTLED, &tty->flags))
+		goto throttled;
+
+	/* If we aren't interrupt driven and aren't throttled, we always
+	 * request a reschedule
+	 */
+	if (!hvc_interruptible(index))
+		poll_mask |= HVC_POLL_READ;
+
+	/* Read data if any */
+	for (;;) {
+		int count = N_INBUF;
+		if (count > (TTY_FLIPBUF_SIZE - tty->flip.count))
+			count = TTY_FLIPBUF_SIZE - tty->flip.count;
+
+		/* If flip is full, just reschedule a later read */
+		if (count == 0) {
+			poll_mask |= HVC_POLL_READ;
+			break;
+		}
+
+		n = hvc_arch_get_chars(index, buf, count);
+		if (n <= 0) {
+			/* XXX What is that supposed to do ? */
+			if (n == -EPIPE)
+				tty_hangup(tty);
+			break;
+		}
+		for (i = 0; i < n; ++i) {
+#ifdef CONFIG_MAGIC_SYSRQ
+			/* Handle the SysRq Hack */
+			if (buf[i] == '\x0f') {	/* ^O -- should support a sequence */
+				sysrq_pressed = 1;
+				continue;
+			} else if (sysrq_pressed) {
+				handle_sysrq(buf[i], NULL, tty);
+				sysrq_pressed = 0;
+				continue;
 			}
+#endif /* CONFIG_MAGIC_SYSRQ */
+			tty_insert_flip_char(tty, buf[i], 0);
 		}
+
 		if (tty->flip.count)
 			tty_schedule_flip(tty);

-		if (hp->do_wakeup) {
-			hp->do_wakeup = 0;
-			if ((tty->flags & (1 << TTY_DO_WRITE_WAKEUP))
-			    && tty->ldisc.write_wakeup)
-				(tty->ldisc.write_wakeup)(tty);
-			wake_up_interruptible(&tty->write_wait);
+		/* Account the total amount read in one loop, and if above 64 bytes,
+		 * we do a quick schedule loop to let the tty grok the data and
+		 * eventually throttle us
+		 */
+		read_total += n;
+		if (read_total >= 64) {
+			poll_mask |= HVC_POLL_QUICK;
+			break;
 		}
 	}
-
+ throttled:
+	/* Wakeup write queue if necessary */
+	if (hp->do_wakeup) {
+		hp->do_wakeup = 0;
+		if ((tty->flags & (1 << TTY_DO_WRITE_WAKEUP))
+		    && tty->ldisc.write_wakeup)
+			(tty->ldisc.write_wakeup)(tty);
+		wake_up_interruptible(&tty->write_wait);
+	}
+ bail:
 	spin_unlock_irqrestore(&hp->lock, flags);
+
+	return poll_mask;
 }

 #if defined(CONFIG_XMON) && defined(CONFIG_SMP)
@@ -271,17 +384,37 @@

 int khvcd(void *unused)
 {
-	int i;
+	int i, poll_mask;

 	daemonize("khvcd");

 	for (;;) {
+		wait_queue_t wait = __WAITQUEUE_INITIALIZER(wait,current);
+
+		poll_mask = 0;
+		hvc_kicked = 0;
+		wmb();
 		if (cpus_empty(cpus_in_xmon)) {
 			for (i = 0; i < MAX_NR_HVC_CONSOLES; ++i)
-				hvc_poll(i);
+				poll_mask |= hvc_poll(i);
+		} else
+			poll_mask |= HVC_POLL_READ;
+		if (hvc_kicked)
+			continue;
+		if (poll_mask & HVC_POLL_QUICK) {
+			yield();
+			continue;
 		}
+		add_wait_queue(&hvc_wait_queue, &wait);
 		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(TIMEOUT);
+		if (!hvc_kicked) {
+			if (poll_mask == 0)
+				schedule();
+			else
+				schedule_timeout(TIMEOUT);
+		}
+		set_current_state(TASK_RUNNING);
+		remove_wait_queue(&hvc_wait_queue, &wait);
 	}
 }

@@ -314,6 +447,7 @@
 	.close = hvc_close,
 	.write = hvc_write,
 	.hangup = hvc_hangup,
+	.unthrottle = hvc_unthrottle,
 	.write_room = hvc_write_room,
 	.chars_in_buffer = hvc_chars_in_buffer,
 	.tiocmget = hvc_tiocmget,
@@ -322,7 +456,9 @@

 int __init hvc_init(void)
 {
-	hvc_driver = alloc_tty_driver(count);
+	init_waitqueue_head(&hvc_wait_queue);
+
+	hvc_driver = alloc_tty_driver(hvc_count);
 	if (!hvc_driver)
 		return -ENOMEM;

@@ -340,7 +476,7 @@
 	if (tty_register_driver(hvc_driver))
 		panic("Couldn't register hvc console driver\n");

-	if (count > 0)
+	if (hvc_count > 0)
 		kernel_thread(khvcd, NULL, CLONE_KERNEL);
 	else
 		printk(KERN_WARNING "no virtual consoles found\n");
@@ -391,7 +527,7 @@
 static int __init hvc_console_setup(struct console *co, char *options)
 {
 	if (co->index < 0 || co->index >= MAX_NR_HVC_CONSOLES
-	    || co->index >= count)
+	    || co->index >= hvc_count)
 		return -1;
 	return 0;
 }
@@ -410,14 +546,14 @@
 {
 	struct hvc_struct *hvc;

-	if (count >= MAX_NR_HVC_CONSOLES)
+	if (hvc_count >= MAX_NR_HVC_CONSOLES)
 		return -1;

-	hvc = &hvc_struct[count];
+	hvc = &hvc_struct[hvc_count];
 	hvc->lock = SPIN_LOCK_UNLOCKED;
-	hvc->index = count;
+	hvc->index = hvc_count;

-	count++;
+	hvc_count++;

 	return 0;
 }
===== include/asm-ppc64/hvconsole.h 1.7 vs edited =====
--- 1.7/include/asm-ppc64/hvconsole.h	Sat Apr 10 08:11:48 2004
+++ edited/include/asm-ppc64/hvconsole.h	Tue Apr 13 18:00:25 2004
@@ -57,6 +57,7 @@
 extern int hvcs_get_partner_info(unsigned int unit_address, struct list_head *head);
 extern int hvcs_register_connection(unsigned int unit_address, unsigned int p_partition_ID, unsigned int p_unit_address);
 extern int hvcs_free_connection(unsigned int unit_address);
+extern int hvc_interruptible(int index);

 #endif /* _PPC64_HVCONSOLE_H */


** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/





More information about the Linuxppc64-dev mailing list