| 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
| ||
|
Message-ID: <a49ab85cbd70469c8d1ebb9a43db0517@huawei.com>
Date: Mon, 14 Jul 2025 16:28:23 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Dan Williams <dan.j.williams@...el.com>, "linux-cxl@...r.kernel.org"
<linux-cxl@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "David
Lechner" <dlechner@...libre.com>, Peter Zijlstra <peterz@...radead.org>,
"Linus Torvalds" <torvalds@...ux-foundation.org>, Ingo Molnar
<mingo@...nel.org>, "Fabio M. De Francesco"
<fabio.m.de.francesco@...ux.intel.com>, "Davidlohr Bueso"
<dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>, 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 v3 8/8] cxl: Convert to ACQUIRE() for conditional rwsem
locking
>-----Original Message-----
>From: Dan Williams <dan.j.williams@...el.com>
>Sent: 12 July 2025 00:50
>To: linux-cxl@...r.kernel.org
>Cc: linux-kernel@...r.kernel.org; David Lechner <dlechner@...libre.com>;
>Peter Zijlstra <peterz@...radead.org>; Linus Torvalds <torvalds@...ux-
>foundation.org>; Ingo Molnar <mingo@...nel.org>; Fabio M. De Francesco
><fabio.m.de.francesco@...ux.intel.com>; Davidlohr Bueso <dave@...olabs.net>;
>Jonathan Cameron <jonathan.cameron@...wei.com>; 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>; Shiju Jose
><shiju.jose@...wei.com>
>Subject: [PATCH v3 8/8] cxl: Convert to ACQUIRE() for conditional rwsem locking
>
>Use ACQUIRE() to cleanup conditional locking paths in the CXL driver The
>ACQUIRE() macro and its associated ACQUIRE_ERR() helpers, like
>scoped_cond_guard(), arrange for scoped-based conditional locking. Unlike
>scoped_cond_guard(), these macros arrange for an ERR_PTR() to be retrieved
>representing the state of the conditional lock.
>
>The goal of this conversion is to complete the removal of all explicit unlock calls
>in the subsystem. I.e. the methods to acquire a lock are solely via guard(),
>scoped_guard() (for limited cases), or ACQUIRE(). All unlock is implicit / scope-
>based. In order to make sure all lock sites are converted, the existing rwsem's
>are consolidated and renamed in 'struct cxl_rwsem'. While that makes the patch
>noisier it gives a clean cut-off between old-world (explicit unlock allowed), and
>new world (explicit unlock deleted).
>
>Cc: David Lechner <dlechner@...libre.com>
>Cc: Peter Zijlstra <peterz@...radead.org>
>Cc: Linus Torvalds <torvalds@...ux-foundation.org>
>Cc: Ingo Molnar <mingo@...nel.org>
>Cc: "Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>
>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>
>Cc: Shiju Jose <shiju.jose@...wei.com>
>Acked-by: "Peter Zijlstra (Intel)" <peterz@...radead.org>
>Signed-off-by: Dan Williams <dan.j.williams@...el.com>
Hi Dan,
For changes in CXL EDAC (drivers/cxl/core/edac.c),
Tested-by: Shiju Jose <shiju.jose@...wei.com>
>---
> drivers/cxl/core/cdat.c | 6 +-
> drivers/cxl/core/core.h | 17 ++-
> drivers/cxl/core/edac.c | 44 +++---
> drivers/cxl/core/hdm.c | 41 +++---
> drivers/cxl/core/mbox.c | 6 +-
> drivers/cxl/core/memdev.c | 50 +++----
> drivers/cxl/core/port.c | 18 +--
> drivers/cxl/core/region.c | 295 ++++++++++++++++----------------------
> drivers/cxl/cxl.h | 13 +-
> include/linux/rwsem.h | 1 +
> 10 files changed, 212 insertions(+), 279 deletions(-)
>
[...]
>diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c index
>2cbc664e5d62..f1ebdbe222c8 100644
>--- a/drivers/cxl/core/edac.c
>+++ b/drivers/cxl/core/edac.c
>@@ -115,10 +115,9 @@ static int cxl_scrub_get_attrbs(struct
>cxl_patrol_scrub_context *cxl_ps_ctx,
> flags, min_cycle);
> }
>
>- struct rw_semaphore *region_lock __free(rwsem_read_release) =
>- rwsem_read_intr_acquire(&cxl_region_rwsem);
>- if (!region_lock)
>- return -EINTR;
>+ ACQUIRE(rwsem_read_intr, rwsem)(&cxl_rwsem.region);
>+ if ((ret = ACQUIRE_ERR(rwsem_read_intr, &rwsem)))
Checkpatch is giving error here and in other places with similar coding style,
"ERROR: do not use assignment in if condition"
>+ return ret;
>
[...]
> p = &cxlr->params;
>@@ -2215,18 +2173,18 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
> struct cxl_region *detach;
>
> /* when the decoder is being destroyed lock unconditionally */
>- if (mode == DETACH_INVALIDATE)
>- down_write(&cxl_region_rwsem);
>- else {
>- int rc = down_write_killable(&cxl_region_rwsem);
>+ if (mode == DETACH_INVALIDATE) {
>+ guard(rwsem_write)(&cxl_rwsem.region);
>+ detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);
>+ } else {
>+ int rc;
>
>- if (rc)
>+ ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
>+ if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
> return rc;
>+ detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);
> }
May be detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);
add outside if ... else as before?
>
>- detach = __cxl_decoder_detach(cxlr, cxled, pos, mode);
>- up_write(&cxl_region_rwsem);
>-
> if (detach) {
> device_release_driver(&detach->dev);
> put_device(&detach->dev);
>@@ -2234,29 +2192,35 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
> return 0;
> }
[...]
> /*
> * downgrade write lock to read lock
>--
>2.50.0
>
Thanks,
Shiju
Powered by blists - more mailing lists