On Mon, Nov 25, 2019 at 2:27 PM Alexey Kardashevskiy <a...@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 <ooh...@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.
