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?