linux-kernel - Re: [PATCH v2 5/8] cxl/region: Split commit_store() into __commit() and queue

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] [day] [month] [year] [list]

Message-ID: <68563a383c145_606210037@dwillia2-mobl4.notmuch>
Date: Fri, 20 Jun 2025 21:51:04 -0700
From: <dan.j.williams@...el.com>
To: Alison Schofield <alison.schofield@...el.com>, Dan Williams
	<dan.j.williams@...el.com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Davidlohr
 Bueso" <dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>,
	Dave Jiang <dave.jiang@...el.com>, Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH v2 5/8] cxl/region: Split commit_store() into __commit()
 and queue_reset() helpers

Alison Schofield wrote:
> On Wed, Jun 18, 2025 at 10:04:13PM -0700, Dan Williams wrote:
> > The complexity of dropping the lock is removed in favor of splitting commit
> > operations to a helper, and leaving all the complexities of "decommit" for
> > commit_store() to coordinate the different locking contexts.
> > 
> > The CPU cache-invalidation in the decommit path is solely handled now by
> > cxl_region_decode_reset(). Previously the CPU caches were being needlessly
> > flushed twice in the decommit path where the first flush had no guarantee
> > that the memory would not be immediately re-dirtied.
> > 
> > Cc: Davidlohr Bueso <dave@...olabs.net>
> > Cc: Jonathan Cameron <jonathan.cameron@...wei.com>
> > Cc: Dave Jiang <dave.jiang@...el.com>
> > Cc: Alison Schofield <alison.schofield@...el.com>
> > Cc: Vishal Verma <vishal.l.verma@...el.com>
> > Cc: Ira Weiny <ira.weiny@...el.com>
> > Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> > ---
> >  drivers/cxl/core/region.c | 99 +++++++++++++++++++++++++++------------
> >  1 file changed, 70 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 6e5e1460068d..3a77aec2c447 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -349,30 +349,42 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
> >  	return rc;
> >  }
> >  
> > -static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
> > -			    const char *buf, size_t len)
> > +static int queue_reset(struct cxl_region *cxlr)
> 
> How about defining both new helpers to return ssize_t type for consistency
> with sysfs functions. The local 'int rc' can remain as long as the function
> return type is ssize_t.

The type change to 'int' for the helpers that are not dealing with
input/ouput is deliberate. queue_reset() is never returning a "size", it
is always returning 0 or a negative error code. An int fits in ssize_t.