[RFC,v4] cleanup: Add cond_guard() to conditional guards
Commit Message
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 whose address is given to its
second argument, that is either '-1' on failure or '0' on success to
acquire a lock. The returned value can be checked to act accordingly
(e.g., to call printk() and return -EINTR in case of failure of an
_interruptible() variant). It stores both success and failure to
always overwrite any old value which is possibly contained in the
return variable.
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 slightly different syntax.
Furthermore, I've only been able to set the return variable by
dereferencing its address. "_ret = _err" fails my tests. Any hint on why
it doesn't work for me is welcome.
The changes to the CXL code are provided only to show the use of this
macro. If consensus is gathered on this macro, the cleanup of
show_targetN() will be submitted later in a separate patch.
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>
---
Changes from v1 and v2:
Addressed Dan's comments (thanks).
Changes from v3:
Fixed a grammar error in macro.
drivers/cxl/core/region.c | 15 +++++----------
include/linux/cleanup.h | 19 +++++++++++++++++++
2 files changed, 24 insertions(+), 10 deletions(-)
@@ -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, &cxl_region_rwsem);
if (rc)
- return rc;
+ return -EINTR;
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)
@@ -134,6 +134,20 @@ 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, args...):
+ * for conditional locks like mutex_trylock() or down_read_interruptible().
+ * 'ret' is a pointer to a variable where this macro stores 0 on success
+ * and -1 on failure to acquire a lock.
+ *
+ * Example:
+ *
+ * int ret;
+ * cond_guard(rwsem_read_try, &ret, &sem);
+ * if (ret) {
+ * dev_dbg("down_read_trylock() failed to down 'sem')\n");
+ * return 0; // down_read_trylock() returns 0 on contention
+ * }
+ *
* 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 +179,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, args...) \
+ CLASS(_name, scope)(args); \
+ if (!__guard_ptr(_name)(&scope)) *_ret = -1; \
+ else *_ret = 0
+
#define scoped_guard(_name, args...) \
for (CLASS(_name, scope)(args), \
*done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)