linux-kernel - Re: [PATCH v2 7/8] cxl/region: Introduce CLASS(cxl_decoder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <68708f37d30d6_1d3d10061@dwillia2-xfh.jf.intel.com.notmuch>
Date: Thu, 10 Jul 2025 21:12:39 -0700
From: <dan.j.williams@...el.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>, Dan Williams
	<dan.j.williams@...el.com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Davidlohr
 Bueso" <dave@...olabs.net>, Dave Jiang <dave.jiang@...el.com>, "Alison
 Schofield" <alison.schofield@...el.com>, Vishal Verma
	<vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH v2 7/8] cxl/region: Introduce CLASS(cxl_decoder_detach...)
 consolidate multiple paths

Jonathan Cameron wrote:
> On Wed, 18 Jun 2025 22:04:15 -0700
> Dan Williams <dan.j.williams@...el.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.