[PATCH v2 0/8] cleanup: Introduce ACQUIRE(), a guard() for conditional locks
From: Dan Williams
Date: Thu Jun 19 2025 - 01:04:58 EST
Changes since v1: [1]
* Peter took one look at v1 and rewrote it into something significantly
better. Unlike my attempt that required suffering a new parallel
universe of lock guards, the rewrite reuses existing lock guards.
ACQUIRE() can be used any place guard() can be used, and adds
ACQUIRE_ERR() to pass the result of conditional locks.
[1]: http://lore.kernel.org/20250507072145.3614298-1-dan.j.williams@xxxxxxxxx
Note, all the code in patch1 is Peter's I just wrapped it in a changelog
and added some commentary. Peter, forgive me if you were still in the
process of circling back to this topic. I marked the patch attributed to
you as: "Not-yet-signed-off-by". Otherwise, my motivation for going
ahead with a formal submission are the multiple patchsets in my review /
development queue where I would like to use ACQUIRE().
The orginal motivation of v1 for this work is that the CXL subsystem
adopted scope-based helpers and achieved some decent cleanups. However,
that work stalled with conditional locks. It stalled due to the pain
points of scoped_cond_guard() detailed in patch1.
This work also allows for replacing open-coded equivalents like
rwsem_read_intr_acquire() that went upstream in v6.16:
0c6e6f1357cb cxl/edac: Add CXL memory device patrol scrub control feature
The open question from the discussion [2] was whether it was worth
defining a __GUARD_IS_ERR() asm helper. I left that alone.
Lastly, this version of ACQUIRE_ERR() matches Peter's original proposal
to have the caller pass the lock scope variable by reference [3]. My
change of heart came from looking at the conversion and wanting
ACQUIRE_ERR() to be more visually distinct from ACQUIRE() especially
because it is accessing lock condition metadata, not the lock itself.
E.g.
ACQUIRE(rwsem_read_intr, rwsem)(&cxl_rwsem.region);
if ((ret = ACQUIRE_ERR(rwsem_read_intr, &rwsem)))
return ret;
Yes, checkpatch disagrees with assignment in if(), but cleanup.h already
demands other expections for historical style, and a compact / limited
idiom for ACQUIRE_ERR() feels reasonable.
[2]: http://lore.kernel.org/20250514064624.GA24938@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
[3]: http://lore.kernel.org/20250512105026.GP4439@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Dan Williams (7):
cxl/mbox: Convert poison list mutex to ACQUIRE()
cxl/decoder: Move decoder register programming to a helper
cxl/decoder: Drop pointless locking
cxl/region: Split commit_store() into __commit() and queue_reset()
helpers
cxl/region: Move ready-to-probe state check to a helper
cxl/region: Introduce CLASS(cxl_decoder_detach...) consolidate
multiple paths
cxl: Convert to ACQUIRE() for conditional rwsem locking
Peter Zijlstra (1):
cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks
drivers/cxl/core/cdat.c | 6 +-
drivers/cxl/core/core.h | 60 +++++-
drivers/cxl/core/edac.c | 44 ++--
drivers/cxl/core/hdm.c | 118 +++++-----
drivers/cxl/core/mbox.c | 13 +-
drivers/cxl/core/memdev.c | 50 ++---
drivers/cxl/core/port.c | 24 +--
drivers/cxl/core/region.c | 443 +++++++++++++++++++-------------------
drivers/cxl/cxl.h | 13 +-
drivers/cxl/cxlmem.h | 4 +-
include/linux/cleanup.h | 77 +++++--
include/linux/mutex.h | 2 +-
include/linux/rwsem.h | 3 +-
13 files changed, 460 insertions(+), 397 deletions(-)
base-commit: e04c78d86a9699d136910cfc0bdcf01087e3267e
--
2.49.0