* [PATCH] locking/lockdep: Fix lockdep_init_map_*() confusion
@ 2022-06-17 13:39 Peter Zijlstra
2022-06-17 14:57 ` Waiman Long
2022-06-28 7:09 ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
0 siblings, 2 replies; 3+ messages in thread
From: Peter Zijlstra @ 2022-06-17 13:39 UTC (permalink / raw)
To: peterz, mingo, will, longman, boqun.feng; +Cc: linux-kernel, x86
Commit dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t") added yet
another lockdep_init_map_*() variant, but forgot to update all the
existing users of the most complicated version.
This could lead to a loss of lock_type and hence an incorrect report.
Given the relative rarity of both local_lock and these annotations,
this is unlikely to happen in practise, still, best fix things.
Fixes: dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/lockdep.h | 30 +++++++++++++++++-------------
kernel/locking/lockdep.c | 7 ++++---
2 files changed, 21 insertions(+), 16 deletions(-)
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -188,7 +188,7 @@ static inline void
lockdep_init_map_waits(struct lockdep_map *lock, const char *name,
struct lock_class_key *key, int subclass, u8 inner, u8 outer)
{
- lockdep_init_map_type(lock, name, key, subclass, inner, LD_WAIT_INV, LD_LOCK_NORMAL);
+ lockdep_init_map_type(lock, name, key, subclass, inner, outer, LD_LOCK_NORMAL);
}
static inline void
@@ -211,24 +211,28 @@ static inline void lockdep_init_map(stru
* or they are too narrow (they suffer from a false class-split):
*/
#define lockdep_set_class(lock, key) \
- lockdep_init_map_waits(&(lock)->dep_map, #key, key, 0, \
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, #key, key, 0, \
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_class_and_name(lock, key, name) \
- lockdep_init_map_waits(&(lock)->dep_map, name, key, 0, \
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, name, key, 0, \
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_class_and_subclass(lock, key, sub) \
- lockdep_init_map_waits(&(lock)->dep_map, #key, key, sub,\
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, #key, key, sub, \
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_subclass(lock, sub) \
- lockdep_init_map_waits(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_novalidate_class(lock) \
lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock)
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5238,9 +5238,10 @@ __lock_set_class(struct lockdep_map *loc
return 0;
}
- lockdep_init_map_waits(lock, name, key, 0,
- lock->wait_type_inner,
- lock->wait_type_outer);
+ lockdep_init_map_type(lock, name, key, 0,
+ lock->wait_type_inner,
+ lock->wait_type_outer,
+ lock->lock_type);
class = register_lock_class(lock, subclass, 0);
hlock->class_idx = class - lock_classes;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] locking/lockdep: Fix lockdep_init_map_*() confusion
2022-06-17 13:39 [PATCH] locking/lockdep: Fix lockdep_init_map_*() confusion Peter Zijlstra
@ 2022-06-17 14:57 ` Waiman Long
2022-06-28 7:09 ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
1 sibling, 0 replies; 3+ messages in thread
From: Waiman Long @ 2022-06-17 14:57 UTC (permalink / raw)
To: Peter Zijlstra, mingo, will, boqun.feng; +Cc: linux-kernel, x86
On 6/17/22 09:39, Peter Zijlstra wrote:
> Commit dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t") added yet
> another lockdep_init_map_*() variant, but forgot to update all the
> existing users of the most complicated version.
>
> This could lead to a loss of lock_type and hence an incorrect report.
> Given the relative rarity of both local_lock and these annotations,
> this is unlikely to happen in practise, still, best fix things.
>
> Fixes: dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> include/linux/lockdep.h | 30 +++++++++++++++++-------------
> kernel/locking/lockdep.c | 7 ++++---
> 2 files changed, 21 insertions(+), 16 deletions(-)
>
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -188,7 +188,7 @@ static inline void
> lockdep_init_map_waits(struct lockdep_map *lock, const char *name,
> struct lock_class_key *key, int subclass, u8 inner, u8 outer)
> {
> - lockdep_init_map_type(lock, name, key, subclass, inner, LD_WAIT_INV, LD_LOCK_NORMAL);
> + lockdep_init_map_type(lock, name, key, subclass, inner, outer, LD_LOCK_NORMAL);
> }
>
> static inline void
> @@ -211,24 +211,28 @@ static inline void lockdep_init_map(stru
> * or they are too narrow (they suffer from a false class-split):
> */
> #define lockdep_set_class(lock, key) \
> - lockdep_init_map_waits(&(lock)->dep_map, #key, key, 0, \
> - (lock)->dep_map.wait_type_inner, \
> - (lock)->dep_map.wait_type_outer)
> + lockdep_init_map_type(&(lock)->dep_map, #key, key, 0, \
> + (lock)->dep_map.wait_type_inner, \
> + (lock)->dep_map.wait_type_outer, \
> + (lock)->dep_map.lock_type)
>
> #define lockdep_set_class_and_name(lock, key, name) \
> - lockdep_init_map_waits(&(lock)->dep_map, name, key, 0, \
> - (lock)->dep_map.wait_type_inner, \
> - (lock)->dep_map.wait_type_outer)
> + lockdep_init_map_type(&(lock)->dep_map, name, key, 0, \
> + (lock)->dep_map.wait_type_inner, \
> + (lock)->dep_map.wait_type_outer, \
> + (lock)->dep_map.lock_type)
>
> #define lockdep_set_class_and_subclass(lock, key, sub) \
> - lockdep_init_map_waits(&(lock)->dep_map, #key, key, sub,\
> - (lock)->dep_map.wait_type_inner, \
> - (lock)->dep_map.wait_type_outer)
> + lockdep_init_map_type(&(lock)->dep_map, #key, key, sub, \
> + (lock)->dep_map.wait_type_inner, \
> + (lock)->dep_map.wait_type_outer, \
> + (lock)->dep_map.lock_type)
>
> #define lockdep_set_subclass(lock, sub) \
> - lockdep_init_map_waits(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
> - (lock)->dep_map.wait_type_inner, \
> - (lock)->dep_map.wait_type_outer)
> + lockdep_init_map_type(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
> + (lock)->dep_map.wait_type_inner, \
> + (lock)->dep_map.wait_type_outer, \
> + (lock)->dep_map.lock_type)
>
> #define lockdep_set_novalidate_class(lock) \
> lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock)
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -5238,9 +5238,10 @@ __lock_set_class(struct lockdep_map *loc
> return 0;
> }
>
> - lockdep_init_map_waits(lock, name, key, 0,
> - lock->wait_type_inner,
> - lock->wait_type_outer);
> + lockdep_init_map_type(lock, name, key, 0,
> + lock->wait_type_inner,
> + lock->wait_type_outer,
> + lock->lock_type);
> class = register_lock_class(lock, subclass, 0);
> hlock->class_idx = class - lock_classes;
>
>
You have almost eliminated all usage of lockdep_init_map_waits() except
in lockdep_init_map_wait(). Should we just kill lockdep_init_map_waits()
and make lockdep_init_map_wait() call lockdep_init_map_type() directly?
Cheers,
Longman
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tip: locking/core] locking/lockdep: Fix lockdep_init_map_*() confusion
2022-06-17 13:39 [PATCH] locking/lockdep: Fix lockdep_init_map_*() confusion Peter Zijlstra
2022-06-17 14:57 ` Waiman Long
@ 2022-06-28 7:09 ` tip-bot2 for Peter Zijlstra
1 sibling, 0 replies; 3+ messages in thread
From: tip-bot2 for Peter Zijlstra @ 2022-06-28 7:09 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the locking/core branch of tip:
Commit-ID: eae6d58d67d9739be5f7ae2dbead1d0ef6528243
Gitweb: https://git.kernel.org/tip/eae6d58d67d9739be5f7ae2dbead1d0ef6528243
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Fri, 17 Jun 2022 15:26:06 +02:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 24 Jun 2022 09:48:56 +02:00
locking/lockdep: Fix lockdep_init_map_*() confusion
Commit dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t") added yet
another lockdep_init_map_*() variant, but forgot to update all the
existing users of the most complicated version.
This could lead to a loss of lock_type and hence an incorrect report.
Given the relative rarity of both local_lock and these annotations,
this is unlikely to happen in practise, still, best fix things.
Fixes: dfd5e3f5fe27 ("locking/lockdep: Mark local_lock_t")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/YqyEDtoan20K0CVD@worktop.programming.kicks-ass.net
---
include/linux/lockdep.h | 30 +++++++++++++++++-------------
kernel/locking/lockdep.c | 7 ++++---
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b6829b9..1f1099d 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -188,7 +188,7 @@ static inline void
lockdep_init_map_waits(struct lockdep_map *lock, const char *name,
struct lock_class_key *key, int subclass, u8 inner, u8 outer)
{
- lockdep_init_map_type(lock, name, key, subclass, inner, LD_WAIT_INV, LD_LOCK_NORMAL);
+ lockdep_init_map_type(lock, name, key, subclass, inner, outer, LD_LOCK_NORMAL);
}
static inline void
@@ -211,24 +211,28 @@ static inline void lockdep_init_map(struct lockdep_map *lock, const char *name,
* or they are too narrow (they suffer from a false class-split):
*/
#define lockdep_set_class(lock, key) \
- lockdep_init_map_waits(&(lock)->dep_map, #key, key, 0, \
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, #key, key, 0, \
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_class_and_name(lock, key, name) \
- lockdep_init_map_waits(&(lock)->dep_map, name, key, 0, \
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, name, key, 0, \
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_class_and_subclass(lock, key, sub) \
- lockdep_init_map_waits(&(lock)->dep_map, #key, key, sub,\
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, #key, key, sub, \
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_subclass(lock, sub) \
- lockdep_init_map_waits(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
- (lock)->dep_map.wait_type_inner, \
- (lock)->dep_map.wait_type_outer)
+ lockdep_init_map_type(&(lock)->dep_map, #lock, (lock)->dep_map.key, sub,\
+ (lock)->dep_map.wait_type_inner, \
+ (lock)->dep_map.wait_type_outer, \
+ (lock)->dep_map.lock_type)
#define lockdep_set_novalidate_class(lock) \
lockdep_set_class_and_name(lock, &__lockdep_no_validate__, #lock)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index f06b91c..e2f1794 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5238,9 +5238,10 @@ __lock_set_class(struct lockdep_map *lock, const char *name,
return 0;
}
- lockdep_init_map_waits(lock, name, key, 0,
- lock->wait_type_inner,
- lock->wait_type_outer);
+ lockdep_init_map_type(lock, name, key, 0,
+ lock->wait_type_inner,
+ lock->wait_type_outer,
+ lock->lock_type);
class = register_lock_class(lock, subclass, 0);
hlock->class_idx = class - lock_classes;
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-28 7:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-17 13:39 [PATCH] locking/lockdep: Fix lockdep_init_map_*() confusion Peter Zijlstra
2022-06-17 14:57 ` Waiman Long
2022-06-28 7:09 ` [tip: locking/core] " tip-bot2 for Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox