Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751975AbdLNMOa (ORCPT ); Thu, 14 Dec 2017 07:14:30 -0500 Received: from fllnx209.ext.ti.com ([198.47.19.16]:20521 "EHLO fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751729AbdLNMO2 (ORCPT ); Thu, 14 Dec 2017 07:14:28 -0500 Subject: Re: [PATCH v4 2/3] PCI: endpoint: Fix error handling in pci_epc_epf_link() To: Lorenzo Pieralisi References: <20171212141634.5985-1-niklas.cassel@axis.com> <20171212141634.5985-3-niklas.cassel@axis.com> <691e3693-9f18-6546-29d0-4cb155ef5bf6@ti.com> <20171214120757.GB27147@e107981-ln.cambridge.arm.com> CC: Niklas Cassel , Bjorn Helgaas , Niklas Cassel , , From: Kishon Vijay Abraham I Message-ID: <8eda75e3-f4e4-151e-5104-69ba2e40a31f@ti.com> Date: Thu, 14 Dec 2017 17:43:33 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20171214120757.GB27147@e107981-ln.cambridge.arm.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1645 Lines: 42 On Thursday 14 December 2017 05:37 PM, Lorenzo Pieralisi wrote: > On Thu, Dec 14, 2017 at 04:37:22PM +0530, Kishon Vijay Abraham I wrote: >> Hi Niklas, >> >> On Tuesday 12 December 2017 07:46 PM, Niklas Cassel wrote: >>> The error handling in pci_epc_epf_link() is broken, >>> since the clean up code for pci_epc_add_epf() calls clear_bit(), >>> even though the matching set_bit() is done after pci_epc_add_epf(). >>> >>> Also, clear_bit() should be done before pci_epc_remove_epf(), >>> since clean up code should normally do things in the reverse order. >>> >>> Fixes: d74679911610 ("PCI: endpoint: Introduce configfs entry for configuring EP functions") >>> Signed-off-by: Niklas Cassel >>> Acked-by: Lorenzo Pieralisi >>> --- >>> drivers/pci/endpoint/pci-ep-cfs.c | 6 ++---- >>> 1 file changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c >>> index 4f74386c1ced..e1f5adc9e113 100644 >>> --- a/drivers/pci/endpoint/pci-ep-cfs.c >>> +++ b/drivers/pci/endpoint/pci-ep-cfs.c >>> @@ -106,7 +106,7 @@ static int pci_epc_epf_link(struct config_item *epc_item, >>> epf = epf_group->epf; >>> ret = pci_epc_add_epf(epc, epf); >>> if (ret) >>> - goto err_add_epf; >>> + return ret; >> >> Actually the func_no should be populated before invoking pci_epc_add_epf. Once >> that is done, the error handling should be fine. > > Which means that current code works because pci_epc_add_epf() is called > with epf->func_no == 0 right ? I agree that the correct fix consists in that's right Lorenzo. Thanks Kishon