diff mbox series

[for-rc,1/2] PCI: Fix faulty logic in pci_reset_bus()

Message ID 20180831173359.21741.61944.stgit@scvm10.sc.intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series IB/hfi1: PCI bug due to pci core changes | expand

Commit Message

Dennis Dalessandro Aug. 31, 2018, 5:34 p.m. UTC
The pci_rest_bus() function calls into pci_probe_reset_slot() to determine
whether to call the slot or bus reset. The check has faulty logic in that
it does not account for pci_probe_reset_slot() being able to return an
errno. Fix by only calling the slot reset when the function returns 0.
Treat < 1 and > 1 the same.

Cc: Sinan Kaya <okaya@codeaurora.org>
Fixes: 811c5cb37df4 ("PCI: Unify try slot and bus reset API")
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/pci/pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Sinan Kaya Aug. 31, 2018, 5:58 p.m. UTC | #1
On 8/31/2018 10:34 AM, Dennis Dalessandro wrote:
> The pci_rest_bus() function calls into pci_probe_reset_slot() to determine
> whether to call the slot or bus reset. The check has faulty logic in that
> it does not account for pci_probe_reset_slot() being able to return an
> errno. Fix by only calling the slot reset when the function returns 0.
> Treat < 1 and > 1 the same.
> 
> Cc: Sinan Kaya<okaya@codeaurora.org>
> Fixes: 811c5cb37df4 ("PCI: Unify try slot and bus reset API")
> Reviewed-by: Michael J. Ruhl<michael.j.ruhl@intel.com>
> Signed-off-by: Dennis Dalessandro<dennis.dalessandro@intel.com>

Nit. Small typo on the first sentence (pci_rest_bus()).

Reviewed-by: Sinan Kaya <okaya@kernel.org>
Jason Gunthorpe Sept. 4, 2018, 8:59 p.m. UTC | #2
On Fri, Aug 31, 2018 at 10:58:32AM -0700, Sinan Kaya wrote:
> On 8/31/2018 10:34 AM, Dennis Dalessandro wrote:
> > The pci_rest_bus() function calls into pci_probe_reset_slot() to determine
> > whether to call the slot or bus reset. The check has faulty logic in that
> > it does not account for pci_probe_reset_slot() being able to return an
> > errno. Fix by only calling the slot reset when the function returns 0.
> > Treat < 1 and > 1 the same.
> > 
> > Cc: Sinan Kaya<okaya@codeaurora.org>
> > Fixes: 811c5cb37df4 ("PCI: Unify try slot and bus reset API")
> > Reviewed-by: Michael J. Ruhl<michael.j.ruhl@intel.com>
> > Signed-off-by: Dennis Dalessandro<dennis.dalessandro@intel.com>
> 
> Nit. Small typo on the first sentence (pci_rest_bus()).
> 
> Reviewed-by: Sinan Kaya <okaya@kernel.org>

Bjorn,

Are you OK to apply this series through the RDMA tree (for rc3), or do
you want to take it through PCI?

https://patchwork.kernel.org/patch/10584277/

Thanks,
Jason
Sinan Kaya Sept. 4, 2018, 9:16 p.m. UTC | #3
On 9/4/2018 1:59 PM, Jason Gunthorpe wrote:
> On Fri, Aug 31, 2018 at 10:58:32AM -0700, Sinan Kaya wrote:
>> On 8/31/2018 10:34 AM, Dennis Dalessandro wrote:
>>> The pci_rest_bus() function calls into pci_probe_reset_slot() to determine
>>> whether to call the slot or bus reset. The check has faulty logic in that
>>> it does not account for pci_probe_reset_slot() being able to return an
>>> errno. Fix by only calling the slot reset when the function returns 0.
>>> Treat < 1 and > 1 the same.
>>>
>>> Cc: Sinan Kaya<okaya@codeaurora.org>
>>> Fixes: 811c5cb37df4 ("PCI: Unify try slot and bus reset API")
>>> Reviewed-by: Michael J. Ruhl<michael.j.ruhl@intel.com>
>>> Signed-off-by: Dennis Dalessandro<dennis.dalessandro@intel.com>
>>
>> Nit. Small typo on the first sentence (pci_rest_bus()).
>>
>> Reviewed-by: Sinan Kaya <okaya@kernel.org>
> 
> Bjorn,
> 
> Are you OK to apply this series through the RDMA tree (for rc3), or do
> you want to take it through PCI?
> 
> https://patchwork.kernel.org/patch/10584277/

Please don't apply the entire series yet. First patch is good to go.

Second one is a hack. We are trying to find a better solution for the second
patch.

https://bugzilla.kernel.org/show_bug.cgi?id=200985


> 
> Thanks,
> Jason
>
Jason Gunthorpe Sept. 4, 2018, 9:30 p.m. UTC | #4
On Tue, Sep 04, 2018 at 02:16:13PM -0700, Sinan Kaya wrote:
> On 9/4/2018 1:59 PM, Jason Gunthorpe wrote:
> > On Fri, Aug 31, 2018 at 10:58:32AM -0700, Sinan Kaya wrote:
> > > On 8/31/2018 10:34 AM, Dennis Dalessandro wrote:
> > > > The pci_rest_bus() function calls into pci_probe_reset_slot() to determine
> > > > whether to call the slot or bus reset. The check has faulty logic in that
> > > > it does not account for pci_probe_reset_slot() being able to return an
> > > > errno. Fix by only calling the slot reset when the function returns 0.
> > > > Treat < 1 and > 1 the same.
> > > > 
> > > > Cc: Sinan Kaya<okaya@codeaurora.org>
> > > > Fixes: 811c5cb37df4 ("PCI: Unify try slot and bus reset API")
> > > > Reviewed-by: Michael J. Ruhl<michael.j.ruhl@intel.com>
> > > > Signed-off-by: Dennis Dalessandro<dennis.dalessandro@intel.com>
> > > 
> > > Nit. Small typo on the first sentence (pci_rest_bus()).
> > > 
> > > Reviewed-by: Sinan Kaya <okaya@kernel.org>
> > 
> > Bjorn,
> > 
> > Are you OK to apply this series through the RDMA tree (for rc3), or do
> > you want to take it through PCI?
> > 
> > https://patchwork.kernel.org/patch/10584277/
> 
> Please don't apply the entire series yet. First patch is good to go.
> 
> Second one is a hack. We are trying to find a better solution for the second
> patch.

Don't expect me to follow bugzilla too :|

I'll drop this series off patchworks then, resend when you have something..

Jason
Sinan Kaya Sept. 4, 2018, 9:50 p.m. UTC | #5
On 9/4/2018 2:30 PM, Jason Gunthorpe wrote:
> On Tue, Sep 04, 2018 at 02:16:13PM -0700, Sinan Kaya wrote:
>> On 9/4/2018 1:59 PM, Jason Gunthorpe wrote:
>>> On Fri, Aug 31, 2018 at 10:58:32AM -0700, Sinan Kaya wrote:
>>>> On 8/31/2018 10:34 AM, Dennis Dalessandro wrote:
>>>>> The pci_rest_bus() function calls into pci_probe_reset_slot() to determine
>>>>> whether to call the slot or bus reset. The check has faulty logic in that
>>>>> it does not account for pci_probe_reset_slot() being able to return an
>>>>> errno. Fix by only calling the slot reset when the function returns 0.
>>>>> Treat < 1 and > 1 the same.
>>>>>
>>>>> Cc: Sinan Kaya<okaya@codeaurora.org>
>>>>> Fixes: 811c5cb37df4 ("PCI: Unify try slot and bus reset API")
>>>>> Reviewed-by: Michael J. Ruhl<michael.j.ruhl@intel.com>
>>>>> Signed-off-by: Dennis Dalessandro<dennis.dalessandro@intel.com>
>>>>
>>>> Nit. Small typo on the first sentence (pci_rest_bus()).
>>>>
>>>> Reviewed-by: Sinan Kaya <okaya@kernel.org>
>>>
>>> Bjorn,
>>>
>>> Are you OK to apply this series through the RDMA tree (for rc3), or do
>>> you want to take it through PCI?
>>>
>>> https://patchwork.kernel.org/patch/10584277/
>>
>> Please don't apply the entire series yet. First patch is good to go.
>>
>> Second one is a hack. We are trying to find a better solution for the second
>> patch.
> 
> Don't expect me to follow bugzilla too :|
> 
> I'll drop this series off patchworks then, resend when you have something..
> 

Sure, I'm trying to contain the change to PCI tree only. Hopefully, it can go
  through Bjorn's tree. If we fail, we are back to this change. I'll ping you in
  that case.

> Jason
>
diff mbox series

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 29ff961..30b2603 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5200,7 +5200,7 @@  static int __pci_reset_bus(struct pci_bus *bus)
  */
 int pci_reset_bus(struct pci_dev *pdev)
 {
-	return pci_probe_reset_slot(pdev->slot) ?
+	return (!pci_probe_reset_slot(pdev->slot)) ?
 	    __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus);
 }
 EXPORT_SYMBOL_GPL(pci_reset_bus);