Re: [PATCH v2 7/8] cxl/region: Introduce CLASS(cxl_decoder_detach...) consolidate multiple paths

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

Jonathan Cameron wrote:
> On Wed, 18 Jun 2025 22:04:15 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Both detach_target() and cxld_unregister() want to tear down a cxl_region
> > when an endpoint decoder is either detached or destroyed.
> > 
> > When a region is to be destroyed cxl_decoder_detach() releases
> > cxl_region_rwsem unbinds the cxl_region driver and re-acquires the rwsem.
> > 
> > This "reverse" locking pattern is difficult to reason about, not amenable
> > to scope-based cleanup, and the minor differences in the calling convention
> > of cxl_decoder_detach() currently results in the cxl_decoder_kill_region()
> > wrapper.
> > 
> > Introduce CLASS(cxl_decoder_detach...) which creates an object that moves
> > the post-detach cleanup work to a destructor, and consolidates minor
> > preamble differences in the constructor.
> 
> I'm struggling somewhat with grasping how the destructor pattern is useful
> here. In the two cases the scope is tightly defined around the
> declaration of the class instance. Doesn't that just boil down to
> automatically calling the destuctor function immediately?  If so what
> is the use of wrapping it up in a destructor?

tl;dr: yes, the CLASS() is not needed.

I came at this from the perspective of removing the mid-function unlock
pattern, and it was useful for me to decompose that into a
constructor/destructor pattern. However, you're right, with the
refactoring in place this further simplifies into a straightforward
helper function scheme.

I have __cxl_decoder_detach() which does the conditional cxl_region
lookup, and cxl_decoder_detach() which handles the conditional locking
and device_release_driver() follow-up.