Message ID | 20191120012859.23300-44-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 |
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 |
cc: Greg. On 20/11/2019 12:28, Oliver O'Halloran wrote: > The only thing we need the pdn for in this function is setting the pe_number > field, which we don't use anymore. Fix the weird refcounting behaviour while > we're here. > > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > Either Fred, or Reza also fixed this in some patch lately and that'll probably get > merged before this one does. > --- > arch/powerpc/platforms/powernv/pci-ioda.c | 27 +++++++++-------------- > 1 file changed, 10 insertions(+), 17 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 45d940730c30..2a9201306543 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -1066,16 +1066,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset) > static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) > { > struct pnv_phb *phb = pci_bus_to_pnvhb(dev->bus); > - struct pci_dn *pdn = pci_get_pdn(dev); > - struct pnv_ioda_pe *pe; > + struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev); > > - if (!pdn) { > - pr_err("%s: Device tree node not associated properly\n", > - pci_name(dev)); > + /* Already has a PE assigned? huh? */ > + if (pe) { > + WARN_ON(1); > return NULL; > } > - if (pdn->pe_number != IODA_INVALID_PE) > - return NULL; > > pe = pnv_ioda_alloc_pe(phb); > if (!pe) { > @@ -1084,29 +1081,25 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) > return NULL; > } > > - /* NOTE: We get only one ref to the pci_dev for the pdn, not for the > - * pointer in the PE data structure, both should be destroyed at the > - * same time. However, this needs to be looked at more closely again > - * once we actually start removing things (Hotplug, SR-IOV, ...) > + /* > + * NB: We **do not** hold a pci_dev ref for pe->pdev. > * > - * At some point we want to remove the PDN completely anyways > + * The pci_dev's release function cleans up the ioda_pe state, so: > + * a) We can't take a ref otherwise the release function is never called > + * b) The pe->pdev pointer will always point to valid pci_dev (or NULL) > */ > - pci_dev_get(dev); > - pdn->pe_number = pe->pe_number; > pe->flags = PNV_IODA_PE_DEV; > pe->pdev = dev; > pe->pbus = NULL; > pe->mve_number = -1; > - pe->rid = dev->bus->number << 8 | pdn->devfn; > + pe->rid = dev->bus->number << 8 | dev->devfn; > > pe_info(pe, "Associated device to PE\n"); > > if (pnv_ioda_configure_pe(phb, pe)) { > /* XXX What do we do here ? */ > pnv_ioda_free_pe(pe); > - pdn->pe_number = IODA_INVALID_PE; > pe->pdev = NULL; > - pci_dev_put(dev); > return NULL; > } > >
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 45d940730c30..2a9201306543 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -1066,16 +1066,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset) static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) { struct pnv_phb *phb = pci_bus_to_pnvhb(dev->bus); - struct pci_dn *pdn = pci_get_pdn(dev); - struct pnv_ioda_pe *pe; + struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev); - if (!pdn) { - pr_err("%s: Device tree node not associated properly\n", - pci_name(dev)); + /* Already has a PE assigned? huh? */ + if (pe) { + WARN_ON(1); return NULL; } - if (pdn->pe_number != IODA_INVALID_PE) - return NULL; pe = pnv_ioda_alloc_pe(phb); if (!pe) { @@ -1084,29 +1081,25 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev) return NULL; } - /* NOTE: We get only one ref to the pci_dev for the pdn, not for the - * pointer in the PE data structure, both should be destroyed at the - * same time. However, this needs to be looked at more closely again - * once we actually start removing things (Hotplug, SR-IOV, ...) + /* + * NB: We **do not** hold a pci_dev ref for pe->pdev. * - * At some point we want to remove the PDN completely anyways + * The pci_dev's release function cleans up the ioda_pe state, so: + * a) We can't take a ref otherwise the release function is never called + * b) The pe->pdev pointer will always point to valid pci_dev (or NULL) */ - pci_dev_get(dev); - pdn->pe_number = pe->pe_number; pe->flags = PNV_IODA_PE_DEV; pe->pdev = dev; pe->pbus = NULL; pe->mve_number = -1; - pe->rid = dev->bus->number << 8 | pdn->devfn; + pe->rid = dev->bus->number << 8 | dev->devfn; pe_info(pe, "Associated device to PE\n"); if (pnv_ioda_configure_pe(phb, pe)) { /* XXX What do we do here ? */ pnv_ioda_free_pe(pe); - pdn->pe_number = IODA_INVALID_PE; pe->pdev = NULL; - pci_dev_put(dev); return NULL; }
The only thing we need the pdn for in this function is setting the pe_number field, which we don't use anymore. Fix the weird refcounting behaviour while we're here. Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- Either Fred, or Reza also fixed this in some patch lately and that'll probably get merged before this one does. --- arch/powerpc/platforms/powernv/pci-ioda.c | 27 +++++++++-------------- 1 file changed, 10 insertions(+), 17 deletions(-)