Re: [RFC/PATCH] virtio-console: wait for console ports
Hi Christian,
On (Wed) 21 Sep 2011 [17:52:23], Christian Borntraeger wrote:
Amit,
can you have a look at the patch below and give feedback or apply
if appropriate?
The patch looks good. Just a couple of comments:
On s390 I have seen some random Warning: unable to open an initial
console boot failure. Turns out that tty_open fails, because the
hvc_alloc was not yet done. In former times this could not happen,
since the probe function automatically called hvc_alloc. With newer
versions (multiport) some host-guest interaction is required
before hvc_alloc is called. This might be too late, especially if
an initramfs is involved. Lets use a completion if we have
multiport and an early console.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
drivers/char/virtio_console.c | 18 ++
1 file changed, 18 insertions(+)
Index: b/drivers/char/virtio_console.c
===
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -19,6 +19,7 @@
*/
#include linux/cdev.h
#include linux/debugfs.h
+#include linux/completion.h
#include linux/device.h
#include linux/err.h
#include linux/fs.h
@@ -73,6 +74,7 @@ struct ports_driver_data {
static struct ports_driver_data pdrvdata;
DEFINE_SPINLOCK(pdrvdata_lock);
+DECLARE_COMPLETION(port_added);
/* This struct holds information that's relevant only for console ports */
struct console {
@@ -1352,6 +1354,7 @@ static void handle_control_message(struc
break;
init_port_console(port);
+ complete(port_added);
/*
* Could remove the port here in case init fails - but
* have to notify the host first.
@@ -1648,6 +1651,10 @@ static int __devinit virtcons_probe(stru
struct ports_device *portdev;
int err;
bool multiport;
+ bool early = early_put_chars != 0;
Check for NULL instead of 0. Is it necessary to create this variable
instead of checking for early_put_chars != NULL below?
+
+ /* Ensure to read early_put_chars now */
+ barrier();
portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
if (!portdev) {
@@ -1719,6 +1726,17 @@ static int __devinit virtcons_probe(stru
__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
VIRTIO_CONSOLE_DEVICE_READY, 1);
+
+ /* If there was an early virtio console, assume that there are no
+ * other consoles. We need to wait until the hvc_alloc matches the
+ * hvc_instantiate, otherwise tty_open will complain, resulting in
+ * a Warning: unable to open an initial console boot failure.
+ * Without multiport this is done in add_port above. With multiport
+ * this might take some host-guest communication - thus we have to
+ * wait. */
This file uses comments in the form
/*
* ...
*/
+ if (multiport early)
+ wait_for_completion(port_added);
+
Can there be a problem to not timeout this wait? Maybe it's not a
real problem; just thinking out aloud.
Amit
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] virtio-console: wait for console ports
On 22/09/11 12:08, Amit Shah wrote: +/* If there was an early virtio console, assume that there are no + * other consoles. We need to wait until the hvc_alloc matches the + * hvc_instantiate, otherwise tty_open will complain, resulting in + * a Warning: unable to open an initial console boot failure. + * Without multiport this is done in add_port above. With multiport + * this might take some host-guest communication - thus we have to + * wait. */ This file uses comments in the form /* * ... */ Will fix. +if (multiport early) +wait_for_completion(port_added); + Can there be a problem to not timeout this wait? Maybe it's not a real problem; just thinking out aloud. I had the same thoughts. I then asked myself, how big the timeout has to be - and the answer was it really depends on the host load. So we can certainly use wait_for_completion_timeout(port_added, HZ*x); which will work in 99.9% of all cases. It might still cause spurious boot failures, if for some reasons it takes too long. So the big question is, is there a case were virtio is used as an early console but virtio_console does not register a console during probe. Ideas? Christian -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] virtio-console: wait for console ports
On (Thu) 22 Sep 2011 [13:20:07], Christian Borntraeger wrote: On 22/09/11 12:08, Amit Shah wrote: + /* If there was an early virtio console, assume that there are no + * other consoles. We need to wait until the hvc_alloc matches the + * hvc_instantiate, otherwise tty_open will complain, resulting in + * a Warning: unable to open an initial console boot failure. + * Without multiport this is done in add_port above. With multiport + * this might take some host-guest communication - thus we have to + * wait. */ This file uses comments in the form /* * ... */ Will fix. + if (multiport early) + wait_for_completion(port_added); + Can there be a problem to not timeout this wait? Maybe it's not a real problem; just thinking out aloud. I had the same thoughts. I then asked myself, how big the timeout has to be - and the answer was it really depends on the host load. So we can certainly use wait_for_completion_timeout(port_added, HZ*x); which will work in 99.9% of all cases. It might still cause spurious boot failures, if for some reasons it takes too long. Yes; there's no deterministic way. So the big question is, is there a case were virtio is used as an early console but virtio_console does not register a console during probe. Ideas? Currently only ppc and s390 use early_console. And since you're sending this patch now, I guess you've started using multiport in the host recently. So it's really upto you to decide :-) I think this is benign as of now. Amit -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] virtio-console: wait for console ports
Amit,
can you have a look at the patch below and give feedback or apply
if appropriate?
---
On s390 I have seen some random Warning: unable to open an initial
console boot failure. Turns out that tty_open fails, because the
hvc_alloc was not yet done. In former times this could not happen,
since the probe function automatically called hvc_alloc. With newer
versions (multiport) some host-guest interaction is required
before hvc_alloc is called. This might be too late, especially if
an initramfs is involved. Lets use a completion if we have
multiport and an early console.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
drivers/char/virtio_console.c | 18 ++
1 file changed, 18 insertions(+)
Index: b/drivers/char/virtio_console.c
===
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -19,6 +19,7 @@
*/
#include linux/cdev.h
#include linux/debugfs.h
+#include linux/completion.h
#include linux/device.h
#include linux/err.h
#include linux/fs.h
@@ -73,6 +74,7 @@ struct ports_driver_data {
static struct ports_driver_data pdrvdata;
DEFINE_SPINLOCK(pdrvdata_lock);
+DECLARE_COMPLETION(port_added);
/* This struct holds information that's relevant only for console ports */
struct console {
@@ -1352,6 +1354,7 @@ static void handle_control_message(struc
break;
init_port_console(port);
+ complete(port_added);
/*
* Could remove the port here in case init fails - but
* have to notify the host first.
@@ -1648,6 +1651,10 @@ static int __devinit virtcons_probe(stru
struct ports_device *portdev;
int err;
bool multiport;
+ bool early = early_put_chars != 0;
+
+ /* Ensure to read early_put_chars now */
+ barrier();
portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
if (!portdev) {
@@ -1719,6 +1726,17 @@ static int __devinit virtcons_probe(stru
__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
VIRTIO_CONSOLE_DEVICE_READY, 1);
+
+ /* If there was an early virtio console, assume that there are no
+* other consoles. We need to wait until the hvc_alloc matches the
+* hvc_instantiate, otherwise tty_open will complain, resulting in
+* a Warning: unable to open an initial console boot failure.
+* Without multiport this is done in add_port above. With multiport
+* this might take some host-guest communication - thus we have to
+* wait. */
+ if (multiport early)
+ wait_for_completion(port_added);
+
return 0;
free_vqs:
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
