diff mbox series

[Very,RFC,35/46] powernv/pci: Remove open-coded PE lookup in pnv_pci_release_device

Message ID 20191120012859.23300-36-oohall@gmail.com (mailing list archive)
State RFC
Headers show
Series [Very,RFC,01/46] powerpc/eeh: Don't attempt to restore VF config space after reset | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/merge (784eee1cc44801366d4f197e0ade7739ee8e1e83)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/next (0695f8bca93ea0c57f0e8e21b4b4db70183b3d1c)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linus/master (c74386d50fbaf4a54fd3fe560f1abc709c0cff4b)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/fixes (7d6475051fb3d9339c5c760ed9883bc0a9048b21)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linux-next (5d1131b4d61e52e5702e0fa4bcbec81ac7d6ef52)
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch

Commit Message

Oliver O'Halloran Nov. 20, 2019, 1:28 a.m. UTC
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Alexey Kardashevskiy Nov. 27, 2019, 5:24 a.m. UTC | #1
On 20/11/2019 12:28, Oliver O'Halloran wrote:
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 4f38652c7cd7..8525642b1256 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -3562,14 +3562,14 @@ static void pnv_ioda_release_pe(struct pnv_ioda_pe *pe)
>  static void pnv_pci_release_device(struct pci_dev *pdev)
>  {
>  	struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
> +	struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
>  	struct pci_dn *pdn = pci_get_pdn(pdev);
> -	struct pnv_ioda_pe *pe;
>  
>  	/* The VF PE state is torn down when sriov_disable() is called */
>  	if (pdev->is_virtfn)
>  		return;
>  
> -	if (!pdn || pdn->pe_number == IODA_INVALID_PE)
> +	if (WARN_ON(!pe))


Is that WARN_ON because there is always a PE - from upstream bridge or a
reserved one?



>  		return;
>  
>  	/*
> @@ -3588,7 +3588,6 @@ static void pnv_pci_release_device(struct pci_dev *pdev)
>  	 * be increased on adding devices. It leads to unbalanced PE's device
>  	 * count and eventually make normal PCI hotplug path broken.
>  	 */
> -	pe = &phb->ioda.pe_array[pdn->pe_number];
>  	pdn->pe_number = IODA_INVALID_PE;
>  
>  	WARN_ON(--pe->device_count < 0);
>
Oliver O'Halloran Nov. 27, 2019, 9:51 a.m. UTC | #2
On Wed, Nov 27, 2019 at 4:24 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>
>
> On 20/11/2019 12:28, Oliver O'Halloran wrote:
> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > ---
> >  arch/powerpc/platforms/powernv/pci-ioda.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> > index 4f38652c7cd7..8525642b1256 100644
> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > @@ -3562,14 +3562,14 @@ static void pnv_ioda_release_pe(struct pnv_ioda_pe *pe)
> >  static void pnv_pci_release_device(struct pci_dev *pdev)
> >  {
> >       struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
> > +     struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
> >       struct pci_dn *pdn = pci_get_pdn(pdev);
> > -     struct pnv_ioda_pe *pe;
> >
> >       /* The VF PE state is torn down when sriov_disable() is called */
> >       if (pdev->is_virtfn)
> >               return;
> >
> > -     if (!pdn || pdn->pe_number == IODA_INVALID_PE)
> > +     if (WARN_ON(!pe))
>
>
> Is that WARN_ON because there is always a PE - from upstream bridge or a

The device should always belong to a PE. If it doesn't (at this point)
then something deeply strange has happened.

> reserved one?

If it's associated with the reserved PE the rmap is set to
IODA_PE_INVALID, so would return NULL and we'd hit the WARN_ON(). I
think that's ok though since PE assignment should always succeed. If
it failed, or we're tearing down the device before we got to the point
of assigning a PE then there's probably a bug.
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 4f38652c7cd7..8525642b1256 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3562,14 +3562,14 @@  static void pnv_ioda_release_pe(struct pnv_ioda_pe *pe)
 static void pnv_pci_release_device(struct pci_dev *pdev)
 {
 	struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
+	struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
 	struct pci_dn *pdn = pci_get_pdn(pdev);
-	struct pnv_ioda_pe *pe;
 
 	/* The VF PE state is torn down when sriov_disable() is called */
 	if (pdev->is_virtfn)
 		return;
 
-	if (!pdn || pdn->pe_number == IODA_INVALID_PE)
+	if (WARN_ON(!pe))
 		return;
 
 	/*
@@ -3588,7 +3588,6 @@  static void pnv_pci_release_device(struct pci_dev *pdev)
 	 * be increased on adding devices. It leads to unbalanced PE's device
 	 * count and eventually make normal PCI hotplug path broken.
 	 */
-	pe = &phb->ioda.pe_array[pdn->pe_number];
 	pdn->pe_number = IODA_INVALID_PE;
 
 	WARN_ON(--pe->device_count < 0);