Hello Tomas,
> -----Original Message-----
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Monday, December 7, 2015 6:06 AM
> To: Raghava Aditya Renukunta; jbottom...@parallels.com; linux-
> s...@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran;
> aacr...@pmc-sierra.com; Rich Bono
> Subject: Re: [PATCH 04/10] aacraid: Fix memory leak in aac_fib_map_free
>
> On 5.12.2015 01:40, Raghava Aditya Renukunta wrote:
> > Hello Tomas,
> >
> >
> >> -----Original Message-----
> >> From: Tomas Henzl [mailto:the...@redhat.com]
> >> Sent: Friday, December 4, 2015 6:35 AM
> >> To: Raghava Aditya Renukunta; jbottom...@parallels.com; linux-
> >> s...@vger.kernel.org
> >> Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran;
> >> aacr...@pmc-sierra.com; Rich Bono
> >> Subject: Re: [PATCH 04/10] aacraid: Fix memory leak in aac_fib_map_free
> >>
> >> On 1.12.2015 13:39, Raghava Aditya Renukunta wrote:
> >>> From: Raghava Aditya Renukunta
> <raghavaaditya.renuku...@pmcs.com>
> >>>
> >>> aac_fib_map_free() calls pci_free_consistent() without checking that
> >>> dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they
> >>> are indeed NULL/0, this will result in a hang as pci_free_consistent()
> >>> will attempt to invalidate cache for the entire 64-bit address space
> >>> (which would take a very long time).
> >>>
> >>> Fixed by adding a check to make sure that dev->hw_fib_va and
> >>> dev->max_fib_size are not NULL and 0 respectively.
> >>>
> >>> Signed-off-by: Raghava Aditya Renukunta
> >> <raghavaaditya.renuku...@pmcs.com>
> >>
> >> Reviewed-by: Tomas Henzl <the...@redhat.com>
> >>
> >> Is the can_queue constant during the driver's life, or is it possible
> >> to manipulate it (aac_change_queue_depth)?
> >>
> >> Tomas
> > can_queue is only changed in aac_init_adapter. Do you want to save
> > (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) in a variable
> > So that the whole can_queue dereference need not be used?
>
> It's fine as it is, (I thought it may change elsewhere in your code
> but now I think that I was wrong).
>
> --tm
I will leave it as it is then.
>
> >
> > Regards,
> > Raghava Aditya
> >
> >>> ---
> >>> drivers/scsi/aacraid/commsup.c | 9 ++++++---
> >>> 1 file changed, 6 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/aacraid/commsup.c
> >> b/drivers/scsi/aacraid/commsup.c
> >>> index b257d3b..9533f47 100644
> >>> --- a/drivers/scsi/aacraid/commsup.c
> >>> +++ b/drivers/scsi/aacraid/commsup.c
> >>> @@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
> >>>
> >>> void aac_fib_map_free(struct aac_dev *dev)
> >>> {
> >>> - pci_free_consistent(dev->pdev,
> >>> - dev->max_fib_size * (dev->scsi_host_ptr->can_queue +
> >> AAC_NUM_MGT_FIB),
> >>> - dev->hw_fib_va, dev->hw_fib_pa);
> >>> + if (dev->hw_fib_va && dev->max_fib_size) {
> >>> + pci_free_consistent(dev->pdev,
> >>> + (dev->max_fib_size *
> >>> + (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
> >>> + dev->hw_fib_va, dev->hw_fib_pa);
> >>> + }
> >>> dev->hw_fib_va = NULL;
> >>> dev->hw_fib_pa = 0;
> >>> }
> > --
> > 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
--
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
