I have taken the original patch from Prarit and made changes to move the allocation 
inside the main for loop along with request_irq.

I feel doing devres would be good but going to take some time. However, we need to fix this now and hence
am submitting this patch. When we have a final devres solution working and tested ,we would move over.

Thanks
Jay
________________________________________
From: Prarit Bhargava [prarit@redhat.com]
Sent: Friday, May 20, 2011 11:51 AM
To: Kallickal, Jayamohan
Cc: linux-scsi@vger.kernel.org; mchristi@redhat.com
Subject: Re: [PATCH]: be2iscsi: Fix MSIX interrupt names

On 05/20/2011 02:33 PM, Jayamohan.Kallickal@Emulex.Com wrote:
> Thanks for the patch. Pl see my comments in line
>
>

np.

> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 24e20ba..8d71e47 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -874,16 +874,20 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
>       struct hwi_controller *phwi_ctrlr;
>       struct hwi_context_memory *phwi_context;
>       int ret, msix_vec, i, j;
> -     char desc[32];
>
>       phwi_ctrlr = phba->phwi_ctrlr;
>       phwi_context = phwi_ctrlr->phwi_ctxt;
>
>       if (phba->msix_enabled) {
>               for (i = 0; i < phba->num_cpus; i++) {
> -                     sprintf(desc, "beiscsi_msix_%04x", i);
> +                     phba->msi_name[i] = kzalloc(BEISCSI_MSI_NAME,
> +                                                 GFP_KERNEL);
> +                     if (!phba->msi_name[i])
> +                             goto free_msix_irqs;
> We need to ensure i != 0 before jumping to free_msix_irqs
>

Will fix in next version ... I think I may have to do a bit more work
here because I just realized that if we free_irq() on a non-allocated
irq we'll get an angry message from the kernel ;)  Unfortunately this
may complicate the code.... I'll rework this and see if I can come up
with something better.

> +                     sprintf(phba->msi_name[i], "beiscsi_msix_%04x", i);
>                       msix_vec = phba->msix_entries[i].vector;
> -                     ret = request_irq(msix_vec, be_isr_msix, 0, desc,
> +                     ret = request_irq(msix_vec, be_isr_msix, 0,
> +                                       phba->msi_name[i],
>                                         &phwi_context->be_eq[i]);
>                       if (ret) {
>                               shost_printk(KERN_ERR, phba->shost,
> @@ -915,8 +919,10 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
>       }
>       return 0;
>  free_msix_irqs:
> -     for (j = i - 1; j == 0; j++)
> +     for (j = i - 1; j == 0; j++) {
>               free_irq(msix_vec, &phwi_context->be_eq[j]);
> +             kfree(phba->msi_name[i]);
> +     }
>

... I was looking at this, and I'm confused by it.

Should this really be 'j++'?  Or should it be j-- ?

It seems to make sense that this code is j-- ...


>       return ret;
>  }
>
> @@ -4117,10 +4123,13 @@ static void beiscsi_remove(struct pci_dev *pcidev)
>               for (i = 0; i <= phba->num_cpus; i++) {
>                       msix_vec = phba->msix_entries[i].vector;
>                       free_irq(msix_vec, &phwi_context->be_eq[i]);
> +                     kfree(phba->msi_name[i]);
>               }
>       } else
> -             if (phba->pcidev->irq)
> +             if (phba->pcidev->irq) {
>                       free_irq(phba->pcidev->irq, phba);
> +                     kfree(phba->msi_name[i]);
> Do we need the free for non-msix case as we are not allocation?
>

Fixed in next version.

<snip>

P.