thr3ads.net - Virtualization - [PATCH ]:Staging: hv: Allocate the vmbus irq dynamically [Feb 2011]

If this information is useful, please help other people find it:
Share via:

K. Y. Srinivasan

2011-Feb-15 15:15 UTC

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

Signed-off-by: K. Y. Srinivasan <kys at microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz at microsoft.com>
Signed-off-by: Hank Janssen <hjanssen at microsoft.com>

---
 drivers/staging/hv/vmbus_drv.c |   49 +++++++++++++++++++++++++++------------
 1 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 459c707..f279e6a 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,25 @@ struct vmbus_driver_context {
 	struct vm_device device_ctx;
 };
 
+/*
+ * Find an un-used IRQ that the VMBUS can use. If none is available; return -1.
+ */
+
+static int vmbus_get_irq(void)
+{
+	unsigned int avail_irq_mask;
+	int irq = -1;
+	/*
+	 * 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 +97,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[] = {
@@ -466,7 +482,7 @@ static int vmbus_bus_init(void)
 	struct hv_driver *driver = &vmbus_drv.drv_obj;
 	struct vm_device *dev_ctx = &vmbus_drv.device_ctx;
 	int ret;
-	unsigned int vector;
+	unsigned int vector, prev_irq = ~0;
 
 	DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
 		    HV_DRV_VERSION);
@@ -518,19 +534,23 @@ static int vmbus_bus_init(void)
 	}
 
 	/* 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_irq_again:
+	vmbus_irq = vmbus_get_irq();
+	if ((vmbus_irq < 0) || (prev_irq == vmbus_irq)) {
+		printk(KERN_WARNING "VMBUS_DRV: Failed to allocate IRQ\n");
 		bus_unregister(&vmbus_drv_ctx->bus);
-
 		ret = -1;
 		goto cleanup;
 	}
-	vector = VMBUS_IRQ_VECTOR;
+	prev_irq = vmbus_irq;
+
+	ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
+			  driver->name, NULL);
+
+	if (ret != 0)
+		goto get_irq_again;
+
+	vector = IRQ0_VECTOR + vmbus_irq;
 
 	DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq, vector);
 
@@ -1117,7 +1137,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

Greg KH

2011-Feb-15 15:59 UTC

head link

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

On Tue, Feb 15, 2011 at 07:15:37AM -0800, K. Y. Srinivasan
wrote:> 
> Signed-off-by: K. Y. Srinivasan <kys at microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz at microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen at microsoft.com>
> 
> ---
>  drivers/staging/hv/vmbus_drv.c |   49
+++++++++++++++++++++++++++------------
>  1 files changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/hv/vmbus_drv.c
b/drivers/staging/hv/vmbus_drv.c
> index 459c707..f279e6a 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,25 @@ struct vmbus_driver_context {
>  	struct vm_device device_ctx;
>  };
>  
> +/*
> + * Find an un-used IRQ that the VMBUS can use. If none is available;
return -1.
> + */
> +
> +static int vmbus_get_irq(void)
No extra blank line between function comment and function.
> +{
> +	unsigned int avail_irq_mask;
> +	int irq = -1;
> +	/*
Put an extra blank line after your variables please.
> +	 * 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 +97,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[] = {
> @@ -466,7 +482,7 @@ static int vmbus_bus_init(void)
>  	struct hv_driver *driver = &vmbus_drv.drv_obj;
>  	struct vm_device *dev_ctx = &vmbus_drv.device_ctx;
>  	int ret;
> -	unsigned int vector;
> +	unsigned int vector, prev_irq = ~0;
>  
>  	DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
>  		    HV_DRV_VERSION);
> @@ -518,19 +534,23 @@ static int vmbus_bus_init(void)
>  	}
>  
>  	/* 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_irq_again:
> +	vmbus_irq = vmbus_get_irq();
> +	if ((vmbus_irq < 0) || (prev_irq == vmbus_irq)) {
> +		printk(KERN_WARNING "VMBUS_DRV: Failed to allocate IRQ\n");
>  		bus_unregister(&vmbus_drv_ctx->bus);
> -
>  		ret = -1;
Please provide a "real" error code.
>  		goto cleanup;
>  	}
> -	vector = VMBUS_IRQ_VECTOR;
> +	prev_irq = vmbus_irq;
> +
> +	ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
> +			  driver->name, NULL);
> +
> +	if (ret != 0)
> +		goto get_irq_again;
You just set up the potential for an endless loop.  You almost _never_
want to goto backwards in a function.  Please don't do this.
> +
> +	vector = IRQ0_VECTOR + vmbus_irq;
What's this for?
>  	DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq,
vector);
And why are you still printing this out for the whole world to see?

thanks,

greg k-h

Greg KH

2011-Feb-15 17:22 UTC

head link

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

On Tue, Feb 15, 2011 at 04:59:28PM +0000, Hank Janssen
wrote:> 
> 
> > -----Original Message-----
> > From: KY Srinivasan
> > Sent: Tuesday, February 15, 2011 8:54 AM
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh at suse.de]
> > > Sent: Tuesday, February 15, 2011 11:00 AM
> > > To: KY Srinivasan
> > > Cc: linux-kernel at vger.kernel.org; devel at
linuxdriverproject.org;
> > > virtualization at lists.osdl.org; Haiyang Zhang; Hank Janssen
> > > Subject: Re: [PATCH ]:Staging: hv: Allocate the vmbus irq
dynamically
> > >
> 
> > > And why are you still printing this out for the whole world to
see?
> > Greg, this patch addressed a specific comment with regards dynamically
> > allocate the irq.
> > Hank is working on a patch to address the various DPRINTS in the code.
> 
> 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.
> 	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.

thanks,

greg k-h

Hank Janssen

2011-Feb-15 17:28 UTC

head link

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

> -----Original Message-----
> From: Greg KH [mailto:gregkh at 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.

Reasonably Related Threads

Search for more apparently analagous threads

Virtualization - Feb 2011 - [PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

[PATCH ]:Staging: hv: Allocate the vmbus irq dynamically

Reasonably Related Threads