| Message ID | 20200819130741.16769-1-fbarrat@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e17a7c0e0aebb956719ce2a8465f649859c2da7d |
| Headers | show |
| Series | powerpc/powernv/pci: Fix typo when releasing DMA resources | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (d4ecce4dcc8f8820286cf4e0859850c555e89854) |
| snowpatch_ozlabs/build-ppc64le | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/build-ppc64be | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/build-ppc64e | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/build-pmac32 | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch fixes a commit that hasn't been released yet |
On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat <fbarrat@linux.ibm.com> wrote: > > Fix typo introduced during recent code cleanup, which could lead to > silently not freeing resources or oops message (on PCI hotplug or CAPI > reset). oof Did you actually hit that oops on CAPI reset? Including the stack trace is helpful for avoiding this sort of problem in the future. Anyway, Reviewed-by: Oliver O'Halloran <oohall@gmail.com> > Only impacts ioda2, the code path for ioda1 is correct. yeah but no ones uses ioda1 > Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA setup state") > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > --- > arch/powerpc/platforms/powernv/pci-ioda.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index c9c25fb0783c..023a4f987bb2 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe) > struct iommu_table *tbl = pe->table_group.tables[0]; > int64_t rc; > > - if (pe->dma_setup_done) > + if (!pe->dma_setup_done) > return; > > rc = pnv_pci_ioda2_unset_window(&pe->table_group, 0); > -- > 2.26.2 >
Le 19/08/2020 à 15:15, Oliver O'Halloran a écrit : > On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat <fbarrat@linux.ibm.com> wrote: >> >> Fix typo introduced during recent code cleanup, which could lead to >> silently not freeing resources or oops message (on PCI hotplug or CAPI >> reset). > > oof > > Did you actually hit that oops on CAPI reset? Including the stack > trace is helpful for avoiding this sort of problem in the future. > Anyway, yeah, I found it with capi reset. It's actually not a oops, we hit the WARN_ON in iommu_tce_table_put(), when we try to release the DMA resource for a PE where it was never configured to start with. So that was easy to track. Fred > Reviewed-by: Oliver O'Halloran <oohall@gmail.com> > >> Only impacts ioda2, the code path for ioda1 is correct. > > yeah but no ones uses ioda1 > >> Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA setup state") >> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> >> --- >> arch/powerpc/platforms/powernv/pci-ioda.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >> index c9c25fb0783c..023a4f987bb2 100644 >> --- a/arch/powerpc/platforms/powernv/pci-ioda.c >> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c >> @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe) >> struct iommu_table *tbl = pe->table_group.tables[0]; >> int64_t rc; >> >> - if (pe->dma_setup_done) >> + if (!pe->dma_setup_done) >> return; >> >> rc = pnv_pci_ioda2_unset_window(&pe->table_group, 0); >> -- >> 2.26.2 >>
Frederic Barrat <fbarrat@linux.ibm.com> writes: > Fix typo introduced during recent code cleanup, which could lead to > silently not freeing resources or oops message (on PCI hotplug or CAPI > reset). > Only impacts ioda2, the code path for ioda1 is correct. > > Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA setup state") > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> I changed the subject to: powerpc/powernv/pci: Fix possible crash when releasing DMA resources To hopefully better convey that it's a moderately serious bug, even if the root cause is just a typo. Otherwise folks scanning the commit log might think it's just a harmless typo. cheers > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index c9c25fb0783c..023a4f987bb2 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe) > struct iommu_table *tbl = pe->table_group.tables[0]; > int64_t rc; > > - if (pe->dma_setup_done) > + if (!pe->dma_setup_done) > return; > > rc = pnv_pci_ioda2_unset_window(&pe->table_group, 0); > -- > 2.26.2
Le 20/08/2020 à 06:18, Michael Ellerman a écrit : > I changed the subject to: > > powerpc/powernv/pci: Fix possible crash when releasing DMA resources Much better, thanks! Fred
On Wed, 19 Aug 2020 15:07:41 +0200, Frederic Barrat wrote: > Fix typo introduced during recent code cleanup, which could lead to > silently not freeing resources or oops message (on PCI hotplug or CAPI > reset). > Only impacts ioda2, the code path for ioda1 is correct. Applied to powerpc/fixes. [1/1] powerpc/powernv/pci: Fix possible crash when releasing DMA resources https://git.kernel.org/powerpc/c/e17a7c0e0aebb956719ce2a8465f649859c2da7d cheers
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index c9c25fb0783c..023a4f987bb2 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe) struct iommu_table *tbl = pe->table_group.tables[0]; int64_t rc; - if (pe->dma_setup_done) + if (!pe->dma_setup_done) return; rc = pnv_pci_ioda2_unset_window(&pe->table_group, 0);
Fix typo introduced during recent code cleanup, which could lead to silently not freeing resources or oops message (on PCI hotplug or CAPI reset). Only impacts ioda2, the code path for ioda1 is correct. Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA setup state") Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> --- arch/powerpc/platforms/powernv/pci-ioda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)