Re: [PATCH v3 01/14] cxl/region: Add region creation ABI

[Ben Widawsky <ben.widawsky@intel.com>]

On 22-02-02 10:26:06, Ben Widawsky wrote:
> On 22-01-28 10:59:26, Dan Williams wrote:
> > On Fri, Jan 28, 2022 at 10:14 AM Dan Williams <dan.j.williams@intel.com> wrote:
> > [..]
> > > Here is that put_device() I was expecting, that kfree() earlier was a
> > > double-free it seems.
> > >
> > > Also, I would have expected a devm action to remove this. Something like:
> > >
> > > struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> > >
> > > cxl_device_lock(&port->dev);
> > > if (port->dev.driver)
> > >     devm_cxl_add_region(port->uport, cxld, id);
> 
> I assume you mean devm_cxl_delete_region(), yes?
> 
> > > else
> > >     rc = -ENXIO;
> > > cxl_device_unlock(&port->dev);
> > >
> > > ...then no matter what you know the region will be unregistered when
> > > the root port goes away.
> > 
> > ...actually, the lock and ->dev.driver check here are not needed
> > because this attribute is only registered while the cxl_acpi driver is
> > bound. So, it is safe to assume this is protected as decoder remove
> > synchronizes against active sysfs users.
> 
> I'm somewhat confused when you say devm action to remove this. The current auto
> region deletion happens when the ->release() is called. Are you suggesting when
> the root decoder is removed I delete the regions at that point?

Hmm. I went back and looked and I had changed this functionality at some
point... So forget I said that, it isn't how it's working currently. But the
question remains, are you suggesting I delete in the root decoder
unregistration?