linux-cxl.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: <linux-cxl@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>,
	Alison Schofield <alison.schofield@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	"David Lechner" <dlechner@baylibre.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	"Fabio M. De Francesco"
	<fabio.maria.de.francesco@linux.intel.com>,
	"Fabio M . De Francesco" <fabio.m.de.francesco@linux.intel.com>,
	Ingo Molnar <mingo@kernel.org>, Ira Weiny <ira.weiny@intel.com>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	"Shiju Jose" <shiju.jose@huawei.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: [PATCH v2 0/8] cleanup: Introduce ACQUIRE(), a guard() for conditional locks
Date: Wed, 18 Jun 2025 22:04:08 -0700	[thread overview]
Message-ID: <20250619050416.782871-1-dan.j.williams@intel.com> (raw)

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@intel.com

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@noisy.programming.kicks-ass.net
[3]: http://lore.kernel.org/20250512105026.GP4439@noisy.programming.kicks-ass.net

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


             reply	other threads:[~2025-06-19  5:04 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-19  5:04 Dan Williams [this message]
2025-06-19  5:04 ` [PATCH v2 1/8] cleanup: Introduce ACQUIRE() and ACQUIRE_ERR() for conditional locks Dan Williams
2025-06-19 21:17   ` Dan Williams
2025-06-23 10:05   ` Jonathan Cameron
2025-07-10 22:46     ` dan.j.williams
2025-06-19  5:04 ` [PATCH v2 2/8] cxl/mbox: Convert poison list mutex to ACQUIRE() Dan Williams
2025-06-20 20:43   ` Alison Schofield
2025-06-23 10:08   ` Jonathan Cameron
2025-07-10 22:25     ` dan.j.williams
2025-06-23 14:49   ` Dave Jiang
2025-06-19  5:04 ` [PATCH v2 3/8] cxl/decoder: Move decoder register programming to a helper Dan Williams
2025-06-20 21:00   ` Alison Schofield
2025-06-23 10:51   ` Jonathan Cameron
2025-06-23 14:50   ` Dave Jiang
2025-06-19  5:04 ` [PATCH v2 4/8] cxl/decoder: Drop pointless locking Dan Williams
2025-06-19 23:40   ` Davidlohr Bueso
2025-06-20 21:02   ` Alison Schofield
2025-06-23 10:53   ` Jonathan Cameron
2025-06-23 14:51   ` Dave Jiang
2025-06-19  5:04 ` [PATCH v2 5/8] cxl/region: Split commit_store() into __commit() and queue_reset() helpers Dan Williams
2025-06-20 21:32   ` Alison Schofield
2025-06-21  4:51     ` dan.j.williams
2025-06-23 10:59   ` Jonathan Cameron
2025-06-23 14:59   ` Dave Jiang
2025-06-19  5:04 ` [PATCH v2 6/8] cxl/region: Move ready-to-probe state check to a helper Dan Williams
2025-06-23 15:01   ` Dave Jiang
2025-06-19  5:04 ` [PATCH v2 7/8] cxl/region: Introduce CLASS(cxl_decoder_detach...) consolidate multiple paths Dan Williams
2025-06-23 10:49   ` Jonathan Cameron
2025-07-11  4:12     ` dan.j.williams
2025-06-19  5:04 ` [PATCH v2 8/8] cxl: Convert to ACQUIRE() for conditional rwsem locking Dan Williams
2025-06-23 10:32   ` Jonathan Cameron
2025-07-11  3:21     ` dan.j.williams
2025-06-19 11:13 ` [PATCH v2 0/8] cleanup: Introduce ACQUIRE(), a guard() for conditional locks Peter Zijlstra
2025-07-02 23:39 ` Alison Schofield
2025-07-11  4:28   ` dan.j.williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250619050416.782871-1-dan.j.williams@intel.com \
    --to=dan.j.williams@intel.com \
    --cc=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=dlechner@baylibre.com \
    --cc=fabio.m.de.francesco@linux.intel.com \
    --cc=fabio.maria.de.francesco@linux.intel.com \
    --cc=ira.weiny@intel.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=shiju.jose@huawei.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).