[1/3] cleanup: Add cond_guard() to conditional guards

Commit Message

Dan Williams Feb. 27, 2024, 4:48 p.m. UTC

From: Fabio M. De Francesco <[e-mail]>

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 statement-expression) that is passed as its
second argument. That statement (or statement-expression) is executed if
waiting for a lock is interrupted or if a _trylock() fails in case of
contention.

Usage example:

	cond_guard(mutex_intr, return -EINTR, &mutex);

Consistent with other usage of _guard(), locks are unlocked at the exit of
the scope where cond_guard() is called. This macro can be called multiple
times in the same scope.

Cc: Dave Jiang <[e-mail]>
Cc: Peter Zijlstra <[e-mail]>
Suggested-by: Dan Williams <[e-mail]>
Suggested-by: Ira Weiny <[e-mail]>
Suggested-by: Jonathan Cameron <[e-mail]>
Signed-off-by: Fabio M. De Francesco <[e-mail]>
Reviewed-by: Jonathan Cameron <[e-mail]>
Reviewed-by: Dave Jiang <[e-mail]>
Signed-off-by: Dan Williams <[e-mail]>
---
 include/linux/cleanup.h |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Linus Torvalds Feb. 27, 2024, 8:49 p.m. UTC | #1

On Tue, 27 Feb 2024 at 08:48, Dan Williams <[e-mail]> wrote:
>
>         cond_guard(mutex_intr, return -EINTR, &mutex);

Again, this is *not* helping make code readable and less likely to have bugs.

The macro has obvious deficiencies, like the "_fail" argument not
being surrounded by  "{ }" (the equivalent of parenthesizing an
expression argument), but even with that trivial fix the syntax is
just too ugly to live, and doesn't match normal C syntax.

And yes, we have other macros that don't have normal C syntax, and
they are ugly too (example: #define CHKINFO(ret) in
drivers/video/fbdev/hgafb.c), but we should have higher standards for
globally visible helpers, and we should have *MUCH* higher standards
for helpers that are supposed to be all about reducing mistakes.

Bad / odd syntax does not reduce mistakes.

If a sane 'guard' model doesn't work for some code, the answer is not
to make an insane guard model. The answer is to not use 'guard' in
code like that.

               Linus

Patch

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..602afb85da34 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -134,6 +134,19 @@  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' is a
+ *	statement or statement-expression that is executed if waiting for a
+ *	lock is interrupted or if a _trylock() fails in case of contention.
+ *
+ *	Example:
+ *
+ *		cond_guard(mutex_intr, return -EINTR, &mutex);
+ *
+ * 	This macro can be called multiple times in the same scope, for it
+ * 	declares unique instances of type 'name'.
+ *
  * 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 +178,13 @@  static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 
 #define __guard_ptr(_name) class_##_name##_lock_ptr
 
+#define __cond_guard(__unique, _name, _fail, args...) \
+	CLASS(_name, __unique)(args); \
+	if (!__guard_ptr(_name)(&__unique)) _fail; \
+	else { }
+#define cond_guard(_name, _fail, args...) \
+	__cond_guard(__UNIQUE_ID(scope), _name, _fail, args)
+
 #define scoped_guard(_name, args...)					\
 	for (CLASS(_name, scope)(args),					\
 	     *done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)