* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: KY Srinivasan @ 2011-02-19 16:46 UTC (permalink / raw)
To: Thomas Gleixner
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <alpine.LFD.2.00.1102191540290.2701@localhost6.localdomain6>
> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Saturday, February 19, 2011 10:12 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
>
> On Sat, 19 Feb 2011, KY Srinivasan wrote:
> > > From: Thomas Gleixner [mailto:tglx@linutronix.de]
> > > Please do not use probe_irq_on for dynamic irq allocation. Highjacking
> > > the lower PIC irqs is really not a good idea. Depending on when this
> > > runs, you might grab an irq required by a driver which gets loaded
> > > later.
> > >
> > > Could you please explain what you're trying to do here ?
> >
> > The IRQ being allocated is for the VMBUS driver for Linux guests running on
> > a Windows virtualization platform (Hyper-V hypervisor).
> > The hypervisor is capable of notifying events on the VMBUS via
> > a guest specified interrupt line. Prior to this patch,
> > the code was statically selecting an interrupt line for
> > use by VMBUS. One of the long standing review comments
> > on that code was to make this irq allocation dynamic and that
> > is what this patch does. For the Linux guest running as a VM
> > on Hyper-V, the concern you raise is not an issue.
>
> That patch does a whole lot of useless crap.
>
> When grabbing some random irq from the PIC is not an issue, then
> what's the point of this probing, retry loop and the comments about
> racing ? What races here? That does not make sense at all.
Like most virtualization platforms, Hyper-V also emulates the full PC
platform. So, it is possible that the driver of some other emulated
devices might register for the IRQ line we might have selected. That
is the race this code addresses. For performance reasons, we want
both storage and network traffic to go over the PV drivers.
>
> I don't know why the previous reviewer wanted to have that
> dynamic. That just does not make sense to me.
Prior to this patch, we had a hard coded interrupt line for use by
this driver. If that line was already in use, the load of this driver
would fail. This would be a fatal issue especially for distributions
that have embedded these PV drivers as part of their installation
media. This patch deals with such collisions in a more graceful way -
we would not bail until we have scanned all low interrupt lines.
>
> Btw, that whole interrupt handler with two tasklets, one of them
> scheduling work is just screaming threaded interrupt handler.
We are in the process of cleaning up these drivers; I am looking at
some of these and other issues.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: Thomas Gleixner @ 2011-02-19 15:12 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
virtualization@lists.osdl.org, devel@linuxdriverproject.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E048011738@TK5EX14MBXC124.redmond.corp.microsoft.com>
On Sat, 19 Feb 2011, KY Srinivasan wrote:
> > From: Thomas Gleixner [mailto:tglx@linutronix.de]
> > Please do not use probe_irq_on for dynamic irq allocation. Highjacking
> > the lower PIC irqs is really not a good idea. Depending on when this
> > runs, you might grab an irq required by a driver which gets loaded
> > later.
> >
> > Could you please explain what you're trying to do here ?
>
> The IRQ being allocated is for the VMBUS driver for Linux guests running on
> a Windows virtualization platform (Hyper-V hypervisor).
> The hypervisor is capable of notifying events on the VMBUS via
> a guest specified interrupt line. Prior to this patch,
> the code was statically selecting an interrupt line for
> use by VMBUS. One of the long standing review comments
> on that code was to make this irq allocation dynamic and that
> is what this patch does. For the Linux guest running as a VM
> on Hyper-V, the concern you raise is not an issue.
That patch does a whole lot of useless crap.
When grabbing some random irq from the PIC is not an issue, then
what's the point of this probing, retry loop and the comments about
racing ? What races here? That does not make sense at all.
I don't know why the previous reviewer wanted to have that
dynamic. That just does not make sense to me.
Btw, that whole interrupt handler with two tasklets, one of them
scheduling work is just screaming threaded interrupt handler.
Thanks,
tglx
^ permalink raw reply
* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: KY Srinivasan @ 2011-02-19 14:34 UTC (permalink / raw)
To: Thomas Gleixner
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <alpine.LFD.2.00.1102191046220.2701@localhost6.localdomain6>
> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Saturday, February 19, 2011 5:23 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
>
> On Tue, 15 Feb 2011, K. Y. Srinivasan wrote:
> > -/* FIXME! We need to do this dynamically for PIC and APIC system */
> > -#define VMBUS_IRQ 0x5
> > -#define VMBUS_IRQ_VECTOR IRQ5_VECTOR
> > +static int vmbus_irq;
> >
> > /* Main vmbus driver data structure */
> > struct vmbus_driver_context {
> > @@ -57,6 +55,27 @@ struct vmbus_driver_context {
> > struct vm_device device_ctx;
> > };
> >
> > +/*
> > + * Find an un-used IRQ that the VMBUS can use. If none is available;
> > + * return -EBUSY.
> > + */
> > +static int vmbus_get_irq(void)
> > +{
> > + unsigned int avail_irq_mask;
> > + int irq = -EBUSY;
> > +
> > + /*
> > + * Pick the first unused interrupt. HyperV can
> > + * interrupt us on any interrupt line we specify.
> > + */
> > +
> > + avail_irq_mask = probe_irq_on();
> > + if (avail_irq_mask != 0)
> > + irq = ffs(avail_irq_mask);
> > + probe_irq_off(avail_irq_mask);
> > + return irq;
>
>
> Please do not use probe_irq_on for dynamic irq allocation. Highjacking
> the lower PIC irqs is really not a good idea. Depending on when this
> runs, you might grab an irq required by a driver which gets loaded
> later.
>
> Could you please explain what you're trying to do here ?
The IRQ being allocated is for the VMBUS driver for Linux guests running on
a Windows virtualization platform (Hyper-V hypervisor).
The hypervisor is capable of notifying events on the VMBUS via
a guest specified interrupt line. Prior to this patch,
the code was statically selecting an interrupt line for
use by VMBUS. One of the long standing review comments
on that code was to make this irq allocation dynamic and that
is what this patch does. For the Linux guest running as a VM
on Hyper-V, the concern you raise is not an issue.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: Thomas Gleixner @ 2011-02-19 10:23 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1297799735-25917-1-git-send-email-kys@microsoft.com>
On Tue, 15 Feb 2011, K. Y. Srinivasan wrote:
> -/* FIXME! We need to do this dynamically for PIC and APIC system */
> -#define VMBUS_IRQ 0x5
> -#define VMBUS_IRQ_VECTOR IRQ5_VECTOR
> +static int vmbus_irq;
>
> /* Main vmbus driver data structure */
> struct vmbus_driver_context {
> @@ -57,6 +55,27 @@ struct vmbus_driver_context {
> struct vm_device device_ctx;
> };
>
> +/*
> + * Find an un-used IRQ that the VMBUS can use. If none is available;
> + * return -EBUSY.
> + */
> +static int vmbus_get_irq(void)
> +{
> + unsigned int avail_irq_mask;
> + int irq = -EBUSY;
> +
> + /*
> + * Pick the first unused interrupt. HyperV can
> + * interrupt us on any interrupt line we specify.
> + */
> +
> + avail_irq_mask = probe_irq_on();
> + if (avail_irq_mask != 0)
> + irq = ffs(avail_irq_mask);
> + probe_irq_off(avail_irq_mask);
> + return irq;
Please do not use probe_irq_on for dynamic irq allocation. Highjacking
the lower PIC irqs is really not a good idea. Depending on when this
runs, you might grab an irq required by a driver which gets loaded
later.
Could you please explain what you're trying to do here ?
Thanks,
tglx
^ permalink raw reply
* [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: K. Y. Srinivasan @ 2011-02-19 1:26 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization
Cc: K. Y. Srinivasan, Haiyang Zhang, Hank Janssen
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 72 +++++++++++++++++++++++++++++++---------
1 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 459c707..441ce85 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -36,9 +36,7 @@
#include "vmbus_private.h"
-/* FIXME! We need to do this dynamically for PIC and APIC system */
-#define VMBUS_IRQ 0x5
-#define VMBUS_IRQ_VECTOR IRQ5_VECTOR
+static int vmbus_irq;
/* Main vmbus driver data structure */
struct vmbus_driver_context {
@@ -57,6 +55,27 @@ struct vmbus_driver_context {
struct vm_device device_ctx;
};
+/*
+ * Find an un-used IRQ that the VMBUS can use. If none is available;
+ * return -EBUSY.
+ */
+static int vmbus_get_irq(void)
+{
+ unsigned int avail_irq_mask;
+ int irq = -EBUSY;
+
+ /*
+ * Pick the first unused interrupt. HyperV can
+ * interrupt us on any interrupt line we specify.
+ */
+
+ avail_irq_mask = probe_irq_on();
+ if (avail_irq_mask != 0)
+ irq = ffs(avail_irq_mask);
+ probe_irq_off(avail_irq_mask);
+ return irq;
+}
+
static int vmbus_match(struct device *device, struct device_driver *driver);
static int vmbus_probe(struct device *device);
static int vmbus_remove(struct device *device);
@@ -80,7 +99,6 @@ EXPORT_SYMBOL(vmbus_loglevel);
/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
/* (((VMBUS | VMBUS_DRV)<<16) | DEBUG_LVL_ENTEREXIT); */
-static int vmbus_irq = VMBUS_IRQ;
/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
static struct device_attribute vmbus_device_attrs[] = {
@@ -467,6 +485,7 @@ static int vmbus_bus_init(void)
struct vm_device *dev_ctx = &vmbus_drv.device_ctx;
int ret;
unsigned int vector;
+ bool irq_assigned = false;
DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
HV_DRV_VERSION);
@@ -517,20 +536,42 @@ static int vmbus_bus_init(void)
goto cleanup;
}
- /* Get the interrupt resource */
- ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
- driver->name, NULL);
-
- if (ret != 0) {
- DPRINT_ERR(VMBUS_DRV, "ERROR - Unable to request IRQ %d",
- vmbus_irq);
+ /*
+ * Get an unused interrupt line and register our handler.
+ * Since there is a window between getting an unused
+ * interrupt line and registering our handler, we close
+ * this window by repeating the sequence if we raced.
+ * This loop will terminate under the following conditions:
+ *
+ * 1) If we succeed in registering our handler OR
+ * 2) If there are no free interrupt lines for our use OR
+ * 3) request_irq fails for reasons other than an irq collision
+ */
+ while (!irq_assigned) {
+ vmbus_irq = vmbus_get_irq();
- bus_unregister(&vmbus_drv_ctx->bus);
+ if (vmbus_irq < 0) {
+ bus_unregister(&vmbus_drv_ctx->bus);
+ ret = -EBUSY;
+ goto cleanup;
+ }
- ret = -1;
- goto cleanup;
+ ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
+ driver->name, NULL);
+ switch (ret) {
+ case -EBUSY:
+ /* We raced and lost; try again.*/
+ continue;
+ case 0:
+ irq_assigned = true;
+ break;
+ default:
+ /* Failed to request_irq; cleanup */
+ goto cleanup;
+ }
}
- vector = VMBUS_IRQ_VECTOR;
+
+ vector = IRQ0_VECTOR + vmbus_irq;
DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq, vector);
@@ -1117,7 +1158,6 @@ MODULE_DEVICE_TABLE(pci, microsoft_hv_pci_table);
MODULE_LICENSE("GPL");
MODULE_VERSION(HV_DRV_VERSION);
-module_param(vmbus_irq, int, S_IRUGO);
module_param(vmbus_loglevel, int, S_IRUGO);
module_init(vmbus_init);
--
1.5.5.6
^ permalink raw reply related
* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: KY Srinivasan @ 2011-02-19 1:19 UTC (permalink / raw)
To: Greg KH
Cc: virtualization@lists.osdl.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org
In-Reply-To: <20110219010230.GA2402@suse.de>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Friday, February 18, 2011 8:03 PM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org
> Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
>
> On Sat, Feb 19, 2011 at 12:56:16AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Friday, February 18, 2011 5:29 PM
> > > To: KY Srinivasan
> > > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > virtualization@lists.osdl.org
> > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > >
> > > On Fri, Feb 18, 2011 at 10:16:05PM +0000, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@suse.de]
> > > > > Sent: Friday, February 18, 2011 5:07 PM
> > > > > To: KY Srinivasan
> > > > > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > > > virtualization@lists.osdl.org
> > > > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > > > >
> > > > > On Fri, Feb 18, 2011 at 10:00:04PM +0000, KY Srinivasan wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Greg KH [mailto:greg@kroah.com]
> > > > > > > Sent: Friday, February 18, 2011 4:14 PM
> > > > > > > To: KY Srinivasan
> > > > > > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > > > > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > > > > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > > > > > >
> > > > > > > On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> > > > > > > >
> > > > > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > > > > >
> > > > > > > You didn't run this through checkpatch.pl.
> > > > > > >
> > > > > > > Please do so and fix the warning it gives you.
> > > > > > Greg, I did run the checkpatch script against this patch and the only
> > > > > > complaint I got was with regards to the IRQF_SAMPLE_RANDOM flag
> that I
> > > > > > pass. As a virtual machine, this is the only external event that the
> > > > > > VM is going to see and so I chose to keep this flag. Is there
> > > > > > something that would replace this flag; looking at the Xen drivers
> > > > > > they do pass this flag.
> > > > >
> > > > > But that flag is going away, right? And this really can't be a valid
> > > > > source of entropy as the HV channel is pretty predictable.
> > > >
> > > > Is it going away? What would replace this. Is all interrupt sources considered
> > > > predictable?
> > >
> > > Did you read the file that the checkpatch script told you to about this
> > > entry?
> >
> > It is only after reading the document, I decided to keep that flag. Please
> > note this is not a question of some interrupt sources not being
> > a good source of entropy; for this VM this is the only source of interrupts.
> > The document on this flag talked about how people were incorrectly
> > marking their interrupt as an entropy source; in this case there is not much of a
> > choice.
> >
> > >
> > > > This is the only unpredictable thing happening in the VM and that is the
> reason
> > > > I chose to keep the flag.
> > >
> > > If you remove it, do we loose all entropy for the VM?
> > >
> > > > > If you are only using this because Xen does/did it, that's not a valid
> > > > > excuse :)
> > > > Surely, you are joking.
> > >
> > > Not at all.
> >
> > To set the record straight here, this flag is in the existing code.
> > After I ran checkpatch, I toyed with the idea of getting rid of this. Then I
> > decided to keep it for all the reasons I mentioned earlier.
> >
> > >
> > > > In any event I am sending you a new patch with that flag removed.
> > >
> > > Have you tested to see if you now loose all entropy, and it causes
> > > problems or not?
> >
> > I am glad you asked me to test it. When I remove this flag, the entropy goes
> down
> > significantly and this is not surprising. Looking at
> > /proc/sys/kernel/entropy/entropy_avail, with the original patch after a couple
> > of compiles the number would be in thousands. With that flag removed,
> > I have the VM up for about an hour, even after a couple of compiles,
> > the entropy number is yet to crack 200.
> >
> > Let me know how you want to proceed here.
>
> Ok, my fault, let's keep the original flag. Care to resend the patch
> you had originally sent?
Thanks Greg; I just sent it. I will deal with irq balancing issue in a separate patch.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: Greg KH @ 2011-02-19 1:02 UTC (permalink / raw)
To: KY Srinivasan
Cc: Greg KH, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E048011693@TK5EX14MBXC124.redmond.corp.microsoft.com>
On Sat, Feb 19, 2011 at 12:56:16AM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Friday, February 18, 2011 5:29 PM
> > To: KY Srinivasan
> > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > virtualization@lists.osdl.org
> > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> >
> > On Fri, Feb 18, 2011 at 10:16:05PM +0000, KY Srinivasan wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@suse.de]
> > > > Sent: Friday, February 18, 2011 5:07 PM
> > > > To: KY Srinivasan
> > > > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > > virtualization@lists.osdl.org
> > > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > > >
> > > > On Fri, Feb 18, 2011 at 10:00:04PM +0000, KY Srinivasan wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg KH [mailto:greg@kroah.com]
> > > > > > Sent: Friday, February 18, 2011 4:14 PM
> > > > > > To: KY Srinivasan
> > > > > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > > > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > > > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > > > > >
> > > > > > On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> > > > > > >
> > > > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > > > >
> > > > > > You didn't run this through checkpatch.pl.
> > > > > >
> > > > > > Please do so and fix the warning it gives you.
> > > > > Greg, I did run the checkpatch script against this patch and the only
> > > > > complaint I got was with regards to the IRQF_SAMPLE_RANDOM flag that I
> > > > > pass. As a virtual machine, this is the only external event that the
> > > > > VM is going to see and so I chose to keep this flag. Is there
> > > > > something that would replace this flag; looking at the Xen drivers
> > > > > they do pass this flag.
> > > >
> > > > But that flag is going away, right? And this really can't be a valid
> > > > source of entropy as the HV channel is pretty predictable.
> > >
> > > Is it going away? What would replace this. Is all interrupt sources considered
> > > predictable?
> >
> > Did you read the file that the checkpatch script told you to about this
> > entry?
>
> It is only after reading the document, I decided to keep that flag. Please
> note this is not a question of some interrupt sources not being
> a good source of entropy; for this VM this is the only source of interrupts.
> The document on this flag talked about how people were incorrectly
> marking their interrupt as an entropy source; in this case there is not much of a
> choice.
>
> >
> > > This is the only unpredictable thing happening in the VM and that is the reason
> > > I chose to keep the flag.
> >
> > If you remove it, do we loose all entropy for the VM?
> >
> > > > If you are only using this because Xen does/did it, that's not a valid
> > > > excuse :)
> > > Surely, you are joking.
> >
> > Not at all.
>
> To set the record straight here, this flag is in the existing code.
> After I ran checkpatch, I toyed with the idea of getting rid of this. Then I
> decided to keep it for all the reasons I mentioned earlier.
>
> >
> > > In any event I am sending you a new patch with that flag removed.
> >
> > Have you tested to see if you now loose all entropy, and it causes
> > problems or not?
>
> I am glad you asked me to test it. When I remove this flag, the entropy goes down
> significantly and this is not surprising. Looking at
> /proc/sys/kernel/entropy/entropy_avail, with the original patch after a couple
> of compiles the number would be in thousands. With that flag removed,
> I have the VM up for about an hour, even after a couple of compiles,
> the entropy number is yet to crack 200.
>
> Let me know how you want to proceed here.
Ok, my fault, let's keep the original flag. Care to resend the patch
you had originally sent?
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: KY Srinivasan @ 2011-02-19 0:56 UTC (permalink / raw)
To: Greg KH
Cc: Greg KH, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110218222916.GA14244@suse.de>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Friday, February 18, 2011 5:29 PM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org
> Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
>
> On Fri, Feb 18, 2011 at 10:16:05PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Friday, February 18, 2011 5:07 PM
> > > To: KY Srinivasan
> > > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > virtualization@lists.osdl.org
> > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > >
> > > On Fri, Feb 18, 2011 at 10:00:04PM +0000, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:greg@kroah.com]
> > > > > Sent: Friday, February 18, 2011 4:14 PM
> > > > > To: KY Srinivasan
> > > > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > > > >
> > > > > On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> > > > > >
> > > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > > >
> > > > > You didn't run this through checkpatch.pl.
> > > > >
> > > > > Please do so and fix the warning it gives you.
> > > > Greg, I did run the checkpatch script against this patch and the only
> > > > complaint I got was with regards to the IRQF_SAMPLE_RANDOM flag that I
> > > > pass. As a virtual machine, this is the only external event that the
> > > > VM is going to see and so I chose to keep this flag. Is there
> > > > something that would replace this flag; looking at the Xen drivers
> > > > they do pass this flag.
> > >
> > > But that flag is going away, right? And this really can't be a valid
> > > source of entropy as the HV channel is pretty predictable.
> >
> > Is it going away? What would replace this. Is all interrupt sources considered
> > predictable?
>
> Did you read the file that the checkpatch script told you to about this
> entry?
It is only after reading the document, I decided to keep that flag. Please
note this is not a question of some interrupt sources not being
a good source of entropy; for this VM this is the only source of interrupts.
The document on this flag talked about how people were incorrectly
marking their interrupt as an entropy source; in this case there is not much of a
choice.
>
> > This is the only unpredictable thing happening in the VM and that is the reason
> > I chose to keep the flag.
>
> If you remove it, do we loose all entropy for the VM?
>
> > > If you are only using this because Xen does/did it, that's not a valid
> > > excuse :)
> > Surely, you are joking.
>
> Not at all.
To set the record straight here, this flag is in the existing code.
After I ran checkpatch, I toyed with the idea of getting rid of this. Then I
decided to keep it for all the reasons I mentioned earlier.
>
> > In any event I am sending you a new patch with that flag removed.
>
> Have you tested to see if you now loose all entropy, and it causes
> problems or not?
I am glad you asked me to test it. When I remove this flag, the entropy goes down
significantly and this is not surprising. Looking at
/proc/sys/kernel/entropy/entropy_avail, with the original patch after a couple
of compiles the number would be in thousands. With that flag removed,
I have the VM up for about an hour, even after a couple of compiles,
the entropy number is yet to crack 200.
Let me know how you want to proceed here.
Regards,
K. Y
^ permalink raw reply
* [PATCH 2/2] staging: hv: Fix the WARN_ON condition in free_net_device()
From: Haiyang Zhang @ 2011-02-18 23:05 UTC (permalink / raw)
To: haiyangz, hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
vir
In-Reply-To: <1298070353-23534-1-git-send-email-haiyangz@microsoft.com>
In a previous commit, 7a09876d, ASSERT was changed to WARN_ON, but
the condition wasn't updated. This patch fixed this error.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/netvsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index f21a59d..20b1597 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -95,7 +95,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
static void free_net_device(struct netvsc_device *device)
{
- WARN_ON(atomic_read(&device->refcnt) == 0);
+ WARN_ON(atomic_read(&device->refcnt) != 0);
device->dev->ext = NULL;
kfree(device);
}
--
1.6.3.2
^ permalink raw reply related
* [PATCH 1/2] staging: hv: Remove unnecessary ASSERTs in netvsc_initialize()
From: Haiyang Zhang @ 2011-02-18 23:05 UTC (permalink / raw)
To: haiyangz, hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
vir
These fields have been assigned in netvsc_drv_init() before calling
netvsc_initialize(), so there is no need to check them.
The ASSERTs were already commented out, and this patch removes
them.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/netvsc.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 8c6d4ae..f21a59d 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -188,11 +188,6 @@ int netvsc_initialize(struct hv_driver *drv)
drv->name = driver_name;
memcpy(&drv->dev_type, &netvsc_device_type, sizeof(struct hv_guid));
- /* Make sure it is set by the caller */
- /* FIXME: These probably should still be tested in some way */
- /* ASSERT(driver->OnReceiveCallback); */
- /* ASSERT(driver->OnLinkStatusChanged); */
-
/* Setup the dispatch table */
driver->base.dev_add = netvsc_device_add;
driver->base.dev_rm = netvsc_device_remove;
--
1.6.3.2
^ permalink raw reply related
* [PATCH]: Staging: hv: Dynamically allocate irqs
From: K. Y. Srinivasan @ 2011-02-18 22:32 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization
Cc: K. Y. Srinivasan, Haiyang Zhang, Hank Janssen
Dynamically allocate the irq for vmbus.
Hyper-V delivers this interrupt on a
specific CPU and so mark the interrupt so
that it cannot be migrated to other CPUs.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 72 +++++++++++++++++++++++++++++++---------
1 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 459c707..9806bac 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -36,9 +36,7 @@
#include "vmbus_private.h"
-/* FIXME! We need to do this dynamically for PIC and APIC system */
-#define VMBUS_IRQ 0x5
-#define VMBUS_IRQ_VECTOR IRQ5_VECTOR
+static int vmbus_irq;
/* Main vmbus driver data structure */
struct vmbus_driver_context {
@@ -57,6 +55,27 @@ struct vmbus_driver_context {
struct vm_device device_ctx;
};
+/*
+ * Find an un-used IRQ that the VMBUS can use. If none is available;
+ * return -EBUSY.
+ */
+static int vmbus_get_irq(void)
+{
+ unsigned int avail_irq_mask;
+ int irq = -EBUSY;
+
+ /*
+ * Pick the first unused interrupt. HyperV can
+ * interrupt us on any interrupt line we specify.
+ */
+
+ avail_irq_mask = probe_irq_on();
+ if (avail_irq_mask != 0)
+ irq = ffs(avail_irq_mask);
+ probe_irq_off(avail_irq_mask);
+ return irq;
+}
+
static int vmbus_match(struct device *device, struct device_driver *driver);
static int vmbus_probe(struct device *device);
static int vmbus_remove(struct device *device);
@@ -80,7 +99,6 @@ EXPORT_SYMBOL(vmbus_loglevel);
/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
/* (((VMBUS | VMBUS_DRV)<<16) | DEBUG_LVL_ENTEREXIT); */
-static int vmbus_irq = VMBUS_IRQ;
/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
static struct device_attribute vmbus_device_attrs[] = {
@@ -467,6 +485,7 @@ static int vmbus_bus_init(void)
struct vm_device *dev_ctx = &vmbus_drv.device_ctx;
int ret;
unsigned int vector;
+ bool irq_assigned = false;
DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
HV_DRV_VERSION);
@@ -517,20 +536,42 @@ static int vmbus_bus_init(void)
goto cleanup;
}
- /* Get the interrupt resource */
- ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
- driver->name, NULL);
-
- if (ret != 0) {
- DPRINT_ERR(VMBUS_DRV, "ERROR - Unable to request IRQ %d",
- vmbus_irq);
+ /*
+ * Get an unused interrupt line and register our handler.
+ * Since there is a window between getting an unused
+ * interrupt line and registering our handler, we close
+ * this window by repeating the sequence if we raced.
+ * This loop will terminate under the following conditions:
+ *
+ * 1) If we succeed in registering our handler OR
+ * 2) If there are no free interrupt lines for our use OR
+ * 3) request_irq fails for reasons other than an irq collision
+ */
+ while (!irq_assigned) {
+ vmbus_irq = vmbus_get_irq();
- bus_unregister(&vmbus_drv_ctx->bus);
+ if (vmbus_irq < 0) {
+ bus_unregister(&vmbus_drv_ctx->bus);
+ ret = -EBUSY;
+ goto cleanup;
+ }
- ret = -1;
- goto cleanup;
+ ret = request_irq(vmbus_irq, vmbus_isr, IRQF_NOBALANCING,
+ driver->name, NULL);
+ switch (ret) {
+ case -EBUSY:
+ /* We raced and lost; try again.*/
+ continue;
+ case 0:
+ irq_assigned = true;
+ break;
+ default:
+ /* Failed to request_irq; cleanup */
+ goto cleanup;
+ }
}
- vector = VMBUS_IRQ_VECTOR;
+
+ vector = IRQ0_VECTOR + vmbus_irq;
DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq, vector);
@@ -1117,7 +1158,6 @@ MODULE_DEVICE_TABLE(pci, microsoft_hv_pci_table);
MODULE_LICENSE("GPL");
MODULE_VERSION(HV_DRV_VERSION);
-module_param(vmbus_irq, int, S_IRUGO);
module_param(vmbus_loglevel, int, S_IRUGO);
module_init(vmbus_init);
--
1.5.5.6
^ permalink raw reply related
* Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: Greg KH @ 2011-02-18 22:29 UTC (permalink / raw)
To: KY Srinivasan
Cc: Greg KH, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E04801162F@TK5EX14MBXC124.redmond.corp.microsoft.com>
On Fri, Feb 18, 2011 at 10:16:05PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Friday, February 18, 2011 5:07 PM
> > To: KY Srinivasan
> > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > virtualization@lists.osdl.org
> > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> >
> > On Fri, Feb 18, 2011 at 10:00:04PM +0000, KY Srinivasan wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:greg@kroah.com]
> > > > Sent: Friday, February 18, 2011 4:14 PM
> > > > To: KY Srinivasan
> > > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > > >
> > > > On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> > > > >
> > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > >
> > > > You didn't run this through checkpatch.pl.
> > > >
> > > > Please do so and fix the warning it gives you.
> > > Greg, I did run the checkpatch script against this patch and the only
> > > complaint I got was with regards to the IRQF_SAMPLE_RANDOM flag that I
> > > pass. As a virtual machine, this is the only external event that the
> > > VM is going to see and so I chose to keep this flag. Is there
> > > something that would replace this flag; looking at the Xen drivers
> > > they do pass this flag.
> >
> > But that flag is going away, right? And this really can't be a valid
> > source of entropy as the HV channel is pretty predictable.
>
> Is it going away? What would replace this. Is all interrupt sources considered
> predictable?
Did you read the file that the checkpatch script told you to about this
entry?
> This is the only unpredictable thing happening in the VM and that is the reason
> I chose to keep the flag.
If you remove it, do we loose all entropy for the VM?
> > If you are only using this because Xen does/did it, that's not a valid
> > excuse :)
> Surely, you are joking.
Not at all.
> In any event I am sending you a new patch with that flag removed.
Have you tested to see if you now loose all entropy, and it causes
problems or not?
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: KY Srinivasan @ 2011-02-18 22:16 UTC (permalink / raw)
To: Greg KH
Cc: Greg KH, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110218220724.GA28986@suse.de>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Friday, February 18, 2011 5:07 PM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org
> Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
>
> On Fri, Feb 18, 2011 at 10:00:04PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Friday, February 18, 2011 4:14 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> > >
> > > On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > >
> > > You didn't run this through checkpatch.pl.
> > >
> > > Please do so and fix the warning it gives you.
> > Greg, I did run the checkpatch script against this patch and the only
> > complaint I got was with regards to the IRQF_SAMPLE_RANDOM flag that I
> > pass. As a virtual machine, this is the only external event that the
> > VM is going to see and so I chose to keep this flag. Is there
> > something that would replace this flag; looking at the Xen drivers
> > they do pass this flag.
>
> But that flag is going away, right? And this really can't be a valid
> source of entropy as the HV channel is pretty predictable.
Is it going away? What would replace this. Is all interrupt sources considered
predictable? This is the only unpredictable thing happening in the VM and that is the reason
I chose to keep the flag.
>
> If you are only using this because Xen does/did it, that's not a valid
> excuse :)
Surely, you are joking. In any event I am sending you a new patch with that flag removed.
Regards,
K. Y
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: Greg KH @ 2011-02-18 22:07 UTC (permalink / raw)
To: KY Srinivasan
Cc: Greg KH, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E048011604@TK5EX14MBXC124.redmond.corp.microsoft.com>
On Fri, Feb 18, 2011 at 10:00:04PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Friday, February 18, 2011 4:14 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
> >
> > On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> >
> > You didn't run this through checkpatch.pl.
> >
> > Please do so and fix the warning it gives you.
> Greg, I did run the checkpatch script against this patch and the only
> complaint I got was with regards to the IRQF_SAMPLE_RANDOM flag that I
> pass. As a virtual machine, this is the only external event that the
> VM is going to see and so I chose to keep this flag. Is there
> something that would replace this flag; looking at the Xen drivers
> they do pass this flag.
But that flag is going away, right? And this really can't be a valid
source of entropy as the HV channel is pretty predictable.
If you are only using this because Xen does/did it, that's not a valid
excuse :)
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: KY Srinivasan @ 2011-02-18 22:00 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110218211423.GH796@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Friday, February 18, 2011 4:14 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
>
> On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
>
> You didn't run this through checkpatch.pl.
>
> Please do so and fix the warning it gives you.
Greg, I did run the checkpatch script against this patch and the only complaint I got was with
regards to the IRQF_SAMPLE_RANDOM flag that I pass. As a virtual machine, this is the only
external event that the VM is going to see and so I chose to keep this flag. Is there something that would
replace this flag; looking at the Xen drivers they do pass this flag.
Regards,
K. Y
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: Greg KH @ 2011-02-18 21:14 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, virtualization
In-Reply-To: <1297799735-25917-1-git-send-email-kys@microsoft.com>
On Tue, Feb 15, 2011 at 11:55:35AM -0800, K. Y. Srinivasan wrote:
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
You didn't run this through checkpatch.pl.
Please do so and fix the warning it gives you.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] Staging: hv: Fixed FIXME comments by using list_for_each_entry
From: Hank Janssen @ 2011-02-18 20:39 UTC (permalink / raw)
To: hjanssen, haiyangz, gregkh, linux-kernel, devel, virtualization
Cc: K.Y. Srinivasan
Fixed FIXME requests in channel_mgmt.c by using
list_for_each_entry.
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: K.Y. Srinivasan <kys@microsoft.com>
---
drivers/staging/hv/channel_mgmt.c | 24 ++++++++----------------
1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c
index 325e0bc..0781c0e 100644
--- a/drivers/staging/hv/channel_mgmt.c
+++ b/drivers/staging/hv/channel_mgmt.c
@@ -567,7 +567,6 @@ static void vmbus_onoffers_delivered(
static void vmbus_onopen_result(struct vmbus_channel_message_header *hdr)
{
struct vmbus_channel_open_result *result;
- struct list_head *curr;
struct vmbus_channel_msginfo *msginfo;
struct vmbus_channel_message_header *requestheader;
struct vmbus_channel_open_channel *openmsg;
@@ -581,9 +580,8 @@ static void vmbus_onopen_result(struct vmbus_channel_message_header *hdr)
*/
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
- list_for_each(curr, &vmbus_connection.chn_msg_list) {
-/* FIXME: this should probably use list_entry() instead */
- msginfo = (struct vmbus_channel_msginfo *)curr;
+ list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
+ msglistentry) {
requestheader =
(struct vmbus_channel_message_header *)msginfo->msg;
@@ -614,7 +612,6 @@ static void vmbus_onopen_result(struct vmbus_channel_message_header *hdr)
static void vmbus_ongpadl_created(struct vmbus_channel_message_header *hdr)
{
struct vmbus_channel_gpadl_created *gpadlcreated;
- struct list_head *curr;
struct vmbus_channel_msginfo *msginfo;
struct vmbus_channel_message_header *requestheader;
struct vmbus_channel_gpadl_header *gpadlheader;
@@ -630,9 +627,8 @@ static void vmbus_ongpadl_created(struct vmbus_channel_message_header *hdr)
*/
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
- list_for_each(curr, &vmbus_connection.chn_msg_list) {
-/* FIXME: this should probably use list_entry() instead */
- msginfo = (struct vmbus_channel_msginfo *)curr;
+ list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
+ msglistentry) {
requestheader =
(struct vmbus_channel_message_header *)msginfo->msg;
@@ -666,7 +662,6 @@ static void vmbus_ongpadl_torndown(
struct vmbus_channel_message_header *hdr)
{
struct vmbus_channel_gpadl_torndown *gpadl_torndown;
- struct list_head *curr;
struct vmbus_channel_msginfo *msginfo;
struct vmbus_channel_message_header *requestheader;
struct vmbus_channel_gpadl_teardown *gpadl_teardown;
@@ -679,9 +674,8 @@ static void vmbus_ongpadl_torndown(
*/
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
- list_for_each(curr, &vmbus_connection.chn_msg_list) {
-/* FIXME: this should probably use list_entry() instead */
- msginfo = (struct vmbus_channel_msginfo *)curr;
+ list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
+ msglistentry) {
requestheader =
(struct vmbus_channel_message_header *)msginfo->msg;
@@ -712,7 +706,6 @@ static void vmbus_ongpadl_torndown(
static void vmbus_onversion_response(
struct vmbus_channel_message_header *hdr)
{
- struct list_head *curr;
struct vmbus_channel_msginfo *msginfo;
struct vmbus_channel_message_header *requestheader;
struct vmbus_channel_initiate_contact *initiate;
@@ -722,9 +715,8 @@ static void vmbus_onversion_response(
version_response = (struct vmbus_channel_version_response *)hdr;
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
- list_for_each(curr, &vmbus_connection.chn_msg_list) {
-/* FIXME: this should probably use list_entry() instead */
- msginfo = (struct vmbus_channel_msginfo *)curr;
+ list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
+ msglistentry) {
requestheader =
(struct vmbus_channel_message_header *)msginfo->msg;
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Srivatsa Vaddagiri @ 2011-02-18 17:03 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin, Peter Zijlstra,
vsrivatsa, Linux Kernel Mailing List, Jan Beulich, Eric Dumazet,
Jeremy Fitzhardinge, suzuki, Avi Kivity, H. Peter Anvin,
Américo Wang, Linux Virtualization
In-Reply-To: <4D3DF5A5.7070301@goop.org>
> On Mon, Jan 24, 2011 at 01:56:53PM -0800, Jeremy Fitzhardinge wrote:
For some reason, I seem to be missing emails from your id/domain and hence had
missed this completely!
> > * bits. However, we need to be careful about this because someone
> > * may just be entering as we leave, and enter the slowpath.
> > */
> > -void __ticket_unlock_release_slowpath(struct arch_spinlock *lock)
> > +void __ticket_unlock_slowpath(struct arch_spinlock *lock)
> > {
> > struct arch_spinlock old, new;
> >
> > BUILD_BUG_ON(((__ticket_t)NR_CPUS) != NR_CPUS);
> >
> > old = ACCESS_ONCE(*lock);
> > -
> > new = old;
> > - new.tickets.head += TICKET_LOCK_INC;
> >
> > /* Clear the slowpath flag */
> > new.tickets.tail &= ~TICKET_SLOWPATH_FLAG;
> > + if (new.tickets.head == new.tickets.tail)
> > + cmpxchg(&lock->head_tail, old.head_tail, new.head_tail);
> >
> > - /*
> > - * If there's currently people waiting or someone snuck in
> > - * since we read the lock above, then do a normal unlock and
> > - * kick. If we managed to unlock with no queued waiters, then
> > - * we can clear the slowpath flag.
> > - */
> > - if (new.tickets.head != new.tickets.tail ||
> > - cmpxchg(&lock->head_tail,
> > - old.head_tail, new.head_tail) != old.head_tail) {
> > - /* still people waiting */
> > - __ticket_unlock_release(lock);
> > - }
> > -
> > + /* Wake up an appropriate waiter */
> > __ticket_unlock_kick(lock, new.tickets.head);
>
> Does the __ticket_unlock_kick need to be unconditional?
I recall having tried optimizing it to be conditional, something along these
lines:
if (new.ticket.head == new.tickets.tail) {
cmpxchg();
} else {
__ticket_unlock_kick(lock, new.tickets.head);
}
but it didn't work for some reason. I left the call unconditional as was the
case previously based on that experiment.
- vatsa
^ permalink raw reply
* CfP 6th Workshop on Virtualization in High-Performance Cloud Computing (VHPC'11)
From: VHPC2011 @ 2011-02-17 12:11 UTC (permalink / raw)
To: virtualization
Apologies if you received multiple copies of this message.
=================================================================
CALL FOR PAPERS
6th Workshop on
Virtualization in High-Performance Cloud Computing
VHPC'11
as part of Euro-Par 2011, Bordeaux, France
=================================================================
Date: August 30, 2011
Euro-Par 2011: http://europar2011.bordeaux.inria.fr/
Workshop URL: http://vhpc.org
SUBMISSION DEADLINE:
Abstracts: May 2, 2011
Full Paper: June 13, 2011
Scope:
Virtualization has become a common abstraction layer in modern data
centers, enabling resource owners to manage complex infrastructure
independently of their applications. Conjointly virtualization is
becoming a driving technology for a manifold of industry grade IT
services. The cloud concept includes the notion of a separation
between resource owners and users, adding services such as hosted
application frameworks and queuing. Utilizing the same infrastructure,
clouds carry significant potential for use in high-performance
scientific computing. The ability of clouds to provide for
requests and releases of vast computing resource dynamically and
close to the marginal cost of providing the services is unprecedented
in the history of scientific and commercial computing.
Distributed computing concepts that leverage federated resource access
are popular within the grid community, but have not seen previously
desired deployed levels so far. Also, many of the scientific
datacenters have not adopted virtualization or cloud concepts yet.
This workshop aims to bring together industrial providers with the
scientific community in order to foster discussion, collaboration and
mutual exchange of knowledge and experience.
The workshop will be one day in length, composed of 20 min paper
presentations, each followed by 10 min discussion sections.
Presentations may be accompanied by interactive demonstrations. It
concludes with a 30 min panel discussion by presenters.
TOPICS
Topics include, but are not limited to, the following subjects:
- Virtualization in cloud, cluster and grid environments
- VM-based cloud performance modeling
- Workload characterizations for VM-based environments
- Software as a Service (SaaS)
- Cloud reliability, fault-tolerance, and security
- Cloud, cluster and grid filesystems
- QoS and and service levels
- Cross-layer VM optimizations
- Virtualized I/O and storage
- Virtualization and HPC architectures including NUMA
- System and process/bytecode VM convergence
- Paravirtualized driver development
- Research and education use cases
- VM cloud, cluster distribution algorithms
- MPI on virtual machines and clouds
- Cloud frameworks and API sets
- Checkpointing of large compute jobs
- Cloud load balancing
- Accelerator virtualization
- Instrumentation interfaces and languages
- Hardware support for virtualization
- High-performance network virtualization
- Auto-tuning of VMM and VM parameters
- High-speed interconnects
- Hypervisor extensions and tools for cluster and grid computing
- VMMs/Hypervisors
- Cloud use cases including optimizations
- Performance modeling
- Fault tolerant VM environments
- VMM performance tuning on various load types
- Cloud provisioning
- Virtual machine monitor platforms
- Pass-through VM device access
- Management, deployment of VM-based environments
PAPER SUBMISSION
Papers submitted to the workshop will be reviewed by at least two
members of the program committee and external reviewers. Submissions
should include abstract, key words, the e-mail address of the
corresponding author, and must not exceed 10 pages, including tables
and figures at a main font size no smaller than 11 point. Submission
of a paper should be regarded as a commitment that, should the paper
be accepted, at least one of the authors will register and attend the
conference to present the work.
Accepted papers will be published in the Springer LNCS series - the
format must be according to the Springer LNCS Style. Initial
submissions are in PDF, accepted papers will be requested to provided
source files.
Format Guidelines: http://www.springer.de/comp/lncs/authors.html
Submission Link: http://edas.info/newPaper.php?c=10155
CHAIR
Michael Alexander (chair), IBM, Austria
Gianluigi Zanetti (co-chair), CRS4, Italy
PROGRAM COMMITTEE
Paolo Anedda, CRS4, Italy
Volker Buege, University of Karlsruhe, Germany
Giovanni Busonera, CRS4, Italy
Roberto Canonico, University of Napoli, Italy
Tommaso Cucinotta, Scuola Superiore Sant'Anna, Italy
William Gardner, University of Guelph, Canada
Werner Fischer, Thomas-Krenn AG, Germany
Wolfgang Gentzsch, Max Planck Gesellschaft, Germany
Marcus Hardt, Forschungszentrum Karlsruhe, Germany
Sverre Jarp, CERN, Switzerland
Shantenu Sjha, Louisiana State University, USA
Xuxian Jiang, NC State, USA
Kenji Kaneda, Google, USA
Simone Leo, CRS4, Italy
Ignancio Llorente, Universidad Complutense de Madrid, Spain,
Naoya Maruyama, Tokyo Institute of Technology, Japan
Jean-Marc Menaud, Ecole des Mines de Nantes, France
Anastassios Nanos, National Technical University of Athens, Greece
Jose Renato Santos, HP Labs, USA
Deepak Singh, Amazon Webservices, USA
Boria Sotomayor, University of Chicago, USA
Yoshio Turner, HP Labs, USA
Kurt Tutschku, University of Vienna, Austria
Lizhe Wang, Indiana University, USA
Chao-Tung Yang, Tunghai University, China
DURATION: Workshop Duration is one day.
GENERAL INFORMATION
The workshop will be held as part of Euro-Par 2011,
organized by INRIA, CNRS and the University of Bordeaux I, II, France.
Euro-Par 2011: http://europar2011.bordeaux.inria.fr/
^ permalink raw reply
* [PATCH]: Staging: hv: Allocate the vmbus irq dynamically
From: K. Y. Srinivasan @ 2011-02-15 19:55 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization
Cc: K. Y. Srinivasan, Haiyang Zhang, Hank Janssen
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 72 +++++++++++++++++++++++++++++++---------
1 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 459c707..441ce85 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -36,9 +36,7 @@
#include "vmbus_private.h"
-/* FIXME! We need to do this dynamically for PIC and APIC system */
-#define VMBUS_IRQ 0x5
-#define VMBUS_IRQ_VECTOR IRQ5_VECTOR
+static int vmbus_irq;
/* Main vmbus driver data structure */
struct vmbus_driver_context {
@@ -57,6 +55,27 @@ struct vmbus_driver_context {
struct vm_device device_ctx;
};
+/*
+ * Find an un-used IRQ that the VMBUS can use. If none is available;
+ * return -EBUSY.
+ */
+static int vmbus_get_irq(void)
+{
+ unsigned int avail_irq_mask;
+ int irq = -EBUSY;
+
+ /*
+ * Pick the first unused interrupt. HyperV can
+ * interrupt us on any interrupt line we specify.
+ */
+
+ avail_irq_mask = probe_irq_on();
+ if (avail_irq_mask != 0)
+ irq = ffs(avail_irq_mask);
+ probe_irq_off(avail_irq_mask);
+ return irq;
+}
+
static int vmbus_match(struct device *device, struct device_driver *driver);
static int vmbus_probe(struct device *device);
static int vmbus_remove(struct device *device);
@@ -80,7 +99,6 @@ EXPORT_SYMBOL(vmbus_loglevel);
/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
/* (((VMBUS | VMBUS_DRV)<<16) | DEBUG_LVL_ENTEREXIT); */
-static int vmbus_irq = VMBUS_IRQ;
/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
static struct device_attribute vmbus_device_attrs[] = {
@@ -467,6 +485,7 @@ static int vmbus_bus_init(void)
struct vm_device *dev_ctx = &vmbus_drv.device_ctx;
int ret;
unsigned int vector;
+ bool irq_assigned = false;
DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
HV_DRV_VERSION);
@@ -517,20 +536,42 @@ static int vmbus_bus_init(void)
goto cleanup;
}
- /* Get the interrupt resource */
- ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
- driver->name, NULL);
-
- if (ret != 0) {
- DPRINT_ERR(VMBUS_DRV, "ERROR - Unable to request IRQ %d",
- vmbus_irq);
+ /*
+ * Get an unused interrupt line and register our handler.
+ * Since there is a window between getting an unused
+ * interrupt line and registering our handler, we close
+ * this window by repeating the sequence if we raced.
+ * This loop will terminate under the following conditions:
+ *
+ * 1) If we succeed in registering our handler OR
+ * 2) If there are no free interrupt lines for our use OR
+ * 3) request_irq fails for reasons other than an irq collision
+ */
+ while (!irq_assigned) {
+ vmbus_irq = vmbus_get_irq();
- bus_unregister(&vmbus_drv_ctx->bus);
+ if (vmbus_irq < 0) {
+ bus_unregister(&vmbus_drv_ctx->bus);
+ ret = -EBUSY;
+ goto cleanup;
+ }
- ret = -1;
- goto cleanup;
+ ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
+ driver->name, NULL);
+ switch (ret) {
+ case -EBUSY:
+ /* We raced and lost; try again.*/
+ continue;
+ case 0:
+ irq_assigned = true;
+ break;
+ default:
+ /* Failed to request_irq; cleanup */
+ goto cleanup;
+ }
}
- vector = VMBUS_IRQ_VECTOR;
+
+ vector = IRQ0_VECTOR + vmbus_irq;
DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq, vector);
@@ -1117,7 +1158,6 @@ MODULE_DEVICE_TABLE(pci, microsoft_hv_pci_table);
MODULE_LICENSE("GPL");
MODULE_VERSION(HV_DRV_VERSION);
-module_param(vmbus_irq, int, S_IRUGO);
module_param(vmbus_loglevel, int, S_IRUGO);
module_init(vmbus_init);
--
1.5.5.6
^ permalink raw reply related
* Re: [PATCH ]:Staging: hv: Allocate the vmbus irq dynamically
From: Greg KH @ 2011-02-15 19:33 UTC (permalink / raw)
To: Hank Janssen
Cc: KY Srinivasan, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hashir Abdi, Mike Sterling
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56233F97479@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Tue, Feb 15, 2011 at 07:09:34PM +0000, Hank Janssen wrote:
>
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Tuesday, February 15, 2011 9:23 AM
> > > > Before the end of the week I will submit two patches for this;
> > > >
> > > > Remove DPRINT and change it to printk
> > >
> > > No, use dev_dbg() and friends instead of "raw" printk() calls.
> > >
> >
> > Will do, you caught me just as I was starting the conversion :)
>
> While cleaning this up there are a few places in vmbus and channel behavior
> where it is not in a device context. Are printk's okay in that context?
No, use pr_* instead for those. But those should be quite rare, as you
should almost always have a device you are operating on, right?
The reason you don't use "raw" printk() is the dev_dbg() and pr_debug()
calls tie into the dynamic debugging core, which you want to use, as you
don't want to roll your own special way of doing debugging.
> The three drivers network/SCSI and Block of course will use dev_XX family.
Good, but note, I think the network has their own version of this macro
as well :)
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH ]:Staging: hv: Allocate the vmbus irq dynamically
From: Hank Janssen @ 2011-02-15 19:09 UTC (permalink / raw)
To: Greg KH
Cc: KY Srinivasan, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hashir Abdi, Mike Sterling
In-Reply-To: <20110215172253.GB18437@suse.de>
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Tuesday, February 15, 2011 9:23 AM
> > > Before the end of the week I will submit two patches for this;
> > >
> > > Remove DPRINT and change it to printk
> >
> > No, use dev_dbg() and friends instead of "raw" printk() calls.
> >
>
> Will do, you caught me just as I was starting the conversion :)
While cleaning this up there are a few places in vmbus and channel behavior
where it is not in a device context. Are printk's okay in that context?
The three drivers network/SCSI and Block of course will use dev_XX family.
Hank.
^ permalink raw reply
* Re: [PATCH 2/3]: Staging: hv: Use native wait primitives
From: Dmitry Torokhov @ 2011-02-15 18:44 UTC (permalink / raw)
To: KY Srinivasan
Cc: Greg KH, Jiri Slaby, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <FB42D5CCD7B5934EB1827DB5ED9B850E07085DE6@TK5EX14MBXC104.redmond.corp.microsoft.com>
On Tue, Feb 15, 2011 at 05:52:46PM +0000, KY Srinivasan wrote:
>
> If I understand you correctly, you would be prefer to have unbounded waiting with comments
> justifying why we cannot have timeouts. I will roll out a patch once the tree stabilizes.
>
Just make sure to add a comment explaining why the wait is unbounded.
--
Dmitry
^ permalink raw reply
* RE: [PATCH 2/3]: Staging: hv: Use native wait primitives
From: KY Srinivasan @ 2011-02-15 17:52 UTC (permalink / raw)
To: Greg KH
Cc: Jiri Slaby, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110215162933.GA30626@suse.de>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, February 15, 2011 11:30 AM
> To: KY Srinivasan
> Cc: Jiri Slaby; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org
> Subject: Re: [PATCH 2/3]: Staging: hv: Use native wait primitives
>
> On Tue, Feb 15, 2011 at 04:22:20PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Tuesday, February 15, 2011 9:03 AM
> > > To: KY Srinivasan
> > > Cc: Jiri Slaby; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > virtualization@lists.osdl.org
> > > Subject: Re: [PATCH 2/3]: Staging: hv: Use native wait primitives
> > >
> > > On Tue, Feb 15, 2011 at 01:35:56PM +0000, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jiri Slaby [mailto:jirislaby@gmail.com]
> > > > > Sent: Tuesday, February 15, 2011 4:21 AM
> > > > > To: KY Srinivasan
> > > > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > > > Subject: Re: [PATCH 2/3]: Staging: hv: Use native wait primitives
> > > > >
> > > > > On 02/11/2011 06:59 PM, K. Y. Srinivasan wrote:
> > > > > > In preperation for getting rid of the osd layer; change
> > > > > > the code to use native wait interfaces. As part of this,
> > > > > > fixed the buggy implementation in the osd_wait_primitive
> > > > > > where the condition was cleared potentially after the
> > > > > > condition was signalled.
> > > > > ...
> > > > > > @@ -566,7 +567,11 @@ int vmbus_establish_gpadl(struct
> vmbus_channel
> > > > > *channel, void *kbuffer,
> > > > > >
> > > > > > }
> > > > > > }
> > > > > > - osd_waitevent_wait(msginfo->waitevent);
> > > > > > + wait_event_timeout(msginfo->waitevent,
> > > > > > + msginfo->wait_condition,
> > > > > > + msecs_to_jiffies(1000));
> > > > > > + BUG_ON(msginfo->wait_condition == 0);
> > > > >
> > > > > The added BUG_ONs all over the code look scary. These shouldn't be
> > > > > BUG_ONs at all. You should maybe warn and bail out, but not kill the
> > > > > whole machine.
> > > >
> > > > This is Linux code running as a guest on a Windows host; and so the guest
> > > cannot
> > > > tolerate a failure of the host. In the cases where I have chosen to BUG_ON,
> > > there
> > > > is no reasonable recovery possible when the host is non-functional (as
> > > determined
> > > > by a non-responsive host).
> > >
> > > If you have a non-responsive host, wouldn't that imply that this guest
> > > code wouldn't run at all? :)
> >
> > The fact that on a particular transaction the host has not responded within an
> expected
> > time interval does not necessarily mean that the guest code would not be
> running. There may be
> > issues on the host side that may be either transient or permanent that may
> cause problems like
> > this. Keep in mind, HyperV is a type 1 hypervisor that would schedule all VMs
> including the host
> > and so, guest would get scheduled.
> >
> > >
> > > Having BUG_ON() in drivers is not a good idea either way. Please remove
> > > these in future patches.
> >
> > In situations where there is not a reasonable rollback strategy (for
> > instance in one of the cases, we are granting access to the guest
> > physical pages to the host) we really have only 2 options:
> >
> > 1) Wait until the host responds. This wait could potentially be unbounded
> > and in fact this was the way the code was to begin with. One of the reviewers
> > had suggested that unbounded wait was to be corrected.
> > 2) Wait for a specific period and if the host does not respond
> > within a reasonable period, kill the guest since there is no recovery
> > possible.
>
> Killing the guest is a very serious thing, causing all sorts of possible
> problems with it, right?
If there was a reasonable rollback strategy, I would not
be killing the guest.
>
> > I chose option 2, as part of addressing some of the prior review
> > comments. If the consensus now is to go back to option 1, I am fine with that;
>
> Unbounded waits aren't ok either, you need some sort of timeout.
>
> But, as this is a bit preferable to dieing, I suggest doing this, and
> comment the heck out of it to explain all of this for anyone who reads
> it.
If I understand you correctly, you would be prefer to have unbounded waiting with comments
justifying why we cannot have timeouts. I will roll out a patch once the tree stabilizes.
Regards,
K. Y
>
> thanks,
>
> greg k-h
^ permalink raw reply
* RE: [PATCH ]:Staging: hv: Allocate the vmbus irq dynamically
From: Hank Janssen @ 2011-02-15 17:28 UTC (permalink / raw)
To: Greg KH
Cc: Hashir Abdi, linux-kernel@vger.kernel.org,
virtualization@lists.osdl.org, Mike Sterling,
devel@linuxdriverproject.org
In-Reply-To: <20110215172253.GB18437@suse.de>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, February 15, 2011 9:23 AM
> > Before the end of the week I will submit two patches for this;
> >
> > Remove DPRINT and change it to printk
>
> No, use dev_dbg() and friends instead of "raw" printk() calls.
>
Will do, you caught me just as I was starting the conversion :)
> > Remove excessive printouts on startup of vmbus
>
> That would be nice to see. There should be almost nothing outputed by
> these drivers on "normal" operation.
Agreed.
Hank.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox