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

2015-06-19 Thread Martin K. Petersen
 Sreekanth Reddy sreekanth.re...@avagotech.com writes:

 Removed the redundancy code while freeing the controller resources.

Reviewed-by: Martin K. Petersen martin.peter...@oracle.com

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in


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

2015-06-18 Thread Sreekanth Reddy
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 jthumsh...@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 jthumsh...@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 jthumsh...@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.re...@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
  jthumsh...@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
 jthumsh...@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
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-18 Thread Johannes Thumshirn
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.re...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Johannes Thumshirn jthumsh...@suse.de

-- 
Johannes Thumshirn   Storage
jthumsh...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-15 Thread Johannes Thumshirn
On Mon, Jun 15, 2015 at 03:56:56PM +0530, Sreekanth Reddy wrote:
 On Fri, Jun 12, 2015 at 6:10 PM, Johannes Thumshirn jthumsh...@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 jthumsh...@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.re...@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
  jthumsh...@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
jthumsh...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-15 Thread Sreekanth Reddy
On Fri, Jun 12, 2015 at 6:10 PM, Johannes Thumshirn jthumsh...@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 jthumsh...@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.re...@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
 jthumsh...@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
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-12 Thread Sreekanth Reddy
Removed the redundancy code while freeing the controller resources.

Signed-off-by: Sreekanth Reddy sreekanth.re...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-12 Thread Sreekanth Reddy
On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn jthumsh...@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.re...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

 --
 Johannes Thumshirn   Storage
 jthumsh...@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
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-12 Thread Johannes Thumshirn
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.re...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Johannes Thumshirn   Storage
jthumsh...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-12 Thread Johannes Thumshirn
On Fri, Jun 12, 2015 at 05:48:56PM +0530, Sreekanth Reddy wrote:
 On Fri, Jun 12, 2015 at 4:58 PM, Johannes Thumshirn jthumsh...@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.re...@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

-- 
Johannes Thumshirn   Storage
jthumsh...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-30 Thread Sreekanth Reddy
Removed the redundancy code while freeing the controller resources.

Signed-off-by: Sreekanth Reddy sreekanth.re...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html