On 18.2.2016 17:55, Ewan Milne wrote:
> On Thu, 2016-02-18 at 11:40 -0500, Ewan Milne wrote:
>> It also appears to me upon further inspection that the existing code has
>> other problems. In particular, once mpt_mapresources() has returned
>> with a nonzero error code, it looks like iounmap() should be called, but
>> it is not in the case of a failed allocation of reset_work_q. I'm also
>> not sure why pci_release_selected_regions() is only called for the case
>> of mpt_do_ioc_recovery() returning != -5 when it is called whenever
>> there is a failed allocation of reset_work_q.
>>
>> Consider the attached patch (untested, because I don't have hardware):
>> It shows what I would do for labels & error handling. If the rc != -5
>> case of return from mpt_do_ioc_recovery() could be eliminated, then
>> another label "out_free_fw_event_q:" could be added prior to the other
>> error cases at the end, and all the code after the printk() in that path
>> could be replaced by "goto out_free_fw_event_q;"
>>
>> if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
>> CAN_SLEEP)) != 0){
>> printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
>> ioc->name, r);
>> goto out_free_fw_event_q;
>> }
>> ...
>>
>> out_free_fw_event_q:
>> destroy_workqueue(ioc->fw_event_q);
>> ioc->fw_event_q = NULL;
>>
>> out_remove_ioc:
>> ...
>>
>> However I do not know if that change is legitimate.
>>
>> -Ewan
> There is an error in the patch attached in my previous mail.
> Please refer to the attached PATCH v2 RFC.
Looks fine to me, do you we could add this change to your patch ?
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index a4f362b3b4..4a42c1534c 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -2012,6 +2012,8 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
if (ioc->alt_ioc)
ioc->alt_ioc->alt_ioc = NULL;
iounmap(ioc->memmap);
+ if (pci_is_enabled(pdev)
+ pci_disable_device(pdev);
if (r != -5)
pci_release_selected_regions(pdev, ioc->bars);
@@ -2019,7 +2021,6 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
ioc->reset_work_q = NULL;
kfree(ioc);
- pci_set_drvdata(pdev, NULL);
return r;
}
@@ -2052,13 +2053,13 @@ out_remove_ioc:
list_del(&ioc->list);
if (ioc->alt_ioc)
ioc->alt_ioc->alt_ioc = NULL;
- pci_set_drvdata(ioc->pcidev, NULL);
destroy_workqueue(ioc->reset_work_q);
ioc->reset_work_q = NULL;
out_unmap_resources:
iounmap(ioc->memmap);
+ pci_disable_device(pdev);
pci_release_selected_regions(pdev, ioc->bars);
out_free_ioc:
-tomash
>
> -Ewan
>
--
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
