RE: [PATCH 05/16] aacraid: Fix memory leak in fib init path
> -Original Message- > From: Johannes Thumshirn [mailto:jthumsh...@suse.de] > Sent: Wednesday, February 15, 2017 12:32 AM > To: Raghava Aditya Renukunta > <raghavaaditya.renuku...@microsemi.com>; j...@linux.vnet.ibm.com; > martin.peter...@oracle.com; linux-scsi@vger.kernel.org > Cc: Dave Carroll <david.carr...@microsemi.com>; Gana Sridaran > <gana.srida...@microsemi.com>; Scott Benesh > <scott.ben...@microsemi.com>; dan.carpen...@oracle.com > Subject: Re: [PATCH 05/16] aacraid: Fix memory leak in fib init path > > EXTERNAL EMAIL > > > On 02/14/2017 09:44 PM, Raghava Aditya Renukunta wrote: > > aac_fib_map_free frees misaligned fib dma memory, additionally it does > not > > free up the whole memory. > > > > Fixed by changing the code to free up the correct and full memory > > allocation. > > > > Cc: sta...@vger.kernel.org > > Fixes: e8b12f0fb835223 ([SCSI] aacraid: Add new code for PMC-Sierra's SRC > based controller family) > > Signed-off-by: Raghava Aditya Renukunta > <raghavaaditya.renuku...@microsemi.com> > > Reviewed-by: David Carroll <david.carr...@microsemi.com> > > --- > > drivers/scsi/aacraid/commsup.c | 20 +--- > > 1 file changed, 9 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/scsi/aacraid/commsup.c > b/drivers/scsi/aacraid/commsup.c > > index f7a3bcb..863c98d 100644 > > --- a/drivers/scsi/aacraid/commsup.c > > +++ b/drivers/scsi/aacraid/commsup.c > > @@ -97,8 +97,8 @@ void aac_fib_map_free(struct aac_dev *dev) > > { > > if (dev->hw_fib_va && dev->max_cmd_size) { > > pci_free_consistent(dev->pdev, > > - (dev->max_cmd_size * > > - (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)), > > + (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) > > + * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) + 31, > > dev->hw_fib_va, dev->hw_fib_pa); > > Can you please do something like: > > size_t alloc_size; > int numtags; > > numtags = dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB; > alloc_size = (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) * > numtags + 31; > pci_free_consistent(dev->pdev, alloc_size, dev->hw_fib_va, > dev->hw_fib_pa); > > And please indent correctly. If it indentation doesn't work correctly > because you hit the 80 chars limit, that's a sign something should be > reconsidered. Yes, I will rework this. Regards, Raghava Aditya > Thanks, > Johannes > -- > Johannes Thumshirn Storage > jthumsh...@suse.de+49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH 05/16] aacraid: Fix memory leak in fib init path
On 02/14/2017 09:44 PM, Raghava Aditya Renukunta wrote: > aac_fib_map_free frees misaligned fib dma memory, additionally it does not > free up the whole memory. > > Fixed by changing the code to free up the correct and full memory > allocation. > > Cc: sta...@vger.kernel.org > Fixes: e8b12f0fb835223 ([SCSI] aacraid: Add new code for PMC-Sierra's SRC > based controller family) > Signed-off-by: Raghava Aditya Renukunta >> Reviewed-by: David Carroll > --- > drivers/scsi/aacraid/commsup.c | 20 +--- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c > index f7a3bcb..863c98d 100644 > --- a/drivers/scsi/aacraid/commsup.c > +++ b/drivers/scsi/aacraid/commsup.c > @@ -97,8 +97,8 @@ void aac_fib_map_free(struct aac_dev *dev) > { > if (dev->hw_fib_va && dev->max_cmd_size) { > pci_free_consistent(dev->pdev, > - (dev->max_cmd_size * > - (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)), > + (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) > + * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) + 31, > dev->hw_fib_va, dev->hw_fib_pa); Can you please do something like: size_t alloc_size; int numtags; numtags = dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB; alloc_size = (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) * numtags + 31; pci_free_consistent(dev->pdev, alloc_size, dev->hw_fib_va, dev->hw_fib_pa); And please indent correctly. If it indentation doesn't work correctly because you hit the 80 chars limit, that's a sign something should be reconsidered. Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH 05/16] aacraid: Fix memory leak in fib init path
aac_fib_map_free frees misaligned fib dma memory, additionally it does not free up the whole memory. Fixed by changing the code to free up the correct and full memory allocation. Cc: sta...@vger.kernel.org Fixes: e8b12f0fb835223 ([SCSI] aacraid: Add new code for PMC-Sierra's SRC based controller family) Signed-off-by: Raghava Aditya RenukuntaReviewed-by: David Carroll --- drivers/scsi/aacraid/commsup.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c index f7a3bcb..863c98d 100644 --- a/drivers/scsi/aacraid/commsup.c +++ b/drivers/scsi/aacraid/commsup.c @@ -97,8 +97,8 @@ void aac_fib_map_free(struct aac_dev *dev) { if (dev->hw_fib_va && dev->max_cmd_size) { pci_free_consistent(dev->pdev, - (dev->max_cmd_size * - (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)), + (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) + * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) + 31, dev->hw_fib_va, dev->hw_fib_pa); } dev->hw_fib_va = NULL; @@ -153,22 +153,20 @@ int aac_fib_setup(struct aac_dev * dev) if (i<0) return -ENOMEM; - /* 32 byte alignment for PMC */ - hw_fib_pa = (dev->hw_fib_pa + (ALIGN32 - 1)) & ~(ALIGN32 - 1); - dev->hw_fib_va = (struct hw_fib *)((unsigned char *)dev->hw_fib_va + - (hw_fib_pa - dev->hw_fib_pa)); - dev->hw_fib_pa = hw_fib_pa; memset(dev->hw_fib_va, 0, (dev->max_cmd_size + sizeof(struct aac_fib_xporthdr)) * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)); + /* 32 byte alignment for PMC */ + hw_fib_pa = (dev->hw_fib_pa + (ALIGN32 - 1)) & ~(ALIGN32 - 1); + hw_fib= (struct hw_fib *)((unsigned char *)dev->hw_fib_va + + (hw_fib_pa - dev->hw_fib_pa)); + /* add Xport header */ - dev->hw_fib_va = (struct hw_fib *)((unsigned char *)dev->hw_fib_va + + hw_fib = (struct hw_fib *)((unsigned char *)hw_fib + sizeof(struct aac_fib_xporthdr)); - dev->hw_fib_pa += sizeof(struct aac_fib_xporthdr); + hw_fib_pa += sizeof(struct aac_fib_xporthdr); - hw_fib = dev->hw_fib_va; - hw_fib_pa = dev->hw_fib_pa; /* * Initialise the fibs */ -- 2.7.4