Re: [PATCH] cxl/region: Delay inserting iomem resource until auto region commit

[Dan Williams <dan.j.williams@intel.com>]

Alison Schofield wrote:
[..]
> > 
> > The ambiguity of iomem resource parenting can be resolved by looking the
> > device-path for dax19.0 ("daxctl list -RDu"). It is also resolved by
> > noticing that the CXL region and the DAX region are using separate
> > memregion_id values (9 vs 19).
> > 
> > The extra effort for incremental precision is not needed, the contents
> > of /proc/iomem are accurate.
> > 
> 
> Incremental Precision?  Isn't it just wrong?

I would say it is "acceptably" wrong. More below...

> The region driver does not have a claim on that address range. It
> inserted the resource before it became a decoder owner. If it wanted
> to place a provisional hold and then drop it on failure to commit,
> that would at least be internally consistent.

When I say it is "acceptably" wrong the user created region flow also has
an unbounded time between when size_store() inserts the resource from
alloc_free_mem_region() to whenever the region becomes committed. In the
same way that the kernel can not know that "cxl create-region" crashed
and is never going to commit that region it started, the kernel can not
know when some wayward device is going to finally arrive to finish off
the region creation process.

The determination on when to give up and when to clean up is policy.
Kernel is allergic to policy, it is a userspace responsibility.
/proc/iomem correctly shows that CXL subsystem was interested in this
address range for a region and can decide "I am done waiting for
regionX, let me go clean that up and repurpose the devices and address
space for something else."

> In retrospect, inserting the resource prior to commit was a bug in the
> region lifecycle. I can re-present this as that, so folks do not get
> side-tracked debating how users interpret memory topology.

I do not think it is a bug. It serves exactly the same purpose as
alloc_hpa() for manual region creation. It says "CXL subsystem is
interested in instantiating a region here, stay tuned...".