[Very RFC 22/46] powernv/eeh: Allocate eeh_dev's when needed

Oliver O'Halloran oohall at gmail.com
Mon Nov 25 15:26:51 AEDT 2019


On Mon, Nov 25, 2019 at 2:27 PM Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
>
>
>
> On 20/11/2019 12:28, Oliver O'Halloran wrote:
> > Have the PowerNV EEH backend allocate the eeh_dev if needed rather than using
> > the one attached to the pci_dn.
>
> So that pci_dn attached one is leaked then?

Sorta, the eeh_dev attached to the pci_dn is supposed to have the same
lifetime as the pci_dn it's attached to. Whatever frees the pci_dn
should also be freeing the eeh_dev, but I'm pretty sure the only
situation where that actually happens is when removing the pci_dn for
VFs. It's bad.

> > This gets us most of the way towards decoupling
> > pci_dn from the PowerNV EEH code.
> >
> > Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> > ---
> > We should probably be free()ing the eeh_dev somewhere. The pci_dev release
> > function is the right place for it.
> > ---
> >  arch/powerpc/platforms/powernv/eeh-powernv.c | 22 ++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > index 1cd80b399995..7aba18e08996 100644
> > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > @@ -366,10 +366,9 @@ static int pnv_eeh_write_config(struct eeh_dev *edev,
> >   */
> >  static struct eeh_dev *pnv_eeh_probe_pdev(struct pci_dev *pdev)
> >  {
> > -     struct pci_dn *pdn = pci_get_pdn(pdev);
> > -     struct pci_controller *hose = pdn->phb;
> > -     struct pnv_phb *phb = hose->private_data;
> > -     struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> > +     struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
> > +     struct pci_controller *hose = phb->hose;
> > +     struct eeh_dev *edev;
> >       uint32_t pcie_flags;
> >       int ret;
> >       int config_addr = (pdev->bus->number << 8) | (pdev->devfn);
> > @@ -415,12 +414,27 @@ static struct eeh_dev *pnv_eeh_probe_pdev(struct pci_dev *pdev)
> >       if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
> >               return NULL;
> >
> > +     /* otherwise allocate and initialise a new eeh_dev */
> > +     edev = kzalloc(sizeof(*edev), GFP_KERNEL);
> > +     if (!edev) {
> > +             pr_err("%s: out of memory lol\n", __func__);
>
> "lol"?

yeah lol

I am pretty sure we do not have to print anything if alloc failed
> as alloc prints an error anyway. Thanks,

It does? Neat.


More information about the Linuxppc-dev mailing list