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@xxxxxxx> 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@xxxxxxx> 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@xxxxxxx> 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@xxxxxxxxxxxxx> >> >> >> --- >> >> >> 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@xxxxxxx +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@xxxxxxx +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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
