From: "Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>
To: linux-kernel@vger.kernel.org
Cc: "Fabio M. De Francesco"
<fabio.maria.de.francesco@linux.intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Dan Williams <dan.j.williams@intel.com>,
Ira Weiny <ira.weiny@intel.com>
Subject: [RFC PATCH] cleanup: Add cond_guard() to conditional guards
Date: Mon, 29 Jan 2024 20:00:55 +0100 [thread overview]
Message-ID: <20240129190100.329791-1-fabio.maria.de.francesco@linux.intel.com> (raw)
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@infradead.org>
Cc: 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>
---
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
next reply other threads:[~2024-01-29 19:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-29 19:00 Fabio M. De Francesco [this message]
2024-01-29 22:23 ` Dan Williams
2024-01-31 13:09 ` Fabio M. De Francesco
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=20240129190100.329791-1-fabio.maria.de.francesco@linux.intel.com \
--to=fabio.maria.de.francesco@linux.intel.com \
--cc=dan.j.williams@intel.com \
--cc=ira.weiny@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
/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
all inboxes | Powered by JetHome®