Received-SPF: pass (google.com: domain of linux-kernel+bounces-48201-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>,
	linux-kernel@vger.kernel.org
Cc: linux-cxl@vger.kernel.org,
	"Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>,
	Ira Weiny <ira.weiny@intel.com>
Subject: [RFC PATCH v5] cleanup: Add cond_guard() to conditional guards
Date: Thu,  1 Feb 2024 14:10:33 +0100
Message-ID: <20240201131033.9850-1-fabio.maria.de.francesco@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Add cond_guard() to conditional guards.

cond_guard() is used for the _interruptible(), _killable(), and _try
versions of locks.

It stores a return value to a variable that is given to its second
argument. In case of success it stores the third argument, whereas in case
of failure it stores the fourth.

The returned value can be checked to act accordingly. cond_guard() always
stores either success or failure codes because certain functions, e.g.
down_read_trylock(), return 1 on success and 0 on contention. By storing 1
to ret, an 'if (!ret)' after a successful down_read_trylock() will correctly
evaluate false and so the failure path is not executed.

As the other guards, it avoids to open code the release of the lock after a
goto to an 'out' label.

This remains an RFC because Dan suggested a different syntax.

The changes to the CXL code are provided only to show the use of this
macro. If consensus is reached on this macro, the cleanup of show_targetN()
will be submitted later with a separate patch.

Cc: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fabio.maria.de.francesco@linux.intel.com>
---

Changes from v1-4:
	Addressed Dan's requests (thanks) to change cond_guard() interface.

 drivers/cxl/core/region.c | 13 ++++---------
 include/linux/cleanup.h   | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 0f05692bfec3..15e03edce7af 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -668,26 +668,21 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
 	struct cxl_endpoint_decoder *cxled;
 	int rc;
 
-	rc = down_read_interruptible(&cxl_region_rwsem);
+	cond_guard(rwsem_read_intr, rc, 0, -EINTR, &cxl_region_rwsem);
 	if (rc)
 		return rc;
 
 	if (pos >= p->interleave_ways) {
 		dev_dbg(&cxlr->dev, "position %d out of range %d\n", pos,
 			p->interleave_ways);
-		rc = -ENXIO;
-		goto out;
+		return -ENXIO;
 	}
 
 	cxled = p->targets[pos];
 	if (!cxled)
-		rc = sysfs_emit(buf, "\n");
+		return sysfs_emit(buf, "\n");
 	else
-		rc = sysfs_emit(buf, "%s\n", dev_name(&cxled->cxld.dev));
-out:
-	up_read(&cxl_region_rwsem);
-
-	return rc;
+		return sysfs_emit(buf, "%s\n", dev_name(&cxled->cxld.dev));
 }
 
 static int match_free_decoder(struct device *dev, void *data)
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..63d7d5bc374f 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -134,6 +134,21 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
  *	an anonymous instance of the (guard) class, not recommended for
  *	conditional locks.
  *
+ * cond_guard(name, ret, scs, err, args...):
+ * 	for conditional locks like mutex_trylock() or down_read_interruptible().
+ * 	'ret' is a variable where this macro stores 'scs' on success and 'err'
+ * 	on failure to acquire a lock.
+ *
+ * 	Example:
+ *
+ * 	int ret;
+ * 	// down_read_trylock() returns 1 on success, 0 on contention
+ * 	cond_guard(rwsem_read_try, ret, 1, 0, &sem);
+ * 	if (!ret) {
+ * 		dev_dbg("down_read_trylock() failed to down 'sem')\n");
+ * 		return ret;
+ * 	}
+ *
  * scoped_guard (name, args...) { }:
  *	similar to CLASS(name, scope)(args), except the variable (with the
  *	explicit name 'scope') is declard in a for-loop such that its scope is
@@ -165,6 +180,11 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 
 #define __guard_ptr(_name) class_##_name##_lock_ptr
 
+#define cond_guard(_name, _ret, _scs, _err, args...) \
+	CLASS(_name, scope)(args); \
+	if (!__guard_ptr(_name)(&scope)) _ret = _err; \
+	else _ret = _scs
+
 #define scoped_guard(_name, args...)					\
 	for (CLASS(_name, scope)(args),					\
 	     *done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)
-- 
2.43.0