Re: [RFC PATCH v3 06/16] cxl/mem: Find device capabilities

From: Ben Widawsky
Date: Tue Jan 12 2021 - 14:23:52 EST


On 21-01-12 19:17:44, Jonathan Cameron wrote:
> On Mon, 11 Jan 2021 14:51:10 -0800
> Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote:
>
> > CXL devices contain an array of capabilities that describe the
> > interactions software can have with the device or firmware running on
> > the device. A CXL compliant device must implement the device status and
> > the mailbox capability. A CXL compliant memory device must implement the
> > memory device capability.
> >
> > Each of the capabilities can [will] provide an offset within the MMIO
> > region for interacting with the CXL device.
> >
> > For more details see 8.2.8 of the CXL 2.0 specification.
> >
> > Link: Link: https://www.computeexpresslink.org/download-the-specification
> > Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx>
>
>
> ...
>
> > /**
> > * cxl_mem_create() - Create a new &struct cxl_mem.
> > * @pdev: The pci device associated with the new &struct cxl_mem.
> > @@ -129,8 +214,20 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > if (rc)
> > return rc;
> >
> > + rc = cxl_mem_setup_regs(cxlm);
> > + if (rc)
> > + goto err;
> > +
> > + rc = cxl_mem_setup_mailbox(cxlm);
> > + if (rc)
> > + goto err;
> > +
> > pci_set_drvdata(pdev, cxlm);
> > return 0;
> > +
> > +err:
> > + kfree(cxlm);
>
> From previous patch that was created using devm_kzalloc in which case
> this free just introduced a double free if you hit this error path.
> Having go rid of that you can do direct returns instead of goto.

Ah okay. Ignore my last email then. I admit I looked really hard to figure out
how it ultimately gets freed and couldn't convince myself it does.

If someone more familiar with devm says it just works, then I'll rip out all of
remove (as it was in v2).

Dan did fix this all up for me previously and I regretfully undid it.

>
>
> > + return rc;
> > }
> >
> > static void cxl_mem_remove(struct pci_dev *pdev)
>