Add cond_guard() macro to conditional guards and use it to replace an
open-coded up_read() in show_targetN().
Cc: Peter Zijlstra <[email protected]>
Suggested-by: Dan Williams <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Fabio M. De Francesco (2):
cleanup: Add cond_guard() to conditional guards
cxl/region: Use cond_guard() in show_targetN()
drivers/cxl/core/region.c | 16 ++++------------
include/linux/cleanup.h | 14 ++++++++++++++
2 files changed, 18 insertions(+), 12 deletions(-)
--
2.43.0
Add cond_guard() macro to conditional guards.
cond_guard() is a guard to be used with the conditional variants of locks,
like down_read_trylock() or mutex_lock_interruptible().
It takes a statement (or more statements in a block) that is passed to its
second argument. That statement (or block) is executed if waiting for a
lock is interrupted or if a _trylock() fails in case of contention.
Usage example:
cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
Consistenly with the other guards, locks are unlocked at the exit of the
scope where cond_guard() is called.
Cc: Peter Zijlstra <[email protected]>
Suggested-by: Dan Williams <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
include/linux/cleanup.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..3826e8ed4e09 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -134,6 +134,16 @@ 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, fail, args...):
+ * a guard to be used with the conditional variants of locks, like
+ * down_read_trylock() or mutex_lock_interruptible. 'fail' are one or more
+ * statements that are executed if waiting for a lock is interrupted or
+ * if a _trylock() fails in case of contention.
+ *
+ * Example:
+ *
+ * cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
+ *
* 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 +175,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); \
+ if (!__guard_ptr(_name)(&scope)) _ret
+
#define scoped_guard(_name, args...) \
for (CLASS(_name, scope)(args), \
*done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)
--
2.43.0
Use cond_guard() in show_target() to not open code an up_read() in an 'out'
block. If the down_read_interruptible() fails, the statement passed to the
second argument of cond_guard() returns -EINTR.
Cc: Peter Zijlstra <[email protected]>
Suggested-by: Dan Williams <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
drivers/cxl/core/region.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 0f05692bfec3..bd3236786a25 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -666,28 +666,20 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
{
struct cxl_region_params *p = &cxlr->params;
struct cxl_endpoint_decoder *cxled;
- int rc;
- rc = down_read_interruptible(&cxl_region_rwsem);
- if (rc)
- return rc;
+ cond_guard(rwsem_read_intr, return -EINTR, &cxl_region_rwsem);
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)
--
2.43.0
On Sun, 4 Feb 2024 18:31:03 +0100
"Fabio M. De Francesco" <[email protected]> wrote:
> Add cond_guard() macro to conditional guards and use it to replace an
> open-coded up_read() in show_targetN().
>
> Cc: Peter Zijlstra <[email protected]>
> Suggested-by: Dan Williams <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
Change log since RFC v5?
>
> Fabio M. De Francesco (2):
> cleanup: Add cond_guard() to conditional guards
> cxl/region: Use cond_guard() in show_targetN()
>
> drivers/cxl/core/region.c | 16 ++++------------
> include/linux/cleanup.h | 14 ++++++++++++++
> 2 files changed, 18 insertions(+), 12 deletions(-)
>
On Sun, 4 Feb 2024 18:31:04 +0100
"Fabio M. De Francesco" <[email protected]> wrote:
> Add cond_guard() macro to conditional guards.
>
> cond_guard() is a guard to be used with the conditional variants of locks,
> like down_read_trylock() or mutex_lock_interruptible().
>
> It takes a statement (or more statements in a block) that is passed to its
> second argument. That statement (or block) is executed if waiting for a
> lock is interrupted or if a _trylock() fails in case of contention.
>
> Usage example:
>
> cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
>
> Consistenly with the other guards, locks are unlocked at the exit of the
Spell check.
> scope where cond_guard() is called.
>
> Cc: Peter Zijlstra <[email protected]>
> Suggested-by: Dan Williams <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
> include/linux/cleanup.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
> index c2d09bc4f976..3826e8ed4e09 100644
> --- a/include/linux/cleanup.h
> +++ b/include/linux/cleanup.h
> @@ -134,6 +134,16 @@ 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, fail, args...):
> + * a guard to be used with the conditional variants of locks, like
> + * down_read_trylock() or mutex_lock_interruptible. 'fail' are one or more
> + * statements that are executed if waiting for a lock is interrupted or
> + * if a _trylock() fails in case of contention.
> + *
> + * Example:
> + *
> + * cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
> + *
> * 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 +175,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); \
> + if (!__guard_ptr(_name)(&scope)) _ret
Use the naming that scoped_cond_guard() uses: _fail rather than _ret
> +
> #define scoped_guard(_name, args...) \
> for (CLASS(_name, scope)(args), \
> *done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)
On Sun, 4 Feb 2024 18:31:05 +0100
"Fabio M. De Francesco" <[email protected]> wrote:
> Use cond_guard() in show_target() to not open code an up_read() in an 'out'
> block. If the down_read_interruptible() fails, the statement passed to the
> second argument of cond_guard() returns -EINTR.
>
> Cc: Peter Zijlstra <[email protected]>
> Suggested-by: Dan Williams <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
LGTM
Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> drivers/cxl/core/region.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 0f05692bfec3..bd3236786a25 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -666,28 +666,20 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
> {
> struct cxl_region_params *p = &cxlr->params;
> struct cxl_endpoint_decoder *cxled;
> - int rc;
>
> - rc = down_read_interruptible(&cxl_region_rwsem);
> - if (rc)
> - return rc;
> + cond_guard(rwsem_read_intr, return -EINTR, &cxl_region_rwsem);
>
> 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)
Fabio M. De Francesco wrote:
> Add cond_guard() macro to conditional guards.
>
> cond_guard() is a guard to be used with the conditional variants of locks,
> like down_read_trylock() or mutex_lock_interruptible().
>
> It takes a statement (or more statements in a block) that is passed to its
> second argument. That statement (or block) is executed if waiting for a
> lock is interrupted or if a _trylock() fails in case of contention.
>
> Usage example:
>
> cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
>
> Consistenly with the other guards, locks are unlocked at the exit of the
Consistently
Reviewed-by: Ira Weiny <[email protected]>
> scope where cond_guard() is called.
>
> Cc: Peter Zijlstra <[email protected]>
> Suggested-by: Dan Williams <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
> include/linux/cleanup.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
> index c2d09bc4f976..3826e8ed4e09 100644
> --- a/include/linux/cleanup.h
> +++ b/include/linux/cleanup.h
> @@ -134,6 +134,16 @@ 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, fail, args...):
> + * a guard to be used with the conditional variants of locks, like
> + * down_read_trylock() or mutex_lock_interruptible. 'fail' are one or more
> + * statements that are executed if waiting for a lock is interrupted or
> + * if a _trylock() fails in case of contention.
> + *
> + * Example:
> + *
> + * cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
> + *
> * 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 +175,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); \
> + if (!__guard_ptr(_name)(&scope)) _ret
> +
> #define scoped_guard(_name, args...) \
> for (CLASS(_name, scope)(args), \
> *done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)
> --
> 2.43.0
>
Fabio M. De Francesco wrote:
> Use cond_guard() in show_target() to not open code an up_read() in an 'out'
> block. If the down_read_interruptible() fails, the statement passed to the
> second argument of cond_guard() returns -EINTR.
>
> Cc: Peter Zijlstra <[email protected]>
> Suggested-by: Dan Williams <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
> drivers/cxl/core/region.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 0f05692bfec3..bd3236786a25 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -666,28 +666,20 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
> {
> struct cxl_region_params *p = &cxlr->params;
> struct cxl_endpoint_decoder *cxled;
> - int rc;
>
> - rc = down_read_interruptible(&cxl_region_rwsem);
> - if (rc)
> - return rc;
> + cond_guard(rwsem_read_intr, return -EINTR, &cxl_region_rwsem);
>
> 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
NIT: This else is not needed anymore.
Reviewed-by: Ira Weiny <[email protected]>
> - 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)
> --
> 2.43.0
>
Fabio M. De Francesco wrote:
> Add cond_guard() macro to conditional guards.
>
> cond_guard() is a guard to be used with the conditional variants of locks,
> like down_read_trylock() or mutex_lock_interruptible().
>
> It takes a statement (or more statements in a block) that is passed to its
s/or more statements in a block/or statement-expression)/
s/to its/as its/
> second argument. That statement (or block) is executed if waiting for a
> lock is interrupted or if a _trylock() fails in case of contention.
>
> Usage example:
>
> cond_guard(rwsem_read_try, { printk(...); return 0; }, &semaphore);
Missed commenting on this in the last posting, but multi-statement fail
cases that print and return 0 are unlikely to ever be the common case. I
think the most simple to understand example is an interruptible lock
that returns -EINTR on failure:
cond_guard(mutex_intr, return -EINTR, &mutex);
..and then maybe mention that _fail can be a statement-expression if
needed.
> Consistenly with the other guards, locks are unlocked at the exit of the
s/Consistenly with the other guards/Consistent with other usage of guard()/