diff mbox

[04/20,SCSI] mpt3sas: Remove redundancy code while freeing the controller resources.

Message ID 1434102153-38581-5-git-send-email-Sreekanth.Reddy@avagotech.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sreekanth Reddy June 12, 2015, 9:42 a.m. UTC
Removed the redundancy code while freeing the controller resources.

Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 25 deletions(-)

Comments

Johannes Thumshirn June 12, 2015, 11:28 a.m. UTC | #1
On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
> Removed the redundancy code while freeing the controller resources.
> 
> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index ce57320..32b86bf 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -1798,6 +1798,36 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
>  }
>  
>  /**
> + * mpt3sas_base_unmap_resources - free controller resources
> + * @ioc: per adapter object
> + */
> +void
> +mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
> +{
> +	struct pci_dev *pdev = ioc->pdev;
> +
> +	dexitprintk(ioc, printk(MPT3SAS_FMT "%s\n",
> +		ioc->name, __func__));
> +
> +	_base_free_irq(ioc);
> +	_base_disable_msix(ioc);
> +
> +	if (ioc->msix96_vector)
> +		kfree(ioc->replyPostRegisterIndex);

kfree() already checks for zero or a NULL pointer.

> +
> +	if (ioc->chip_phys) {
> +		iounmap(ioc->chip);
> +		ioc->chip_phys = 0;
> +	}
> +
> +	if (pci_is_enabled(pdev)) {
> +		pci_release_selected_regions(ioc->pdev, ioc->bars);
> +		pci_disable_pcie_error_reporting(pdev);
> +		pci_disable_device(pdev);
> +	}
> +}
> +
> +/**
>   * mpt3sas_base_map_resources - map in controller resources (io/irq/memap)
>   * @ioc: per adapter object
>   *
> @@ -1925,14 +1955,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>  	return 0;
>  
>   out_fail:
> -	if (ioc->chip_phys)
> -		iounmap(ioc->chip);
> -	ioc->chip_phys = 0;
> -	pci_release_selected_regions(ioc->pdev, ioc->bars);
> -	pci_disable_pcie_error_reporting(pdev);
> -	pci_disable_device(pdev);
> -	if (ioc->msix96_vector)
> -		kfree(ioc->replyPostRegisterIndex);
> +	mpt3sas_base_unmap_resources(ioc);
>  	return r;
>  }
>  
> @@ -4667,8 +4690,6 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
>  void
>  mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
>  {
> -	struct pci_dev *pdev = ioc->pdev;
> -
>  	dexitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
>  	    __func__));
>  
> @@ -4679,21 +4700,7 @@ mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
>  		ioc->shost_recovery = 0;
>  	}
>  
> -	_base_free_irq(ioc);
> -	_base_disable_msix(ioc);
> -
> -	if (ioc->msix96_vector)
> -		kfree(ioc->replyPostRegisterIndex);
> -
> -	if (ioc->chip_phys && ioc->chip)
> -		iounmap(ioc->chip);
> -	ioc->chip_phys = 0;
> -
> -	if (pci_is_enabled(pdev)) {
> -		pci_release_selected_regions(ioc->pdev, ioc->bars);
> -		pci_disable_pcie_error_reporting(pdev);
> -		pci_disable_device(pdev);
> -	}
> +	mpt3sas_base_unmap_resources(ioc);
>  	return;
>  }
>  
> -- 
> 2.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sreekanth Reddy June 12, 2015, 12:18 p.m. UTC | #2
On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
>> Removed the redundancy code while freeing the controller resources.
>>
>> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
>>  1 file changed, 32 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index ce57320..32b86bf 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -1798,6 +1798,36 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
>>  }
>>
>>  /**
>> + * mpt3sas_base_unmap_resources - free controller resources
>> + * @ioc: per adapter object
>> + */
>> +void
>> +mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
>> +{
>> +     struct pci_dev *pdev = ioc->pdev;
>> +
>> +     dexitprintk(ioc, printk(MPT3SAS_FMT "%s\n",
>> +             ioc->name, __func__));
>> +
>> +     _base_free_irq(ioc);
>> +     _base_disable_msix(ioc);
>> +
>> +     if (ioc->msix96_vector)
>> +             kfree(ioc->replyPostRegisterIndex);
>
> kfree() already checks for zero or a NULL pointer.

Sorry Johannes, I didn't get you. If I understand this correctly, you
are suggesting to check for NULL pointer before calling kree() API as
shown below,

if (ioc->msix96_vector && (ioc->replyPostRegisterIndex != NULL))
        kfree(ioc->replyPostRegisterIndex);

>
>> +
>> +     if (ioc->chip_phys) {
>> +             iounmap(ioc->chip);
>> +             ioc->chip_phys = 0;
>> +     }
>> +
>> +     if (pci_is_enabled(pdev)) {
>> +             pci_release_selected_regions(ioc->pdev, ioc->bars);
>> +             pci_disable_pcie_error_reporting(pdev);
>> +             pci_disable_device(pdev);
>> +     }
>> +}
>> +
>> +/**
>>   * mpt3sas_base_map_resources - map in controller resources (io/irq/memap)
>>   * @ioc: per adapter object
>>   *
>> @@ -1925,14 +1955,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>>       return 0;
>>
>>   out_fail:
>> -     if (ioc->chip_phys)
>> -             iounmap(ioc->chip);
>> -     ioc->chip_phys = 0;
>> -     pci_release_selected_regions(ioc->pdev, ioc->bars);
>> -     pci_disable_pcie_error_reporting(pdev);
>> -     pci_disable_device(pdev);
>> -     if (ioc->msix96_vector)
>> -             kfree(ioc->replyPostRegisterIndex);
>> +     mpt3sas_base_unmap_resources(ioc);
>>       return r;
>>  }
>>
>> @@ -4667,8 +4690,6 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
>>  void
>>  mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
>>  {
>> -     struct pci_dev *pdev = ioc->pdev;
>> -
>>       dexitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
>>           __func__));
>>
>> @@ -4679,21 +4700,7 @@ mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
>>               ioc->shost_recovery = 0;
>>       }
>>
>> -     _base_free_irq(ioc);
>> -     _base_disable_msix(ioc);
>> -
>> -     if (ioc->msix96_vector)
>> -             kfree(ioc->replyPostRegisterIndex);
>> -
>> -     if (ioc->chip_phys && ioc->chip)
>> -             iounmap(ioc->chip);
>> -     ioc->chip_phys = 0;
>> -
>> -     if (pci_is_enabled(pdev)) {
>> -             pci_release_selected_regions(ioc->pdev, ioc->bars);
>> -             pci_disable_pcie_error_reporting(pdev);
>> -             pci_disable_device(pdev);
>> -     }
>> +     mpt3sas_base_unmap_resources(ioc);
>>       return;
>>  }
>>
>> --
>> 2.0.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Johannes Thumshirn                                       Storage
> jthumshirn@suse.de                             +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
Johannes Thumshirn June 12, 2015, 12:40 p.m. UTC | #3
On Fri, Jun 12, 2015 at 05:48:56PM +0530, Sreekanth Reddy wrote:
> On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> > On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
> >> Removed the redundancy code while freeing the controller resources.
> >>
> >> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
> >> ---
> >>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
[...]
> >> +     _base_free_irq(ioc);
> >> +     _base_disable_msix(ioc);
> >> +
> >> +     if (ioc->msix96_vector)
> >> +             kfree(ioc->replyPostRegisterIndex);
> >
> > kfree() already checks for zero or a NULL pointer.
> 
> Sorry Johannes, I didn't get you. If I understand this correctly, you
> are suggesting to check for NULL pointer before calling kree() API as
> shown below,
> 
> if (ioc->msix96_vector && (ioc->replyPostRegisterIndex != NULL))
>         kfree(ioc->replyPostRegisterIndex);

Correct me if I'm wrong, but I thought you don't need the if
(ioc->msix96_vector) before the kfree(). ioc->replyPostRegisterIndex should be
NULL if ioc->msix96_vector is 0, as far as I can see.

In _scsih_probe() you have:
shost = scsi_host_alloc(&scsih_driver_template,
            sizeof(struct MPT3SAS_ADAPTER));
if (!shost)
           return -ENODEV;

/* init local params */
ioc = shost_priv(shost);

and scsi_host_alloc() does a kzalloc() for shost.

so ioc->replyPortRegisterIndex is NULL.

Or am I thinking wrong here?

> 
> Regards,
> Sreekanth
Sreekanth Reddy June 15, 2015, 10:26 a.m. UTC | #4
On Fri, Jun 12, 2015 at 6:10 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Fri, Jun 12, 2015 at 05:48:56PM +0530, Sreekanth Reddy wrote:
>> On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
>> > On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
>> >> Removed the redundancy code while freeing the controller resources.
>> >>
>> >> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
>> >> ---
>> >>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
> [...]
>> >> +     _base_free_irq(ioc);
>> >> +     _base_disable_msix(ioc);
>> >> +
>> >> +     if (ioc->msix96_vector)
>> >> +             kfree(ioc->replyPostRegisterIndex);
>> >
>> > kfree() already checks for zero or a NULL pointer.
>>
>> Sorry Johannes, I didn't get you. If I understand this correctly, you
>> are suggesting to check for NULL pointer before calling kree() API as
>> shown below,
>>
>> if (ioc->msix96_vector && (ioc->replyPostRegisterIndex != NULL))
>>         kfree(ioc->replyPostRegisterIndex);
>
> Correct me if I'm wrong, but I thought you don't need the if
> (ioc->msix96_vector) before the kfree(). ioc->replyPostRegisterIndex should be
> NULL if ioc->msix96_vector is 0, as far as I can see.
>
> In _scsih_probe() you have:
> shost = scsi_host_alloc(&scsih_driver_template,
>             sizeof(struct MPT3SAS_ADAPTER));
> if (!shost)
>            return -ENODEV;
>
> /* init local params */
> ioc = shost_priv(shost);
>
> and scsi_host_alloc() does a kzalloc() for shost.
>
> so ioc->replyPortRegisterIndex is NULL.
>
> Or am I thinking wrong here?

Yes, ioc->replyPostRegisterIndex will be NULL if ioc->msix96_vector is 0,
We have added this checks as a precautionary. since calling this
function (i.e kfree()) on memory not previously allocated with
kmalloc(), or on memory which has already been freed, may results in
very bad things, such as freeing memory belonging to another part of
the kernel.

>
>>
>> Regards,
>> Sreekanth
>
> --
> Johannes Thumshirn                                       Storage
> jthumshirn@suse.de                             +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
Johannes Thumshirn June 15, 2015, 10:48 a.m. UTC | #5
On Mon, Jun 15, 2015 at 03:56:56PM +0530, Sreekanth Reddy wrote:
> On Fri, Jun 12, 2015 at 6:10 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> > On Fri, Jun 12, 2015 at 05:48:56PM +0530, Sreekanth Reddy wrote:
> >> On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> >> > On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
> >> >> Removed the redundancy code while freeing the controller resources.
> >> >>
> >> >> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
> >> >> ---
> >> >>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
> > [...]
> >> >> +     _base_free_irq(ioc);
> >> >> +     _base_disable_msix(ioc);
> >> >> +
> >> >> +     if (ioc->msix96_vector)
> >> >> +             kfree(ioc->replyPostRegisterIndex);
> >> >
> >> > kfree() already checks for zero or a NULL pointer.
> >>
> >> Sorry Johannes, I didn't get you. If I understand this correctly, you
> >> are suggesting to check for NULL pointer before calling kree() API as
> >> shown below,
> >>
> >> if (ioc->msix96_vector && (ioc->replyPostRegisterIndex != NULL))
> >>         kfree(ioc->replyPostRegisterIndex);
> >
> > Correct me if I'm wrong, but I thought you don't need the if
> > (ioc->msix96_vector) before the kfree(). ioc->replyPostRegisterIndex should be
> > NULL if ioc->msix96_vector is 0, as far as I can see.
> >
> > In _scsih_probe() you have:
> > shost = scsi_host_alloc(&scsih_driver_template,
> >             sizeof(struct MPT3SAS_ADAPTER));
> > if (!shost)
> >            return -ENODEV;
> >
> > /* init local params */
> > ioc = shost_priv(shost);
> >
> > and scsi_host_alloc() does a kzalloc() for shost.
> >
> > so ioc->replyPortRegisterIndex is NULL.
> >
> > Or am I thinking wrong here?
> 
> Yes, ioc->replyPostRegisterIndex will be NULL if ioc->msix96_vector is 0,
> We have added this checks as a precautionary. since calling this
> function (i.e kfree()) on memory not previously allocated with
> kmalloc(), or on memory which has already been freed, may results in
> very bad things, such as freeing memory belonging to another part of
> the kernel.
>

OK, then please leave it in.

> >
> >>
> >> Regards,
> >> Sreekanth
> >
> > --
> > Johannes Thumshirn                                       Storage
> > jthumshirn@suse.de                             +49 911 74053 689
> > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> > GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> > HRB 21284 (AG Nürnberg)
> 
> 
> 
> -- 
> 
> Regards,
> Sreekanth
Sreekanth Reddy June 18, 2015, 9:52 a.m. UTC | #6
Hi,

Any other review comments on this patch. please let us known if any
changes are required.

Thanks,
Sreekanth

On Mon, Jun 15, 2015 at 4:18 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Mon, Jun 15, 2015 at 03:56:56PM +0530, Sreekanth Reddy wrote:
>> On Fri, Jun 12, 2015 at 6:10 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
>> > On Fri, Jun 12, 2015 at 05:48:56PM +0530, Sreekanth Reddy wrote:
>> >> On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
>> >> > On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
>> >> >> Removed the redundancy code while freeing the controller resources.
>> >> >>
>> >> >> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
>> >> >> ---
>> >> >>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
>> > [...]
>> >> >> +     _base_free_irq(ioc);
>> >> >> +     _base_disable_msix(ioc);
>> >> >> +
>> >> >> +     if (ioc->msix96_vector)
>> >> >> +             kfree(ioc->replyPostRegisterIndex);
>> >> >
>> >> > kfree() already checks for zero or a NULL pointer.
>> >>
>> >> Sorry Johannes, I didn't get you. If I understand this correctly, you
>> >> are suggesting to check for NULL pointer before calling kree() API as
>> >> shown below,
>> >>
>> >> if (ioc->msix96_vector && (ioc->replyPostRegisterIndex != NULL))
>> >>         kfree(ioc->replyPostRegisterIndex);
>> >
>> > Correct me if I'm wrong, but I thought you don't need the if
>> > (ioc->msix96_vector) before the kfree(). ioc->replyPostRegisterIndex should be
>> > NULL if ioc->msix96_vector is 0, as far as I can see.
>> >
>> > In _scsih_probe() you have:
>> > shost = scsi_host_alloc(&scsih_driver_template,
>> >             sizeof(struct MPT3SAS_ADAPTER));
>> > if (!shost)
>> >            return -ENODEV;
>> >
>> > /* init local params */
>> > ioc = shost_priv(shost);
>> >
>> > and scsi_host_alloc() does a kzalloc() for shost.
>> >
>> > so ioc->replyPortRegisterIndex is NULL.
>> >
>> > Or am I thinking wrong here?
>>
>> Yes, ioc->replyPostRegisterIndex will be NULL if ioc->msix96_vector is 0,
>> We have added this checks as a precautionary. since calling this
>> function (i.e kfree()) on memory not previously allocated with
>> kmalloc(), or on memory which has already been freed, may results in
>> very bad things, such as freeing memory belonging to another part of
>> the kernel.
>>
>
> OK, then please leave it in.
>
>> >
>> >>
>> >> Regards,
>> >> Sreekanth
>> >
>> > --
>> > Johannes Thumshirn                                       Storage
>> > jthumshirn@suse.de                             +49 911 74053 689
>> > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
>> > GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
>> > HRB 21284 (AG Nürnberg)
>>
>>
>>
>> --
>>
>> Regards,
>> Sreekanth
>
> --
> Johannes Thumshirn                                       Storage
> jthumshirn@suse.de                             +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
Johannes Thumshirn June 18, 2015, 10:08 a.m. UTC | #7
On Fri, Jun 12, 2015 at 03:12:16PM +0530, Sreekanth Reddy wrote:
> Removed the redundancy code while freeing the controller resources.
> 
> Signed-off-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index ce57320..32b86bf 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -1798,6 +1798,36 @@ _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
>  }
>  
>  /**
> + * mpt3sas_base_unmap_resources - free controller resources
> + * @ioc: per adapter object
> + */
> +void
> +mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
> +{
> +	struct pci_dev *pdev = ioc->pdev;
> +
> +	dexitprintk(ioc, printk(MPT3SAS_FMT "%s\n",
> +		ioc->name, __func__));
> +
> +	_base_free_irq(ioc);
> +	_base_disable_msix(ioc);
> +
> +	if (ioc->msix96_vector)
> +		kfree(ioc->replyPostRegisterIndex);
> +
> +	if (ioc->chip_phys) {
> +		iounmap(ioc->chip);
> +		ioc->chip_phys = 0;
> +	}
> +
> +	if (pci_is_enabled(pdev)) {
> +		pci_release_selected_regions(ioc->pdev, ioc->bars);
> +		pci_disable_pcie_error_reporting(pdev);
> +		pci_disable_device(pdev);
> +	}
> +}
> +
> +/**
>   * mpt3sas_base_map_resources - map in controller resources (io/irq/memap)
>   * @ioc: per adapter object
>   *
> @@ -1925,14 +1955,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>  	return 0;
>  
>   out_fail:
> -	if (ioc->chip_phys)
> -		iounmap(ioc->chip);
> -	ioc->chip_phys = 0;
> -	pci_release_selected_regions(ioc->pdev, ioc->bars);
> -	pci_disable_pcie_error_reporting(pdev);
> -	pci_disable_device(pdev);
> -	if (ioc->msix96_vector)
> -		kfree(ioc->replyPostRegisterIndex);
> +	mpt3sas_base_unmap_resources(ioc);
>  	return r;
>  }
>  
> @@ -4667,8 +4690,6 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
>  void
>  mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
>  {
> -	struct pci_dev *pdev = ioc->pdev;
> -
>  	dexitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
>  	    __func__));
>  
> @@ -4679,21 +4700,7 @@ mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
>  		ioc->shost_recovery = 0;
>  	}
>  
> -	_base_free_irq(ioc);
> -	_base_disable_msix(ioc);
> -
> -	if (ioc->msix96_vector)
> -		kfree(ioc->replyPostRegisterIndex);
> -
> -	if (ioc->chip_phys && ioc->chip)
> -		iounmap(ioc->chip);
> -	ioc->chip_phys = 0;
> -
> -	if (pci_is_enabled(pdev)) {
> -		pci_release_selected_regions(ioc->pdev, ioc->bars);
> -		pci_disable_pcie_error_reporting(pdev);
> -		pci_disable_device(pdev);
> -	}
> +	mpt3sas_base_unmap_resources(ioc);
>  	return;
>  }
>  
> -- 
> 2.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Martin K. Petersen June 19, 2015, 3:09 p.m. UTC | #8
>>>>> Sreekanth Reddy <sreekanth.reddy@avagotech.com> writes:

> Removed the redundancy code while freeing the controller resources.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
diff mbox

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index ce57320..32b86bf 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1798,6 +1798,36 @@  _base_enable_msix(struct MPT3SAS_ADAPTER *ioc)
 }
 
 /**
+ * mpt3sas_base_unmap_resources - free controller resources
+ * @ioc: per adapter object
+ */
+void
+mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
+{
+	struct pci_dev *pdev = ioc->pdev;
+
+	dexitprintk(ioc, printk(MPT3SAS_FMT "%s\n",
+		ioc->name, __func__));
+
+	_base_free_irq(ioc);
+	_base_disable_msix(ioc);
+
+	if (ioc->msix96_vector)
+		kfree(ioc->replyPostRegisterIndex);
+
+	if (ioc->chip_phys) {
+		iounmap(ioc->chip);
+		ioc->chip_phys = 0;
+	}
+
+	if (pci_is_enabled(pdev)) {
+		pci_release_selected_regions(ioc->pdev, ioc->bars);
+		pci_disable_pcie_error_reporting(pdev);
+		pci_disable_device(pdev);
+	}
+}
+
+/**
  * mpt3sas_base_map_resources - map in controller resources (io/irq/memap)
  * @ioc: per adapter object
  *
@@ -1925,14 +1955,7 @@  mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
 	return 0;
 
  out_fail:
-	if (ioc->chip_phys)
-		iounmap(ioc->chip);
-	ioc->chip_phys = 0;
-	pci_release_selected_regions(ioc->pdev, ioc->bars);
-	pci_disable_pcie_error_reporting(pdev);
-	pci_disable_device(pdev);
-	if (ioc->msix96_vector)
-		kfree(ioc->replyPostRegisterIndex);
+	mpt3sas_base_unmap_resources(ioc);
 	return r;
 }
 
@@ -4667,8 +4690,6 @@  _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc, int sleep_flag)
 void
 mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
 {
-	struct pci_dev *pdev = ioc->pdev;
-
 	dexitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
 	    __func__));
 
@@ -4679,21 +4700,7 @@  mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc)
 		ioc->shost_recovery = 0;
 	}
 
-	_base_free_irq(ioc);
-	_base_disable_msix(ioc);
-
-	if (ioc->msix96_vector)
-		kfree(ioc->replyPostRegisterIndex);
-
-	if (ioc->chip_phys && ioc->chip)
-		iounmap(ioc->chip);
-	ioc->chip_phys = 0;
-
-	if (pci_is_enabled(pdev)) {
-		pci_release_selected_regions(ioc->pdev, ioc->bars);
-		pci_disable_pcie_error_reporting(pdev);
-		pci_disable_device(pdev);
-	}
+	mpt3sas_base_unmap_resources(ioc);
 	return;
 }