Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] xen: wait up to 5 minutes for device connection

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


pbonzini at redhat

Jul 3, 2009, 6:12 AM

Post #1 of 7 (148 views)
Permalink
[PATCH] xen: wait up to 5 minutes for device connection

Increases the device timeout from 10s to 5 minutes, giving the user a visual
indication during that time in case there are problems. The patch is a
backport of changesets 144, 146, 150 and 909 in the Xenbits tree.

Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge[at]citrix.com>
Signed-off-by: Paolo Bonzini <pbonzini[at]redhat.com>
---
drivers/xen/xenbus/xenbus_probe.c | 42 +++++++++++++++++++++++++-----------
1 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index d42e25d..4f69159 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -843,7 +843,7 @@ postcore_initcall(xenbus_probe_init);

MODULE_LICENSE("GPL");

-static int is_disconnected_device(struct device *dev, void *data)
+static int is_device_connecting(struct device *dev, void *data)
{
struct xenbus_device *xendev = to_xenbus_device(dev);
struct device_driver *drv = data;
@@ -861,14 +861,15 @@ static int is_disconnected_device(struct device *dev, void *data)
return 0;

xendrv = to_xenbus_driver(dev->driver);
- return (xendev->state != XenbusStateConnected ||
- (xendrv->is_ready && !xendrv->is_ready(xendev)));
+ return (xendev->state < XenbusStateConnected ||
+ (xendev->state == XenbusStateConnected &&
+ xendrv->is_ready && !xendrv->is_ready(xendev)));
}

-static int exists_disconnected_device(struct device_driver *drv)
+static int exists_connecting_device(struct device_driver *drv)
{
return bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
- is_disconnected_device);
+ is_device_connecting);
}

static int print_device_status(struct device *dev, void *data)
@@ -884,10 +885,13 @@ static int print_device_status(struct device *dev, void *data)
/* Information only: is this too noisy? */
printk(KERN_INFO "XENBUS: Device with no driver: %s\n",
xendev->nodename);
- } else if (xendev->state != XenbusStateConnected) {
+ } else if (xendev->state < XenbusStateConnected) {
+ enum xenbus_state rstate = XenbusStateUnknown;
+ if (xendev->otherend)
+ rstate = xenbus_read_driver_state(xendev->otherend);
printk(KERN_WARNING "XENBUS: Timeout connecting "
- "to device: %s (state %d)\n",
- xendev->nodename, xendev->state);
+ "to device: %s (local state %d, remote state %d)\n",
+ xendev->nodename, xendev->state, rstate);
}

return 0;
@@ -897,7 +901,7 @@ static int print_device_status(struct device *dev, void *data)
static int ready_to_wait_for_devices;

/*
- * On a 10 second timeout, wait for all devices currently configured. We need
+ * On a 5-minute timeout, wait for all devices currently configured. We need
* to do this to guarantee that the filesystems and / or network devices
* needed for boot are available, before we can allow the boot to proceed.
*
@@ -912,18 +916,30 @@ static int ready_to_wait_for_devices;
*/
static void wait_for_devices(struct xenbus_driver *xendrv)
{
- unsigned long timeout = jiffies + 10*HZ;
+ unsigned long start = jiffies;
struct device_driver *drv = xendrv ? &xendrv->driver : NULL;
+ unsigned int seconds_waited = 0;

if (!ready_to_wait_for_devices || !xen_domain())
return;

- while (exists_disconnected_device(drv)) {
- if (time_after(jiffies, timeout))
- break;
+ while (exists_connecting_device(drv)) {
+ if (time_after(jiffies, start + (seconds_waited+5)*HZ)) {
+ if (!seconds_waited)
+ printk(KERN_WARNING "XENBUS: Waiting for "
+ "devices to initialise: ");
+ seconds_waited += 5;
+ printk("%us...", 300 - seconds_waited);
+ if (seconds_waited == 300)
+ break;
+ }
+
schedule_timeout_interruptible(HZ/10);
}

+ if (seconds_waited)
+ printk("\n");
+
bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
print_device_status);
}
--
1.6.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


ijc at hellion

Jul 3, 2009, 6:28 AM

Post #2 of 7 (131 views)
Permalink
Re: [PATCH] xen: wait up to 5 minutes for device connection [In reply to]

On Fri, 2009-07-03 at 15:12 +0200, Paolo Bonzini wrote:
> Increases the device timeout from 10s to 5 minutes, giving the user a visual
> indication during that time in case there are problems. The patch is a
> backport of changesets 144, 146, 150 and 909 in the Xenbits tree.
>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge[at]citrix.com>
> Signed-off-by: Paolo Bonzini <pbonzini[at]redhat.com>

Acked-by: Ian Campbell <ian.campbell[at]citrix.com>

Although with the recent printk changes you might need to sprinkle some
KERN_CONT around the place.

Ian.

--
Ian Campbell

"Cable is not a luxury, since many areas have poor TV reception."
-- The mayor of Tucson, Arizona, 1989

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hch at infradead

Jul 3, 2009, 9:00 AM

Post #3 of 7 (131 views)
Permalink
Re: [PATCH] xen: wait up to 5 minutes for device connection [In reply to]

On Fri, Jul 03, 2009 at 03:12:51PM +0200, Paolo Bonzini wrote:
> Increases the device timeout from 10s to 5 minutes, giving the user a visual
> indication during that time in case there are problems. The patch is a
> backport of changesets 144, 146, 150 and 909 in the Xenbits tree.

Why would it take 5 minutes to wait for a device?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


kraxel at redhat

Jul 3, 2009, 2:21 PM

Post #4 of 7 (126 views)
Permalink
Re: [PATCH] xen: wait up to 5 minutes for device connection [In reply to]

On 07/03/09 18:00, Christoph Hellwig wrote:
> On Fri, Jul 03, 2009 at 03:12:51PM +0200, Paolo Bonzini wrote:
>> Increases the device timeout from 10s to 5 minutes, giving the user a visual
>> indication during that time in case there are problems. The patch is a
>> backport of changesets 144, 146, 150 and 909 in the Xenbits tree.
>
> Why would it take 5 minutes to wait for a device?

Usually it is *much* faster, but when the host is quite loaded it can
take a unusual long time. With 10 seconds it happends in practice now
and then that a virtual machine fails to boot just because the virtual
root disk didn't show up fast enough.

cheers,
Gerd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


Valdis.Kletnieks at vt

Jul 4, 2009, 8:15 AM

Post #5 of 7 (119 views)
Permalink
Re: [PATCH] xen: wait up to 5 minutes for device connection [In reply to]

On Fri, 03 Jul 2009 23:21:35 +0200, Gerd Hoffmann said:

> Usually it is *much* faster, but when the host is quite loaded it can
> take a unusual long time. With 10 seconds it happends in practice now
> and then that a virtual machine fails to boot just because the virtual
> root disk didn't show up fast enough.

Are people actually trying to boot a guest on a host machine so loaded that
disks take that long to show up - and expect things to work in any sane
matter?

I'm tempted to suggest that booting under conditions like that is almost
deserving of its own kernel Tainted flag. If devices aren't showing up in
a timely manner, we probably need to be leery of any other kernel timeout
values as well...


kraxel at redhat

Jul 6, 2009, 12:43 AM

Post #6 of 7 (98 views)
Permalink
Re: [PATCH] xen: wait up to 5 minutes for device connection [In reply to]

On 07/04/09 17:15, Valdis.Kletnieks[at]vt.edu wrote:
> On Fri, 03 Jul 2009 23:21:35 +0200, Gerd Hoffmann said:
>
>> Usually it is *much* faster, but when the host is quite loaded it can
>> take a unusual long time. With 10 seconds it happends in practice now
>> and then that a virtual machine fails to boot just because the virtual
>> root disk didn't show up fast enough.
>
> Are people actually trying to boot a guest on a host machine so loaded that
> disks take that long to show up - and expect things to work in any sane
> matter?

Even on machines which can handle their load just fine under normal
circumstances you may see that behavior in case of load peaks. Booting
a bunch of virtual machines at the same time can trigger it for example.

> I'm tempted to suggest that booting under conditions like that is almost
> deserving of its own kernel Tainted flag. If devices aren't showing up in
> a timely manner, we probably need to be leery of any other kernel timeout
> values as well...

It is pointless to taint just because of a unusual delay at some random
point in time. A virtual machine can see higher delays due to load
peaks on the host anytime. The flag wouldn't carry any useful information.

You could add TAINT_VIRT to flag *all* vm guests, but as the boot log
gives you that information already it is pointless too IMHO.

cheers,
Gerd

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


Jeremy.Fitzhardinge at citrix

Jul 6, 2009, 2:54 PM

Post #7 of 7 (93 views)
Permalink
Re: [PATCH] xen: wait up to 5 minutes for device connection [In reply to]

On 07/03/09 06:12, Paolo Bonzini wrote:
> Increases the device timeout from 10s to 5 minutes, giving the user a visual
> indication during that time in case there are problems. The patch is a
> backport of changesets 144, 146, 150 and 909 in the Xenbits tree.
>

This patch does a lot more than change the timeout. Please split it
into logically distinct patches (one for each of the changesets, if that
makes sense) with proper descriptions and a note linking it back to the
source changeset.

Thanks,
J

> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge[at]citrix.com>
> Signed-off-by: Paolo Bonzini <pbonzini[at]redhat.com>
> ---
> drivers/xen/xenbus/xenbus_probe.c | 42 +++++++++++++++++++++++++-----------
> 1 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> index d42e25d..4f69159 100644
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -843,7 +843,7 @@ postcore_initcall(xenbus_probe_init);
>
> MODULE_LICENSE("GPL");
>
> -static int is_disconnected_device(struct device *dev, void *data)
> +static int is_device_connecting(struct device *dev, void *data)
> {
> struct xenbus_device *xendev = to_xenbus_device(dev);
> struct device_driver *drv = data;
> @@ -861,14 +861,15 @@ static int is_disconnected_device(struct device *dev, void *data)
> return 0;
>
> xendrv = to_xenbus_driver(dev->driver);
> - return (xendev->state != XenbusStateConnected ||
> - (xendrv->is_ready && !xendrv->is_ready(xendev)));
> + return (xendev->state < XenbusStateConnected ||
> + (xendev->state == XenbusStateConnected &&
> + xendrv->is_ready && !xendrv->is_ready(xendev)));
> }
>
> -static int exists_disconnected_device(struct device_driver *drv)
> +static int exists_connecting_device(struct device_driver *drv)
> {
> return bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
> - is_disconnected_device);
> + is_device_connecting);
> }
>
> static int print_device_status(struct device *dev, void *data)
> @@ -884,10 +885,13 @@ static int print_device_status(struct device *dev, void *data)
> /* Information only: is this too noisy? */
> printk(KERN_INFO "XENBUS: Device with no driver: %s\n",
> xendev->nodename);
> - } else if (xendev->state != XenbusStateConnected) {
> + } else if (xendev->state < XenbusStateConnected) {
> + enum xenbus_state rstate = XenbusStateUnknown;
> + if (xendev->otherend)
> + rstate = xenbus_read_driver_state(xendev->otherend);
> printk(KERN_WARNING "XENBUS: Timeout connecting "
> - "to device: %s (state %d)\n",
> - xendev->nodename, xendev->state);
> + "to device: %s (local state %d, remote state %d)\n",
> + xendev->nodename, xendev->state, rstate);
> }
>
> return 0;
> @@ -897,7 +901,7 @@ static int print_device_status(struct device *dev, void *data)
> static int ready_to_wait_for_devices;
>
> /*
> - * On a 10 second timeout, wait for all devices currently configured. We need
> + * On a 5-minute timeout, wait for all devices currently configured. We need
> * to do this to guarantee that the filesystems and / or network devices
> * needed for boot are available, before we can allow the boot to proceed.
> *
> @@ -912,18 +916,30 @@ static int ready_to_wait_for_devices;
> */
> static void wait_for_devices(struct xenbus_driver *xendrv)
> {
> - unsigned long timeout = jiffies + 10*HZ;
> + unsigned long start = jiffies;
> struct device_driver *drv = xendrv ? &xendrv->driver : NULL;
> + unsigned int seconds_waited = 0;
>
> if (!ready_to_wait_for_devices || !xen_domain())
> return;
>
> - while (exists_disconnected_device(drv)) {
> - if (time_after(jiffies, timeout))
> - break;
> + while (exists_connecting_device(drv)) {
> + if (time_after(jiffies, start + (seconds_waited+5)*HZ)) {
> + if (!seconds_waited)
> + printk(KERN_WARNING "XENBUS: Waiting for "
> + "devices to initialise: ");
> + seconds_waited += 5;
> + printk("%us...", 300 - seconds_waited);
> + if (seconds_waited == 300)
> + break;
> + }
> +
> schedule_timeout_interruptible(HZ/10);
> }
>
> + if (seconds_waited)
> + printk("\n");
> +
> bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
> print_device_status);
> }
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.