From patchwork Fri Feb 19 13:46:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomas Henzl X-Patchwork-Id: 8361281 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3BB39C0553 for ; Fri, 19 Feb 2016 13:47:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6328020443 for ; Fri, 19 Feb 2016 13:47:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6F6C32042B for ; Fri, 19 Feb 2016 13:47:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1426749AbcBSNqv (ORCPT ); Fri, 19 Feb 2016 08:46:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57074 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1426165AbcBSNqa (ORCPT ); Fri, 19 Feb 2016 08:46:30 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id CF7553107; Fri, 19 Feb 2016 13:46:29 +0000 (UTC) Received: from dhcp-26-122.brq.redhat.com (dhcp-26-122.brq.redhat.com [10.34.26.122]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1JDkQIj024085; Fri, 19 Feb 2016 08:46:26 -0500 Subject: Re: [PATCH v2] fusion-mptbase: handle failed allocation for workqueue To: emilne@redhat.com, Johannes Thumshirn References: <1455770459-905-1-git-send-email-wuninsu@gmail.com> <20160218090043.GQ18134@c203.arch.suse.de> <1455813615.22112.473.camel@localhost.localdomain> <1455814515.22112.476.camel@localhost.localdomain> Cc: Insu Yun , nagalakshmi.nandigama@avagotech.com, praveen.krishnamoorthy@avagotech.com, sreekanth.reddy@avagotech.com, abhijit.mahajan@avagotech.com, MPT-FusionLinux.pdl@avagotech.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, taesoo@gatech.edu, yeongjin.jang@gatech.edu, insu@gatech.edu, changwoo@gatech.edu From: Tomas Henzl Message-ID: <56C71CB1.9040500@redhat.com> Date: Fri, 19 Feb 2016 14:46:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <1455814515.22112.476.camel@localhost.localdomain> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 ? -tomash > > -Ewan > --- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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: