All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miche Baker-Harvey <miche@google.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>,
	linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
	Amit Shah <amit.shah@redhat.com>,
	Anton Blanchard <anton@samba.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	virtualization@lists.linux-foundation.org,
	Miche Baker-Harvey <miche@google.com>
Subject: [PATCH] VirtioConsole support.
Date: Thu, 27 Oct 2011 14:43:22 -0700	[thread overview]
Message-ID: <1319751802-27013-1-git-send-email-miche@google.com> (raw)
In-Reply-To: <20111027173527.GA23839@phenom.dumpdata.com>

Multiple HVC console terminals enabled.

Serialize device and port open and initialization.  Added a mutex
which gates the handling of control messages in virtio_console.c.
This includes adding and removing ports, and making opened ports be
consoles.  Extended the use of the prvdata spinlock to cover some missed
modifications to prvdata.next_vtermno.

I also added a mutex in hvc_console::hvc_alloc() to coordinate waiting
for the driver to be ready, and for the one-time call to hvc_init().  It
had been the case that this was sometimes being called mulitple times, and
partially setup state was being used by the second caller of hvc_alloc().

Make separate struct console* for each new port.  There was a single static
struct console* hvc_console, to be used for early printk.  We aren't doing
early printk, but more importantly, there is no code to multiplex on that
one console.  Multiple virtio_console ports were "sharing" this, which was
disasterous since both the index and the flags for the console are stored
there. The console struct is remembered in the hvc_struct, and it is
deallocated when the hvc_struct is deallocated.

Signed-off-by: Miche Baker-Harvey <miche@google.com>
Reported-by-and-Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/char/virtio_console.c |   22 +++++++++++++++++++---
 drivers/tty/hvc/hvc_console.c |   39 +++++++++++++++++++++++++++++++++------
 drivers/tty/hvc/hvc_console.h |    1 +
 3 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index fb68b12..e819d46 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -24,6 +24,7 @@
 #include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -95,6 +96,11 @@ struct console {
 	u32 vtermno;
 };
 
+/* serialize the handling of control messages, which includes
+ * the initialization of the virtio_consoles.
+ */
+static DEFINE_MUTEX(virtio_console_mutex);
+
 struct port_buffer {
 	char *buf;
 
@@ -979,8 +985,14 @@ int init_port_console(struct port *port)
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
-	port->cons.vtermno = pdrvdata.next_vtermno;
+	spin_lock_irq(&pdrvdata_lock);
+	port->cons.vtermno = pdrvdata.next_vtermno++;
+	spin_unlock_irq(&pdrvdata_lock);
 
+	/*
+	 * xxx Use index 0 for now assuming there is no early HVC, since
+	 * we don't support it.
+	 */
 	port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
 	if (IS_ERR(port->cons.hvc)) {
 		ret = PTR_ERR(port->cons.hvc);
@@ -990,7 +1002,6 @@ int init_port_console(struct port *port)
 		return ret;
 	}
 	spin_lock_irq(&pdrvdata_lock);
-	pdrvdata.next_vtermno++;
 	list_add_tail(&port->cons.list, &pdrvdata.consoles);
 	spin_unlock_irq(&pdrvdata_lock);
 	port->guest_connected = true;
@@ -1317,7 +1328,6 @@ static void handle_control_message(struct ports_device *portdev,
 	int err;
 
 	cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
-
 	port = find_port_by_id(portdev, cpkt->id);
 	if (!port && cpkt->event != VIRTIO_CONSOLE_PORT_ADD) {
 		/* No valid header at start of buffer.  Drop it. */
@@ -1326,6 +1336,11 @@ static void handle_control_message(struct ports_device *portdev,
 		return;
 	}
 
+	/*
+	 * These are rare initialization-time events that should be
+	 * serialized.
+	 */
+	mutex_lock(&virtio_console_mutex);
 	switch (cpkt->event) {
 	case VIRTIO_CONSOLE_PORT_ADD:
 		if (port) {
@@ -1429,6 +1444,7 @@ static void handle_control_message(struct ports_device *portdev,
 		}
 		break;
 	}
+	mutex_unlock(&virtio_console_mutex);
 }
 
 static void control_work_handler(struct work_struct *work)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 7430bc3..03ff6ed 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -29,8 +29,9 @@
 #include <linux/kernel.h>
 #include <linux/kthread.h>
 #include <linux/list.h>
-#include <linux/module.h>
 #include <linux/major.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/sysrq.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -84,6 +85,10 @@ static LIST_HEAD(hvc_structs);
  * list traversal.
  */
 static DEFINE_SPINLOCK(hvc_structs_lock);
+/*
+ * only one task does allocation at a time.
+ */
+static DEFINE_MUTEX(hvc_ports_mutex);
 
 /*
  * This value is used to assign a tty->index value to a hvc_struct based
@@ -242,6 +247,7 @@ static void destroy_hvc_struct(struct kref *kref)
 
 	spin_unlock(&hvc_structs_lock);
 
+	kfree(hp->hvc_console);
 	kfree(hp);
 }
 
@@ -822,19 +828,25 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 			     int outbuf_size)
 {
 	struct hvc_struct *hp;
+	struct console *cp;
 	int i;
 
 	/* We wait until a driver actually comes along */
+	mutex_lock(&hvc_ports_mutex);
 	if (!hvc_driver) {
 		int err = hvc_init();
-		if (err)
+		if (err) {
+			mutex_unlock(&hvc_ports_mutex);
 			return ERR_PTR(err);
+		}
 	}
 
 	hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
 			GFP_KERNEL);
-	if (!hp)
+	if (!hp) {
+		mutex_unlock(&hvc_ports_mutex);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	hp->vtermno = vtermno;
 	hp->data = data;
@@ -845,6 +857,19 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 	kref_init(&hp->kref);
 
 	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
+	/*
+	 * make each console its own struct console.
+	 * No need to do allocation and copy under lock.
+	 */
+	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
+	if (!cp) {
+		kfree(hp);
+		mutex_unlock(&hvc_ports_mutex);
+		return ERR_PTR(-ENOMEM);
+	}
+	memcpy(cp, &hvc_console, sizeof(*cp));
+	hp->hvc_console = cp;
+
 	spin_lock_init(&hp->lock);
 	spin_lock(&hvc_structs_lock);
 
@@ -862,13 +887,14 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 		i = ++last_hvc;
 
 	hp->index = i;
-	hvc_console.index = i;
+	cp->index = i;
 	vtermnos[i] = vtermno;
 	cons_ops[i] = ops;
 
 	list_add_tail(&(hp->next), &hvc_structs);
 	spin_unlock(&hvc_structs_lock);
-	register_console(&hvc_console);
+	register_console(cp);
+	mutex_unlock(&hvc_ports_mutex);
 
 	return hp;
 }
@@ -879,7 +905,8 @@ int hvc_remove(struct hvc_struct *hp)
 	unsigned long flags;
 	struct tty_struct *tty;
 
-	unregister_console(&hvc_console);
+	BUG_ON(!hp->hvc_console);
+	unregister_console(hp->hvc_console);
 	spin_lock_irqsave(&hp->lock, flags);
 	tty = tty_kref_get(hp->tty);
 
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index c335a14..2d20ab7 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -58,6 +58,7 @@ struct hvc_struct {
 	const struct hv_ops *ops;
 	int irq_requested;
 	int data;
+	struct console *hvc_console;
 	struct winsize ws;
 	struct work_struct tty_resize;
 	struct list_head next;
-- 
1.7.3.1


  parent reply	other threads:[~2011-10-27 21:44 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-27  5:30 Regression: patch " hvc_console: display printk messages on console." causing infinite loop with 3.2-rc0 + Xen Konrad Rzeszutek Wilk
2011-10-27  5:34 ` Konrad Rzeszutek Wilk
2011-10-27  5:48 ` Greg KH
2011-10-27 14:45   ` Miche Baker-Harvey
2011-10-27 14:50     ` Fwd: " Miche Baker-Harvey
2011-10-27 15:08     ` Konrad Rzeszutek Wilk
2011-10-27 15:31       ` Konrad Rzeszutek Wilk
2011-10-27 17:22         ` Miche Baker-Harvey
2011-10-27 17:35           ` Konrad Rzeszutek Wilk
2011-10-27 18:39             ` [PATCH] VirtioConsole support Miche Baker-Harvey
2011-10-27 18:39             ` Miche Baker-Harvey
2011-10-27 21:15               ` Konrad Rzeszutek Wilk
2011-10-27 21:15                 ` Konrad Rzeszutek Wilk
2011-10-31  9:55               ` Amit Shah
2011-10-31  9:55                 ` Amit Shah
2011-10-27 21:43             ` Miche Baker-Harvey [this message]
2011-10-27 21:55               ` Konrad Rzeszutek Wilk
2011-10-27 21:55                 ` Konrad Rzeszutek Wilk
2011-10-28 16:27                 ` Greg KH
2011-10-28 16:27                 ` Greg KH
2011-10-28  2:55               ` Joe Perches
2011-10-28  2:55                 ` Joe Perches
2011-10-28 18:19                 ` Stephen Boyd
2011-11-02 22:02                   ` Rusty Russell
2011-11-02 22:02                     ` Rusty Russell
2011-11-03 12:38                     ` Christian Borntraeger
2011-11-03 12:38                       ` Christian Borntraeger
2011-11-03 14:15                       ` Konrad Rzeszutek Wilk
2011-11-03 14:15                         ` Konrad Rzeszutek Wilk
2011-10-28 18:19                 ` Stephen Boyd
2011-10-27 21:43             ` Miche Baker-Harvey
2011-11-02  1:13   ` Regression: patch " hvc_console: display printk messages on console." causing infinite loop with 3.2-rc0 + Xen Stephen Rothwell
2011-11-02  1:13     ` Stephen Rothwell
2011-11-02  1:13     ` Stephen Rothwell
2011-11-03  1:30     ` Greg KH
2011-11-03  1:30       ` Greg KH
2011-11-03  1:30       ` Greg KH
2011-11-07  6:19       ` Stephen Rothwell
2011-11-07  6:19         ` Stephen Rothwell
2011-11-07  6:19         ` Stephen Rothwell
2011-11-07 20:24         ` Greg KH
2011-11-07 20:24           ` Greg KH
2011-11-07 20:24           ` Greg KH
2011-10-31  7:48 ` Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1319751802-27013-1-git-send-email-miche@google.com \
    --to=miche@google.com \
    --cc=amit.shah@redhat.com \
    --cc=anton@samba.org \
    --cc=benh@kernel.crashing.org \
    --cc=gregkh@suse.de \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.