Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources

From: Frederic Barrat <fbarrat@linux.ibm.com>
To: "Oliver O'Halloran" <oohall@gmail.com>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
Date: Wed, 19 Aug 2020 15:31:29 +0200	[thread overview]
Message-ID: <75d99821-ee3d-137e-4e56-b2427f8721da@linux.ibm.com> (raw)
In-Reply-To: <CAOSf1CG=ecx53ySsmX-7pK_zUJsd9LRMX5V1HBhYZBcOT2z89w@mail.gmail.com>

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
>>