| Message ID | 20180831173359.21741.61944.stgit@scvm10.sc.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | IB/hfi1: PCI bug due to pci core changes | expand |
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>
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
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 >
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
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 --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);
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(-)