2021-02-14 17:54:48

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 1/2] lockdep: add lockdep_assert_not_held()

On Fri, Feb 12, 2021 at 04:28:42PM -0700, Shuah Khan wrote:

> +#define lockdep_assert_not_held(l) do { \
> + WARN_ON(debug_locks && lockdep_is_held(l)); \
> + } while (0)
> +

This thing isn't as straight forward as you might think, but it'll
mostly work.

Notably this thing will misfire when lockdep_off() is employed. It
certainyl needs a comment to explain the subtleties.


2021-02-15 10:46:05

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 1/2] lockdep: add lockdep_assert_not_held()

On Sun, Feb 14, 2021 at 06:53:01PM +0100, Peter Zijlstra wrote:
> On Fri, Feb 12, 2021 at 04:28:42PM -0700, Shuah Khan wrote:
>
> > +#define lockdep_assert_not_held(l) do { \
> > + WARN_ON(debug_locks && lockdep_is_held(l)); \
> > + } while (0)
> > +
>
> This thing isn't as straight forward as you might think, but it'll
> mostly work.
>
> Notably this thing will misfire when lockdep_off() is employed. It
> certainyl needs a comment to explain the subtleties.

I think something like so will work, but please double check.

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b9e9adec73e8..c8b0d292bf8e 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -294,11 +294,15 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);

#define lockdep_depth(tsk) (debug_locks ? (tsk)->lockdep_depth : 0)

-#define lockdep_assert_held(l) do { \
- WARN_ON(debug_locks && !lockdep_is_held(l)); \
+#define lockdep_assert_held(l) do { \
+ WARN_ON(debug_locks && lockdep_is_held(l) == 0)); \
} while (0)

-#define lockdep_assert_held_write(l) do { \
+#define lockdep_assert_not_held(l) do { \
+ WARN_ON(debug_locks && lockdep_is_held(l) == 1)); \
+ } while (0)
+
+#define lockdep_assert_held_write(l) do { \
WARN_ON(debug_locks && !lockdep_is_held_type(l, 0)); \
} while (0)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index c1418b47f625..983ba206f7b2 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5467,7 +5467,7 @@ noinstr int lock_is_held_type(const struct lockdep_map *lock, int read)
int ret = 0;

if (unlikely(!lockdep_enabled()))
- return 1; /* avoid false negative lockdep_assert_held() */
+ return -1; /* avoid false negative lockdep_assert_held() */

raw_local_irq_save(flags);
check_flags(flags);