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@xxxxxxxxx> 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?
