linux-kernel - [RFC PATCH] cleanup: Add cond_guard() to conditional guards

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

Message-ID: <20240129190100.329791-1-fabio.maria.de.francesco@linux.intel.com>
Date: Mon, 29 Jan 2024 20:00:55 +0100
From: "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>
To: linux-kernel@...r.kernel.org
Cc: "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Dan Williams <dan.j.williams@...el.com>,
	Ira Weiny <ira.weiny@...el.com>
Subject: [RFC PATCH] cleanup: Add cond_guard() to conditional guards

Add cond_guard() to conditional guards.

cond_guard() stores the return value from _interruptible(), _killable(),
and _try versions of locks to a variable whose address is passed as the
second parameter, so that the value can later be checked and an action
be taken accordingly (e.g., calling printk() in case of failure).

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

This is an RFC at least for two reasons...

1) I'm not sure that this guard is needed unless we want to avoid the
use of scoped_cond_guard() with its necessary indentation of the code
in the successful path and/or we want to check the return value of the
conditional lock.

2) By using cond_guard() it is not possible to release the lock before
calling other functions from the current one. The lock is held until the
current function exits. This is not always desirable.

The changes for cxl/region are only added to show a possible use case for
cond_guard().

Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Dan Williams <dan.j.williams@...el.com>
Suggested-by: Ira Weiny <ira.weiny@...el.com>
Signed-off-by: Fabio M. De Francesco <fabio.maria.de.francesco@...ux.intel.com>
---
 drivers/cxl/core/region.c | 7 ++-----
 include/linux/cleanup.h   | 9 +++++++++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 0f05692bfec3..4a72a8d161f0 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -668,15 +668,14 @@ 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, &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];
@@ -684,8 +683,6 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
 		rc = sysfs_emit(buf, "\n");
 	else
 		rc = sysfs_emit(buf, "%s\n", dev_name(&cxled->cxld.dev));
-out:
-	up_read(&cxl_region_rwsem);
 
 	return rc;
 }
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..550d21a425d3 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -134,6 +134,11 @@ 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, _retp, args...):
+ * 	this assigns *_retp with the return values from conditional locks like
+ * 	down_read_trylock() or mutex_lock_interruptible(). *_retp can then be
+ * 	checked and actions can be taken accordingly.
+ *
  * 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 +170,10 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 
 #define __guard_ptr(_name) class_##_name##_lock_ptr
 
+#define cond_guard(_name, _ret, args...) \
+	CLASS(_name, scope)(args); \
+	*_ret = (!__guard_ptr(_name)(&scope))
+
 #define scoped_guard(_name, args...)					\
 	for (CLASS(_name, scope)(args),					\
 	     *done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)
-- 
2.43.0