Re: [PATCH v6 03/12] cxl/pmem: Refactor nvdimm device registration, delete the workqueue

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 01 Dec 2022 13:33:37 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> The three objects 'struct cxl_nvdimm_bridge', 'struct cxl_nvdimm', and
> 'struct cxl_pmem_region' manage CXL persistent memory resources. The
> bridge represents base platform resources, the nvdimm represents one or
> more endpoints, and the region is a collection of nvdimms that
> contribute to an assembled address range.
> 
> Their relationship is such that a region is torn down if any component
> endpoints are removed. All regions and endpoints are torn down if the
> foundational bridge device goes down.
> 
> A workqueue was deployed to manage these interdependencies, but it is
> difficult to reason about, and fragile. A recent attempt to take the CXL
> root device lock in the cxl_mem driver was reported by lockdep as
> colliding with the flush_work() in the cxl_pmem flows.
> 
> Instead of the workqueue, arrange for all pmem/nvdimm devices to be torn
> down immediately and hierarchically. A similar change is made to both
> the 'cxl_nvdimm' and 'cxl_pmem_region' objects. For bisect-ability both
> changes are made in the same patch which unfortunately makes the patch
> bigger than desired.
> 
> Arrange for cxl_memdev and cxl_region to register a cxl_nvdimm and
> cxl_pmem_region as a devres release action of the bridge device.
> Additionally, include a devres release action of the cxl_memdev or
> cxl_region device that triggers the bridge's release action if an endpoint
> exits before the bridge. I.e. this allows either unplugging the bridge,
> or unplugging and endpoint to result in the same cleanup actions.
> 
> To keep the patch smaller the cleanup of the now defunct workqueue
> infrastructure is saved for a follow-on patch.
> 
> Tested-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

I wouldn't say it's the most confident review tag I've ever given, but I've
taken another look at it and couldn't identify any remaining issues...

So with that in mind
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>