* [PATCH v7 01/23] locking/lockdep: Fix two 32-bit compiler warnings
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:02 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 02/23] locking/lockdep: Fix reported required memory size (1/2) Bart Van Assche
` (22 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Use %zu to format size_t instead of %lu to avoid that the compiler
complains about a mismatch between format specifier and argument on
32-bit systems.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 95932333a48b..9cdb6292b3c0 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4278,7 +4278,7 @@ void __init lockdep_init(void)
printk("... MAX_LOCKDEP_CHAINS: %lu\n", MAX_LOCKDEP_CHAINS);
printk("... CHAINHASH_SIZE: %lu\n", CHAINHASH_SIZE);
- printk(" memory used by lock dependency info: %lu kB\n",
+ printk(" memory used by lock dependency info: %zu kB\n",
(sizeof(struct lock_class) * MAX_LOCKDEP_KEYS +
sizeof(struct list_head) * CLASSHASH_SIZE +
sizeof(struct lock_list) * MAX_LOCKDEP_ENTRIES +
@@ -4290,7 +4290,7 @@ void __init lockdep_init(void)
) / 1024
);
- printk(" per task-struct memory footprint: %lu bytes\n",
+ printk(" per task-struct memory footprint: %zu bytes\n",
sizeof(struct held_lock) * MAX_LOCK_DEPTH);
}
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Fix two 32-bit compiler warnings
2019-02-14 23:00 ` [PATCH v7 01/23] locking/lockdep: Fix two 32-bit compiler warnings Bart Van Assche
@ 2019-02-28 7:02 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:02 UTC (permalink / raw)
To: linux-tip-commits
Cc: bvanassche, tglx, hpa, mingo, longman, paulmck, torvalds, akpm,
peterz, linux-kernel, johannes, will.deacon
Commit-ID: 09d75ecb122d8b600d76e3b8d53a10ffbe3bcec2
Gitweb: https://git.kernel.org/tip/09d75ecb122d8b600d76e3b8d53a10ffbe3bcec2
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:36 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:38 +0100
locking/lockdep: Fix two 32-bit compiler warnings
Use %zu to format size_t instead of %lu to avoid that the compiler
complains about a mismatch between format specifier and argument on
32-bit systems.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-2-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 7f7db23fc002..5c5283bf499c 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4266,7 +4266,7 @@ void __init lockdep_init(void)
printk("... MAX_LOCKDEP_CHAINS: %lu\n", MAX_LOCKDEP_CHAINS);
printk("... CHAINHASH_SIZE: %lu\n", CHAINHASH_SIZE);
- printk(" memory used by lock dependency info: %lu kB\n",
+ printk(" memory used by lock dependency info: %zu kB\n",
(sizeof(struct lock_class) * MAX_LOCKDEP_KEYS +
sizeof(struct list_head) * CLASSHASH_SIZE +
sizeof(struct lock_list) * MAX_LOCKDEP_ENTRIES +
@@ -4278,7 +4278,7 @@ void __init lockdep_init(void)
) / 1024
);
- printk(" per task-struct memory footprint: %lu bytes\n",
+ printk(" per task-struct memory footprint: %zu bytes\n",
sizeof(struct held_lock) * MAX_LOCK_DEPTH);
}
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 02/23] locking/lockdep: Fix reported required memory size (1/2)
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 01/23] locking/lockdep: Fix two 32-bit compiler warnings Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:03 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 03/23] locking/lockdep: Fix reported required memory size (2/2) Bart Van Assche
` (21 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Change the sizeof(array element time) * (array size) expressions into
sizeof(array). This fixes the size computations of the classhash_table[]
and chainhash_table[] arrays. Commit a63f38cc4ccf ("locking/lockdep:
Convert hash tables to hlists") namely changed the type of the elements
of that array from struct list_head into struct hlist_head.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 9cdb6292b3c0..193fef487a15 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4279,19 +4279,19 @@ void __init lockdep_init(void)
printk("... CHAINHASH_SIZE: %lu\n", CHAINHASH_SIZE);
printk(" memory used by lock dependency info: %zu kB\n",
- (sizeof(struct lock_class) * MAX_LOCKDEP_KEYS +
- sizeof(struct list_head) * CLASSHASH_SIZE +
- sizeof(struct lock_list) * MAX_LOCKDEP_ENTRIES +
- sizeof(struct lock_chain) * MAX_LOCKDEP_CHAINS +
- sizeof(struct list_head) * CHAINHASH_SIZE
+ (sizeof(lock_classes) +
+ sizeof(classhash_table) +
+ sizeof(list_entries) +
+ sizeof(lock_chains) +
+ sizeof(chainhash_table)
#ifdef CONFIG_PROVE_LOCKING
- + sizeof(struct circular_queue)
+ + sizeof(lock_cq)
#endif
) / 1024
);
printk(" per task-struct memory footprint: %zu bytes\n",
- sizeof(struct held_lock) * MAX_LOCK_DEPTH);
+ sizeof(((struct task_struct *)NULL)->held_locks));
}
static void
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Fix reported required memory size (1/2)
2019-02-14 23:00 ` [PATCH v7 02/23] locking/lockdep: Fix reported required memory size (1/2) Bart Van Assche
@ 2019-02-28 7:03 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:03 UTC (permalink / raw)
To: linux-tip-commits
Cc: torvalds, akpm, bvanassche, tglx, mingo, linux-kernel,
will.deacon, peterz, longman, paulmck, hpa, johannes
Commit-ID: 7ff8517e1034f26dde03d6df4026f085480408f0
Gitweb: https://git.kernel.org/tip/7ff8517e1034f26dde03d6df4026f085480408f0
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:37 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:39 +0100
locking/lockdep: Fix reported required memory size (1/2)
Change the sizeof(array element time) * (array size) expressions into
sizeof(array). This fixes the size computations of the classhash_table[]
and chainhash_table[] arrays.
The reason is that commit:
a63f38cc4ccf ("locking/lockdep: Convert hash tables to hlists")
changed the type of the elements of that array from 'struct list_head' into
'struct hlist_head'.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-3-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 5c5283bf499c..57a523f0273c 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4267,19 +4267,19 @@ void __init lockdep_init(void)
printk("... CHAINHASH_SIZE: %lu\n", CHAINHASH_SIZE);
printk(" memory used by lock dependency info: %zu kB\n",
- (sizeof(struct lock_class) * MAX_LOCKDEP_KEYS +
- sizeof(struct list_head) * CLASSHASH_SIZE +
- sizeof(struct lock_list) * MAX_LOCKDEP_ENTRIES +
- sizeof(struct lock_chain) * MAX_LOCKDEP_CHAINS +
- sizeof(struct list_head) * CHAINHASH_SIZE
+ (sizeof(lock_classes) +
+ sizeof(classhash_table) +
+ sizeof(list_entries) +
+ sizeof(lock_chains) +
+ sizeof(chainhash_table)
#ifdef CONFIG_PROVE_LOCKING
- + sizeof(struct circular_queue)
+ + sizeof(lock_cq)
#endif
) / 1024
);
printk(" per task-struct memory footprint: %zu bytes\n",
- sizeof(struct held_lock) * MAX_LOCK_DEPTH);
+ sizeof(((struct task_struct *)NULL)->held_locks));
}
static void
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 03/23] locking/lockdep: Fix reported required memory size (2/2)
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 01/23] locking/lockdep: Fix two 32-bit compiler warnings Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 02/23] locking/lockdep: Fix reported required memory size (1/2) Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:03 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 04/23] locking/lockdep: Avoid that add_chain_cache() adds an invalid chain to the cache Bart Van Assche
` (20 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Lock chains are only tracked with CONFIG_PROVE_LOCKING=y. Do not report
the memory required for the lock chain array if CONFIG_PROVE_LOCKING=n.
See also commit ca58abcb4a6d ("lockdep: sanitise CONFIG_PROVE_LOCKING").
Include the size of the chain_hlocks[] array.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 193fef487a15..b00c6edd6a28 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4282,10 +4282,11 @@ void __init lockdep_init(void)
(sizeof(lock_classes) +
sizeof(classhash_table) +
sizeof(list_entries) +
- sizeof(lock_chains) +
sizeof(chainhash_table)
#ifdef CONFIG_PROVE_LOCKING
+ sizeof(lock_cq)
+ + sizeof(lock_chains)
+ + sizeof(chain_hlocks)
#endif
) / 1024
);
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Fix reported required memory size (2/2)
2019-02-14 23:00 ` [PATCH v7 03/23] locking/lockdep: Fix reported required memory size (2/2) Bart Van Assche
@ 2019-02-28 7:03 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:03 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, mingo, johannes, tglx, akpm, paulmck, bvanassche,
hpa, peterz, will.deacon, longman, torvalds
Commit-ID: 15ea86b58c71d05e0921bebcf707aa30e43e9e25
Gitweb: https://git.kernel.org/tip/15ea86b58c71d05e0921bebcf707aa30e43e9e25
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:38 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:39 +0100
locking/lockdep: Fix reported required memory size (2/2)
Lock chains are only tracked with CONFIG_PROVE_LOCKING=y. Do not report
the memory required for the lock chain array if CONFIG_PROVE_LOCKING=n.
See also commit:
ca58abcb4a6d ("lockdep: sanitise CONFIG_PROVE_LOCKING")
Include the size of the chain_hlocks[] array.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-4-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 57a523f0273c..ec6f6aff4d8d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4270,10 +4270,11 @@ void __init lockdep_init(void)
(sizeof(lock_classes) +
sizeof(classhash_table) +
sizeof(list_entries) +
- sizeof(lock_chains) +
sizeof(chainhash_table)
#ifdef CONFIG_PROVE_LOCKING
+ sizeof(lock_cq)
+ + sizeof(lock_chains)
+ + sizeof(chain_hlocks)
#endif
) / 1024
);
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 04/23] locking/lockdep: Avoid that add_chain_cache() adds an invalid chain to the cache
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (2 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 03/23] locking/lockdep: Fix reported required memory size (2/2) Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:04 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 05/23] locking/lockdep: Reorder struct lock_class members Bart Van Assche
` (19 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Make sure that add_chain_cache() returns 0 and does not modify the
chain hash if nr_chain_hlocks == MAX_LOCKDEP_CHAIN_HLOCKS before this
function is called.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index b00c6edd6a28..404086b23fc7 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2206,16 +2206,8 @@ static inline int add_chain_cache(struct task_struct *curr,
chain_hlocks[chain->base + j] = lock_id;
}
chain_hlocks[chain->base + j] = class - lock_classes;
- }
-
- if (nr_chain_hlocks < MAX_LOCKDEP_CHAIN_HLOCKS)
nr_chain_hlocks += chain->depth;
-
-#ifdef CONFIG_DEBUG_LOCKDEP
- /*
- * Important for check_no_collision().
- */
- if (unlikely(nr_chain_hlocks > MAX_LOCKDEP_CHAIN_HLOCKS)) {
+ } else {
if (!debug_locks_off_graph_unlock())
return 0;
@@ -2223,7 +2215,6 @@ static inline int add_chain_cache(struct task_struct *curr,
dump_stack();
return 0;
}
-#endif
hlist_add_head_rcu(&chain->entry, hash_head);
debug_atomic_inc(chain_lookup_misses);
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Avoid that add_chain_cache() adds an invalid chain to the cache
2019-02-14 23:00 ` [PATCH v7 04/23] locking/lockdep: Avoid that add_chain_cache() adds an invalid chain to the cache Bart Van Assche
@ 2019-02-28 7:04 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:04 UTC (permalink / raw)
To: linux-tip-commits
Cc: johannes, longman, linux-kernel, hpa, paulmck, mingo, peterz,
tglx, torvalds, will.deacon, akpm, bvanassche
Commit-ID: 523b113bace5e64e860d8c61d7aa25057d274753
Gitweb: https://git.kernel.org/tip/523b113bace5e64e860d8c61d7aa25057d274753
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:39 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:40 +0100
locking/lockdep: Avoid that add_chain_cache() adds an invalid chain to the cache
Make sure that add_chain_cache() returns 0 and does not modify the
chain hash if nr_chain_hlocks == MAX_LOCKDEP_CHAIN_HLOCKS before this
function is called.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-5-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ec6f6aff4d8d..21d84510e28f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2195,16 +2195,8 @@ static inline int add_chain_cache(struct task_struct *curr,
chain_hlocks[chain->base + j] = lock_id;
}
chain_hlocks[chain->base + j] = class - lock_classes;
- }
-
- if (nr_chain_hlocks < MAX_LOCKDEP_CHAIN_HLOCKS)
nr_chain_hlocks += chain->depth;
-
-#ifdef CONFIG_DEBUG_LOCKDEP
- /*
- * Important for check_no_collision().
- */
- if (unlikely(nr_chain_hlocks > MAX_LOCKDEP_CHAIN_HLOCKS)) {
+ } else {
if (!debug_locks_off_graph_unlock())
return 0;
@@ -2212,7 +2204,6 @@ static inline int add_chain_cache(struct task_struct *curr,
dump_stack();
return 0;
}
-#endif
hlist_add_head_rcu(&chain->entry, hash_head);
debug_atomic_inc(chain_lookup_misses);
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 05/23] locking/lockdep: Reorder struct lock_class members
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (3 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 04/23] locking/lockdep: Avoid that add_chain_cache() adds an invalid chain to the cache Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:05 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 06/23] locking/lockdep: Make zap_class() remove all matching lock order entries Bart Van Assche
` (18 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
This patch does not change any functionality but makes the patch that
frees lock classes that are no longer in use easier to read.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/lockdep.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index c5335df2372f..0c38bade84b7 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -76,6 +76,13 @@ struct lock_class {
*/
struct list_head lock_entry;
+ /*
+ * These fields represent a directed graph of lock dependencies,
+ * to every node we attach a list of "forward" and a list of
+ * "backward" graph nodes.
+ */
+ struct list_head locks_after, locks_before;
+
struct lockdep_subclass_key *key;
unsigned int subclass;
unsigned int dep_gen_id;
@@ -86,13 +93,6 @@ struct lock_class {
unsigned long usage_mask;
struct stack_trace usage_traces[XXX_LOCK_USAGE_STATES];
- /*
- * These fields represent a directed graph of lock dependencies,
- * to every node we attach a list of "forward" and a list of
- * "backward" graph nodes.
- */
- struct list_head locks_after, locks_before;
-
/*
* Generation counter, when doing certain classes of graph walking,
* to ensure that we check one node only once:
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Reorder struct lock_class members
2019-02-14 23:00 ` [PATCH v7 05/23] locking/lockdep: Reorder struct lock_class members Bart Van Assche
@ 2019-02-28 7:05 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:05 UTC (permalink / raw)
To: linux-tip-commits
Cc: will.deacon, peterz, johannes, paulmck, tglx, mingo,
linux-kernel, akpm, torvalds, longman, hpa, bvanassche
Commit-ID: 09329d1c2024522308ca4de977fc6bba753bab1a
Gitweb: https://git.kernel.org/tip/09329d1c2024522308ca4de977fc6bba753bab1a
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:40 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:40 +0100
locking/lockdep: Reorder struct lock_class members
This patch does not change any functionality but makes the patch that
frees lock classes that are no longer in use easier to read.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-6-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/lockdep.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index c5335df2372f..0c38bade84b7 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -76,6 +76,13 @@ struct lock_class {
*/
struct list_head lock_entry;
+ /*
+ * These fields represent a directed graph of lock dependencies,
+ * to every node we attach a list of "forward" and a list of
+ * "backward" graph nodes.
+ */
+ struct list_head locks_after, locks_before;
+
struct lockdep_subclass_key *key;
unsigned int subclass;
unsigned int dep_gen_id;
@@ -86,13 +93,6 @@ struct lock_class {
unsigned long usage_mask;
struct stack_trace usage_traces[XXX_LOCK_USAGE_STATES];
- /*
- * These fields represent a directed graph of lock dependencies,
- * to every node we attach a list of "forward" and a list of
- * "backward" graph nodes.
- */
- struct list_head locks_after, locks_before;
-
/*
* Generation counter, when doing certain classes of graph walking,
* to ensure that we check one node only once:
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 06/23] locking/lockdep: Make zap_class() remove all matching lock order entries
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (4 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 05/23] locking/lockdep: Reorder struct lock_class members Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:05 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 07/23] locking/lockdep: Initialize the locks_before and locks_after lists earlier Bart Van Assche
` (17 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Make sure that all lock order entries that refer to a class are removed
from the list_entries[] array when a kernel module is unloaded.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/lockdep.h | 1 +
kernel/locking/lockdep.c | 19 +++++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 0c38bade84b7..b5e6bfe0ae4a 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -178,6 +178,7 @@ static inline void lockdep_copy_map(struct lockdep_map *to,
struct lock_list {
struct list_head entry;
struct lock_class *class;
+ struct lock_class *links_to;
struct stack_trace trace;
int distance;
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 404086b23fc7..16657662ca4f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -859,7 +859,8 @@ static struct lock_list *alloc_list_entry(void)
/*
* Add a new dependency to the head of the list:
*/
-static int add_lock_to_list(struct lock_class *this, struct list_head *head,
+static int add_lock_to_list(struct lock_class *this,
+ struct lock_class *links_to, struct list_head *head,
unsigned long ip, int distance,
struct stack_trace *trace)
{
@@ -873,6 +874,7 @@ static int add_lock_to_list(struct lock_class *this, struct list_head *head,
return 0;
entry->class = this;
+ entry->links_to = links_to;
entry->distance = distance;
entry->trace = *trace;
/*
@@ -1918,14 +1920,14 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
* Ok, all validations passed, add the new lock
* to the previous lock's dependency list:
*/
- ret = add_lock_to_list(hlock_class(next),
+ ret = add_lock_to_list(hlock_class(next), hlock_class(prev),
&hlock_class(prev)->locks_after,
next->acquire_ip, distance, trace);
if (!ret)
return 0;
- ret = add_lock_to_list(hlock_class(prev),
+ ret = add_lock_to_list(hlock_class(prev), hlock_class(next),
&hlock_class(next)->locks_before,
next->acquire_ip, distance, trace);
if (!ret)
@@ -4119,15 +4121,20 @@ void lockdep_reset(void)
*/
static void zap_class(struct lock_class *class)
{
+ struct lock_list *entry;
int i;
/*
* Remove all dependencies this lock is
* involved in:
*/
- for (i = 0; i < nr_list_entries; i++) {
- if (list_entries[i].class == class)
- list_del_rcu(&list_entries[i].entry);
+ for (i = 0, entry = list_entries; i < nr_list_entries; i++, entry++) {
+ if (entry->class != class && entry->links_to != class)
+ continue;
+ list_del_rcu(&entry->entry);
+ /* Clear .class and .links_to to avoid double removal. */
+ WRITE_ONCE(entry->class, NULL);
+ WRITE_ONCE(entry->links_to, NULL);
}
/*
* Unhash the class and remove it from the all_lock_classes list:
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Make zap_class() remove all matching lock order entries
2019-02-14 23:00 ` [PATCH v7 06/23] locking/lockdep: Make zap_class() remove all matching lock order entries Bart Van Assche
@ 2019-02-28 7:05 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:05 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, johannes, peterz, torvalds, longman, tglx, bvanassche,
linux-kernel, akpm, hpa, paulmck, will.deacon
Commit-ID: 86cffb80a525f7b8f969c8c79669d383e02f17d1
Gitweb: https://git.kernel.org/tip/86cffb80a525f7b8f969c8c79669d383e02f17d1
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:41 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:40 +0100
locking/lockdep: Make zap_class() remove all matching lock order entries
Make sure that all lock order entries that refer to a class are removed
from the list_entries[] array when a kernel module is unloaded.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-7-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/lockdep.h | 1 +
kernel/locking/lockdep.c | 19 +++++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 0c38bade84b7..b5e6bfe0ae4a 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -178,6 +178,7 @@ static inline void lockdep_copy_map(struct lockdep_map *to,
struct lock_list {
struct list_head entry;
struct lock_class *class;
+ struct lock_class *links_to;
struct stack_trace trace;
int distance;
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 21d84510e28f..28fbeb2a10cc 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -859,7 +859,8 @@ static struct lock_list *alloc_list_entry(void)
/*
* Add a new dependency to the head of the list:
*/
-static int add_lock_to_list(struct lock_class *this, struct list_head *head,
+static int add_lock_to_list(struct lock_class *this,
+ struct lock_class *links_to, struct list_head *head,
unsigned long ip, int distance,
struct stack_trace *trace)
{
@@ -873,6 +874,7 @@ static int add_lock_to_list(struct lock_class *this, struct list_head *head,
return 0;
entry->class = this;
+ entry->links_to = links_to;
entry->distance = distance;
entry->trace = *trace;
/*
@@ -1907,14 +1909,14 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
* Ok, all validations passed, add the new lock
* to the previous lock's dependency list:
*/
- ret = add_lock_to_list(hlock_class(next),
+ ret = add_lock_to_list(hlock_class(next), hlock_class(prev),
&hlock_class(prev)->locks_after,
next->acquire_ip, distance, trace);
if (!ret)
return 0;
- ret = add_lock_to_list(hlock_class(prev),
+ ret = add_lock_to_list(hlock_class(prev), hlock_class(next),
&hlock_class(next)->locks_before,
next->acquire_ip, distance, trace);
if (!ret)
@@ -4107,15 +4109,20 @@ void lockdep_reset(void)
*/
static void zap_class(struct lock_class *class)
{
+ struct lock_list *entry;
int i;
/*
* Remove all dependencies this lock is
* involved in:
*/
- for (i = 0; i < nr_list_entries; i++) {
- if (list_entries[i].class == class)
- list_del_rcu(&list_entries[i].entry);
+ for (i = 0, entry = list_entries; i < nr_list_entries; i++, entry++) {
+ if (entry->class != class && entry->links_to != class)
+ continue;
+ list_del_rcu(&entry->entry);
+ /* Clear .class and .links_to to avoid double removal. */
+ WRITE_ONCE(entry->class, NULL);
+ WRITE_ONCE(entry->links_to, NULL);
}
/*
* Unhash the class and remove it from the all_lock_classes list:
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 07/23] locking/lockdep: Initialize the locks_before and locks_after lists earlier
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (5 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 06/23] locking/lockdep: Make zap_class() remove all matching lock order entries Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:06 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 08/23] locking/lockdep: Split lockdep_free_key_range() and lockdep_reset_lock() Bart Van Assche
` (16 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
This patch does not change any functionality. A later patch will reuse
lock classes that have been freed. In combination with that patch this
patch wil have the effect of initializing lock class order lists once
instead of every time a lock class structure is reinitialized.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 16657662ca4f..9967599d7864 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -735,6 +735,25 @@ static bool assign_lock_key(struct lockdep_map *lock)
return true;
}
+/*
+ * Initialize the lock_classes[] array elements.
+ */
+static void init_data_structures_once(void)
+{
+ static bool initialization_happened;
+ int i;
+
+ if (likely(initialization_happened))
+ return;
+
+ initialization_happened = true;
+
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ INIT_LIST_HEAD(&lock_classes[i].locks_after);
+ INIT_LIST_HEAD(&lock_classes[i].locks_before);
+ }
+}
+
/*
* Register a lock's class in the hash-table, if the class is not present
* yet. Otherwise we look it up. We cache the result in the lock object
@@ -775,6 +794,8 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
goto out_unlock_set;
}
+ init_data_structures_once();
+
/*
* Allocate a new key from the static array, and add it to
* the hash:
@@ -793,8 +814,8 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
class->key = key;
class->name = lock->name;
class->subclass = subclass;
- INIT_LIST_HEAD(&class->locks_before);
- INIT_LIST_HEAD(&class->locks_after);
+ WARN_ON_ONCE(!list_empty(&class->locks_before));
+ WARN_ON_ONCE(!list_empty(&class->locks_after));
class->name_version = count_matching_names(class);
/*
* We use RCU's safe list-add method to make
@@ -4167,6 +4188,8 @@ void lockdep_free_key_range(void *start, unsigned long size)
int i;
int locked;
+ init_data_structures_once();
+
raw_local_irq_save(flags);
locked = graph_lock();
@@ -4230,6 +4253,8 @@ void lockdep_reset_lock(struct lockdep_map *lock)
unsigned long flags;
int j, locked;
+ init_data_structures_once();
+
raw_local_irq_save(flags);
locked = graph_lock();
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Initialize the locks_before and locks_after lists earlier
2019-02-14 23:00 ` [PATCH v7 07/23] locking/lockdep: Initialize the locks_before and locks_after lists earlier Bart Van Assche
@ 2019-02-28 7:06 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:06 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, will.deacon, hpa, torvalds, mingo, bvanassche,
akpm, paulmck, johannes, tglx, longman, peterz
Commit-ID: feb0a3865ed2f7d66a1f2686f7ad784422c249ad
Gitweb: https://git.kernel.org/tip/feb0a3865ed2f7d66a1f2686f7ad784422c249ad
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:42 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:41 +0100
locking/lockdep: Initialize the locks_before and locks_after lists earlier
This patch does not change any functionality. A later patch will reuse
lock classes that have been freed. In combination with that patch this
patch wil have the effect of initializing lock class order lists once
instead of every time a lock class structure is reinitialized.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-8-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 28fbeb2a10cc..d1a6daf1f51f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -735,6 +735,25 @@ static bool assign_lock_key(struct lockdep_map *lock)
return true;
}
+/*
+ * Initialize the lock_classes[] array elements.
+ */
+static void init_data_structures_once(void)
+{
+ static bool initialization_happened;
+ int i;
+
+ if (likely(initialization_happened))
+ return;
+
+ initialization_happened = true;
+
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ INIT_LIST_HEAD(&lock_classes[i].locks_after);
+ INIT_LIST_HEAD(&lock_classes[i].locks_before);
+ }
+}
+
/*
* Register a lock's class in the hash-table, if the class is not present
* yet. Otherwise we look it up. We cache the result in the lock object
@@ -775,6 +794,8 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
goto out_unlock_set;
}
+ init_data_structures_once();
+
/*
* Allocate a new key from the static array, and add it to
* the hash:
@@ -793,8 +814,8 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
class->key = key;
class->name = lock->name;
class->subclass = subclass;
- INIT_LIST_HEAD(&class->locks_before);
- INIT_LIST_HEAD(&class->locks_after);
+ WARN_ON_ONCE(!list_empty(&class->locks_before));
+ WARN_ON_ONCE(!list_empty(&class->locks_after));
class->name_version = count_matching_names(class);
/*
* We use RCU's safe list-add method to make
@@ -4155,6 +4176,8 @@ void lockdep_free_key_range(void *start, unsigned long size)
int i;
int locked;
+ init_data_structures_once();
+
raw_local_irq_save(flags);
locked = graph_lock();
@@ -4218,6 +4241,8 @@ void lockdep_reset_lock(struct lockdep_map *lock)
unsigned long flags;
int j, locked;
+ init_data_structures_once();
+
raw_local_irq_save(flags);
locked = graph_lock();
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 08/23] locking/lockdep: Split lockdep_free_key_range() and lockdep_reset_lock()
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (6 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 07/23] locking/lockdep: Initialize the locks_before and locks_after lists earlier Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:07 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 09/23] locking/lockdep: Make it easy to detect whether or not inside a selftest Bart Van Assche
` (15 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
This patch does not change the behavior of these functions but makes the
patch that frees unused lock classes easier to read.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 72 ++++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 9967599d7864..7f80d8789978 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4172,6 +4172,24 @@ static inline int within(const void *addr, void *start, unsigned long size)
return addr >= start && addr < start + size;
}
+static void __lockdep_free_key_range(void *start, unsigned long size)
+{
+ struct lock_class *class;
+ struct hlist_head *head;
+ int i;
+
+ /* Unhash all classes that were created by a module. */
+ for (i = 0; i < CLASSHASH_SIZE; i++) {
+ head = classhash_table + i;
+ hlist_for_each_entry_rcu(class, head, hash_entry) {
+ if (!within(class->key, start, size) &&
+ !within(class->name, start, size))
+ continue;
+ zap_class(class);
+ }
+ }
+}
+
/*
* Used in module.c to remove lock classes from memory that is going to be
* freed; and possibly re-used by other modules.
@@ -4182,30 +4200,14 @@ static inline int within(const void *addr, void *start, unsigned long size)
*/
void lockdep_free_key_range(void *start, unsigned long size)
{
- struct lock_class *class;
- struct hlist_head *head;
unsigned long flags;
- int i;
int locked;
init_data_structures_once();
raw_local_irq_save(flags);
locked = graph_lock();
-
- /*
- * Unhash all classes that were created by this module:
- */
- for (i = 0; i < CLASSHASH_SIZE; i++) {
- head = classhash_table + i;
- hlist_for_each_entry_rcu(class, head, hash_entry) {
- if (within(class->key, start, size))
- zap_class(class);
- else if (within(class->name, start, size))
- zap_class(class);
- }
- }
-
+ __lockdep_free_key_range(start, size);
if (locked)
graph_unlock();
raw_local_irq_restore(flags);
@@ -4247,16 +4249,11 @@ static bool lock_class_cache_is_registered(struct lockdep_map *lock)
return false;
}
-void lockdep_reset_lock(struct lockdep_map *lock)
+/* The caller must hold the graph lock. Does not sleep. */
+static void __lockdep_reset_lock(struct lockdep_map *lock)
{
struct lock_class *class;
- unsigned long flags;
- int j, locked;
-
- init_data_structures_once();
-
- raw_local_irq_save(flags);
- locked = graph_lock();
+ int j;
/*
* Remove all classes this lock might have:
@@ -4273,19 +4270,22 @@ void lockdep_reset_lock(struct lockdep_map *lock)
* Debug check: in the end all mapped classes should
* be gone.
*/
- if (unlikely(lock_class_cache_is_registered(lock))) {
- if (debug_locks_off_graph_unlock()) {
- /*
- * We all just reset everything, how did it match?
- */
- WARN_ON(1);
- }
- goto out_restore;
- }
+ if (WARN_ON_ONCE(lock_class_cache_is_registered(lock)))
+ debug_locks_off();
+}
+
+void lockdep_reset_lock(struct lockdep_map *lock)
+{
+ unsigned long flags;
+ int locked;
+
+ init_data_structures_once();
+
+ raw_local_irq_save(flags);
+ locked = graph_lock();
+ __lockdep_reset_lock(lock);
if (locked)
graph_unlock();
-
-out_restore:
raw_local_irq_restore(flags);
}
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Split lockdep_free_key_range() and lockdep_reset_lock()
2019-02-14 23:00 ` [PATCH v7 08/23] locking/lockdep: Split lockdep_free_key_range() and lockdep_reset_lock() Bart Van Assche
@ 2019-02-28 7:07 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:07 UTC (permalink / raw)
To: linux-tip-commits
Cc: will.deacon, torvalds, bvanassche, paulmck, akpm, longman,
linux-kernel, mingo, tglx, peterz, johannes, hpa
Commit-ID: 956f3563a8387beb7758f2e8ee483639ef91afc6
Gitweb: https://git.kernel.org/tip/956f3563a8387beb7758f2e8ee483639ef91afc6
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:43 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:42 +0100
locking/lockdep: Split lockdep_free_key_range() and lockdep_reset_lock()
This patch does not change the behavior of these functions but makes the
patch that frees unused lock classes easier to read.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-9-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 72 ++++++++++++++++++++++++------------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index d1a6daf1f51f..2d4c21a02546 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4160,6 +4160,24 @@ static inline int within(const void *addr, void *start, unsigned long size)
return addr >= start && addr < start + size;
}
+static void __lockdep_free_key_range(void *start, unsigned long size)
+{
+ struct lock_class *class;
+ struct hlist_head *head;
+ int i;
+
+ /* Unhash all classes that were created by a module. */
+ for (i = 0; i < CLASSHASH_SIZE; i++) {
+ head = classhash_table + i;
+ hlist_for_each_entry_rcu(class, head, hash_entry) {
+ if (!within(class->key, start, size) &&
+ !within(class->name, start, size))
+ continue;
+ zap_class(class);
+ }
+ }
+}
+
/*
* Used in module.c to remove lock classes from memory that is going to be
* freed; and possibly re-used by other modules.
@@ -4170,30 +4188,14 @@ static inline int within(const void *addr, void *start, unsigned long size)
*/
void lockdep_free_key_range(void *start, unsigned long size)
{
- struct lock_class *class;
- struct hlist_head *head;
unsigned long flags;
- int i;
int locked;
init_data_structures_once();
raw_local_irq_save(flags);
locked = graph_lock();
-
- /*
- * Unhash all classes that were created by this module:
- */
- for (i = 0; i < CLASSHASH_SIZE; i++) {
- head = classhash_table + i;
- hlist_for_each_entry_rcu(class, head, hash_entry) {
- if (within(class->key, start, size))
- zap_class(class);
- else if (within(class->name, start, size))
- zap_class(class);
- }
- }
-
+ __lockdep_free_key_range(start, size);
if (locked)
graph_unlock();
raw_local_irq_restore(flags);
@@ -4235,16 +4237,11 @@ static bool lock_class_cache_is_registered(struct lockdep_map *lock)
return false;
}
-void lockdep_reset_lock(struct lockdep_map *lock)
+/* The caller must hold the graph lock. Does not sleep. */
+static void __lockdep_reset_lock(struct lockdep_map *lock)
{
struct lock_class *class;
- unsigned long flags;
- int j, locked;
-
- init_data_structures_once();
-
- raw_local_irq_save(flags);
- locked = graph_lock();
+ int j;
/*
* Remove all classes this lock might have:
@@ -4261,19 +4258,22 @@ void lockdep_reset_lock(struct lockdep_map *lock)
* Debug check: in the end all mapped classes should
* be gone.
*/
- if (unlikely(lock_class_cache_is_registered(lock))) {
- if (debug_locks_off_graph_unlock()) {
- /*
- * We all just reset everything, how did it match?
- */
- WARN_ON(1);
- }
- goto out_restore;
- }
+ if (WARN_ON_ONCE(lock_class_cache_is_registered(lock)))
+ debug_locks_off();
+}
+
+void lockdep_reset_lock(struct lockdep_map *lock)
+{
+ unsigned long flags;
+ int locked;
+
+ init_data_structures_once();
+
+ raw_local_irq_save(flags);
+ locked = graph_lock();
+ __lockdep_reset_lock(lock);
if (locked)
graph_unlock();
-
-out_restore:
raw_local_irq_restore(flags);
}
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 09/23] locking/lockdep: Make it easy to detect whether or not inside a selftest
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (7 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 08/23] locking/lockdep: Split lockdep_free_key_range() and lockdep_reset_lock() Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:07 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 10/23] locking/lockdep: Update two outdated comments Bart Van Assche
` (14 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
The patch that frees unused lock classes will modify the behavior of
lockdep_free_key_range() and lockdep_reset_lock() depending on whether
or not these functions are called from the context of the lockdep
selftests. Hence make it easy to detect whether or not lockdep code
is called from the context of a lockdep selftest.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/lockdep.h | 5 +++++
kernel/locking/lockdep.c | 6 ++++++
lib/locking-selftest.c | 2 ++
3 files changed, 13 insertions(+)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b5e6bfe0ae4a..66eee1ba0f2a 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -265,6 +265,7 @@ extern void lockdep_reset(void);
extern void lockdep_reset_lock(struct lockdep_map *lock);
extern void lockdep_free_key_range(void *start, unsigned long size);
extern asmlinkage void lockdep_sys_exit(void);
+extern void lockdep_set_selftest_task(struct task_struct *task);
extern void lockdep_off(void);
extern void lockdep_on(void);
@@ -395,6 +396,10 @@ static inline void lockdep_on(void)
{
}
+static inline void lockdep_set_selftest_task(struct task_struct *task)
+{
+}
+
# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
# define lock_release(l, n, i) do { } while (0)
# define lock_downgrade(l, i) do { } while (0)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 7f80d8789978..42161b8f0e68 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -81,6 +81,7 @@ module_param(lock_stat, int, 0644);
* code to recurse back into the lockdep code...
*/
static arch_spinlock_t lockdep_lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
+static struct task_struct *lockdep_selftest_task_struct;
static int graph_lock(void)
{
@@ -331,6 +332,11 @@ void lockdep_on(void)
}
EXPORT_SYMBOL(lockdep_on);
+void lockdep_set_selftest_task(struct task_struct *task)
+{
+ lockdep_selftest_task_struct = task;
+}
+
/*
* Debugging switches:
*/
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 1e1bbf171eca..a1705545e6ac 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -1989,6 +1989,7 @@ void locking_selftest(void)
init_shared_classes();
debug_locks_silent = !debug_locks_verbose;
+ lockdep_set_selftest_task(current);
DO_TESTCASE_6R("A-A deadlock", AA);
DO_TESTCASE_6R("A-B-B-A deadlock", ABBA);
@@ -2097,5 +2098,6 @@ void locking_selftest(void)
printk("---------------------------------\n");
debug_locks = 1;
}
+ lockdep_set_selftest_task(NULL);
debug_locks_silent = 0;
}
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Make it easy to detect whether or not inside a selftest
2019-02-14 23:00 ` [PATCH v7 09/23] locking/lockdep: Make it easy to detect whether or not inside a selftest Bart Van Assche
@ 2019-02-28 7:07 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:07 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, akpm, torvalds, bvanassche, longman, tglx, will.deacon,
peterz, linux-kernel, mingo, paulmck, johannes
Commit-ID: cdc84d794947b5431c0a6916c303aee7114819d2
Gitweb: https://git.kernel.org/tip/cdc84d794947b5431c0a6916c303aee7114819d2
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:44 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:43 +0100
locking/lockdep: Make it easy to detect whether or not inside a selftest
The patch that frees unused lock classes will modify the behavior of
lockdep_free_key_range() and lockdep_reset_lock() depending on whether
or not these functions are called from the context of the lockdep
selftests. Hence make it easy to detect whether or not lockdep code
is called from the context of a lockdep selftest.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-10-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/lockdep.h | 5 +++++
kernel/locking/lockdep.c | 6 ++++++
lib/locking-selftest.c | 2 ++
3 files changed, 13 insertions(+)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b5e6bfe0ae4a..66eee1ba0f2a 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -265,6 +265,7 @@ extern void lockdep_reset(void);
extern void lockdep_reset_lock(struct lockdep_map *lock);
extern void lockdep_free_key_range(void *start, unsigned long size);
extern asmlinkage void lockdep_sys_exit(void);
+extern void lockdep_set_selftest_task(struct task_struct *task);
extern void lockdep_off(void);
extern void lockdep_on(void);
@@ -395,6 +396,10 @@ static inline void lockdep_on(void)
{
}
+static inline void lockdep_set_selftest_task(struct task_struct *task)
+{
+}
+
# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
# define lock_release(l, n, i) do { } while (0)
# define lock_downgrade(l, i) do { } while (0)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 2d4c21a02546..34cd87c65f5d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -81,6 +81,7 @@ module_param(lock_stat, int, 0644);
* code to recurse back into the lockdep code...
*/
static arch_spinlock_t lockdep_lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
+static struct task_struct *lockdep_selftest_task_struct;
static int graph_lock(void)
{
@@ -331,6 +332,11 @@ void lockdep_on(void)
}
EXPORT_SYMBOL(lockdep_on);
+void lockdep_set_selftest_task(struct task_struct *task)
+{
+ lockdep_selftest_task_struct = task;
+}
+
/*
* Debugging switches:
*/
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 1e1bbf171eca..a1705545e6ac 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -1989,6 +1989,7 @@ void locking_selftest(void)
init_shared_classes();
debug_locks_silent = !debug_locks_verbose;
+ lockdep_set_selftest_task(current);
DO_TESTCASE_6R("A-A deadlock", AA);
DO_TESTCASE_6R("A-B-B-A deadlock", ABBA);
@@ -2097,5 +2098,6 @@ void locking_selftest(void)
printk("---------------------------------\n");
debug_locks = 1;
}
+ lockdep_set_selftest_task(NULL);
debug_locks_silent = 0;
}
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 10/23] locking/lockdep: Update two outdated comments
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (8 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 09/23] locking/lockdep: Make it easy to detect whether or not inside a selftest Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:08 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 11/23] locking/lockdep: Free lock classes that are no longer in use Bart Van Assche
` (13 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
synchronize_sched() has been removed recently. Update the comments that
refer to synchronize_sched().
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Fixes: 51959d85f32d ("lockdep: Replace synchronize_sched() with synchronize_rcu()") # v5.0-rc1
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 42161b8f0e68..4bab8ecb88be 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4200,9 +4200,9 @@ static void __lockdep_free_key_range(void *start, unsigned long size)
* Used in module.c to remove lock classes from memory that is going to be
* freed; and possibly re-used by other modules.
*
- * We will have had one sync_sched() before getting here, so we're guaranteed
- * nobody will look up these exact classes -- they're properly dead but still
- * allocated.
+ * We will have had one synchronize_rcu() before getting here, so we're
+ * guaranteed nobody will look up these exact classes -- they're properly dead
+ * but still allocated.
*/
void lockdep_free_key_range(void *start, unsigned long size)
{
@@ -4221,8 +4221,6 @@ void lockdep_free_key_range(void *start, unsigned long size)
/*
* Wait for any possible iterators from look_up_lock_class() to pass
* before continuing to free the memory they refer to.
- *
- * sync_sched() is sufficient because the read-side is IRQ disable.
*/
synchronize_rcu();
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Update two outdated comments
2019-02-14 23:00 ` [PATCH v7 10/23] locking/lockdep: Update two outdated comments Bart Van Assche
@ 2019-02-28 7:08 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, bvanassche, hpa, linux-kernel, peterz, will.deacon, mingo,
akpm, torvalds, longman, paulmck, johannes
Commit-ID: 29fc33fb7283970701355dc89badba4ed21c7092
Gitweb: https://git.kernel.org/tip/29fc33fb7283970701355dc89badba4ed21c7092
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:45 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:43 +0100
locking/lockdep: Update two outdated comments
synchronize_sched() has been removed recently. Update the comments that
refer to synchronize_sched().
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Fixes: 51959d85f32d ("lockdep: Replace synchronize_sched() with synchronize_rcu()") # v5.0-rc1
Link: https://lkml.kernel.org/r/20190214230058.196511-11-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 34cd87c65f5d..c7ca3a4def7e 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -4188,9 +4188,9 @@ static void __lockdep_free_key_range(void *start, unsigned long size)
* Used in module.c to remove lock classes from memory that is going to be
* freed; and possibly re-used by other modules.
*
- * We will have had one sync_sched() before getting here, so we're guaranteed
- * nobody will look up these exact classes -- they're properly dead but still
- * allocated.
+ * We will have had one synchronize_rcu() before getting here, so we're
+ * guaranteed nobody will look up these exact classes -- they're properly dead
+ * but still allocated.
*/
void lockdep_free_key_range(void *start, unsigned long size)
{
@@ -4209,8 +4209,6 @@ void lockdep_free_key_range(void *start, unsigned long size)
/*
* Wait for any possible iterators from look_up_lock_class() to pass
* before continuing to free the memory they refer to.
- *
- * sync_sched() is sufficient because the read-side is IRQ disable.
*/
synchronize_rcu();
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 11/23] locking/lockdep: Free lock classes that are no longer in use
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (9 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 10/23] locking/lockdep: Update two outdated comments Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:09 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 12/23] locking/lockdep: Reuse list entries " Bart Van Assche
` (12 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Instead of leaving lock classes that are no longer in use in the
lock_classes array, reuse entries from that array that are no longer in
use. Maintain a linked list of free lock classes with list head
'free_lock_class'. Only add freed lock classes to the free_lock_classes
list after a grace period to avoid that a lock_classes[] element would
be reused while an RCU reader is accessing it. Since the lockdep
selftests run in a context where sleeping is not allowed and since the
selftests require that lock resetting/zapping works with debug_locks
off, make the behavior of lockdep_free_key_range() and
lockdep_reset_lock() depend on whether or not these are called from
the context of the lockdep selftests.
Thanks to Peter for having shown how to modify get_pending_free()
such that that function does not have to sleep.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/lockdep.h | 9 +-
kernel/locking/lockdep.c | 396 ++++++++++++++++++++++++++++++++++-----
2 files changed, 354 insertions(+), 51 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 66eee1ba0f2a..619ec3f26cdc 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -63,7 +63,8 @@ extern struct lock_class_key __lockdep_no_validate__;
#define LOCKSTAT_POINTS 4
/*
- * The lock-class itself:
+ * The lock-class itself. The order of the structure members matters.
+ * reinit_class() zeroes the key member and all subsequent members.
*/
struct lock_class {
/*
@@ -72,7 +73,9 @@ struct lock_class {
struct hlist_node hash_entry;
/*
- * global list of all lock-classes:
+ * Entry in all_lock_classes when in use. Entry in free_lock_classes
+ * when not in use. Instances that are being freed are on one of the
+ * zapped_classes lists.
*/
struct list_head lock_entry;
@@ -104,7 +107,7 @@ struct lock_class {
unsigned long contention_point[LOCKSTAT_POINTS];
unsigned long contending_point[LOCKSTAT_POINTS];
#endif
-};
+} __no_randomize_layout;
#ifdef CONFIG_LOCK_STAT
struct lock_time {
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 4bab8ecb88be..6920f406ee91 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -50,6 +50,7 @@
#include <linux/random.h>
#include <linux/jhash.h>
#include <linux/nmi.h>
+#include <linux/rcupdate.h>
#include <asm/sections.h>
@@ -135,8 +136,8 @@ static struct lock_list list_entries[MAX_LOCKDEP_ENTRIES];
/*
* All data structures here are protected by the global debug_lock.
*
- * Mutex key structs only get allocated, once during bootup, and never
- * get freed - this significantly simplifies the debugging code.
+ * nr_lock_classes is the number of elements of lock_classes[] that is
+ * in use.
*/
unsigned long nr_lock_classes;
#ifndef CONFIG_DEBUG_LOCKDEP
@@ -278,11 +279,39 @@ static inline void lock_release_holdtime(struct held_lock *hlock)
#endif
/*
- * We keep a global list of all lock classes. The list only grows,
- * never shrinks. The list is only accessed with the lockdep
- * spinlock lock held.
+ * We keep a global list of all lock classes. The list is only accessed with
+ * the lockdep spinlock lock held. free_lock_classes is a list with free
+ * elements. These elements are linked together by the lock_entry member in
+ * struct lock_class.
*/
LIST_HEAD(all_lock_classes);
+static LIST_HEAD(free_lock_classes);
+
+/**
+ * struct pending_free - information about data structures about to be freed
+ * @zapped: Head of a list with struct lock_class elements.
+ */
+struct pending_free {
+ struct list_head zapped;
+};
+
+/**
+ * struct delayed_free - data structures used for delayed freeing
+ *
+ * A data structure for delayed freeing of data structures that may be
+ * accessed by RCU readers at the time these were freed.
+ *
+ * @rcu_head: Used to schedule an RCU callback for freeing data structures.
+ * @index: Index of @pf to which freed data structures are added.
+ * @scheduled: Whether or not an RCU callback has been scheduled.
+ * @pf: Array with information about data structures about to be freed.
+ */
+static struct delayed_free {
+ struct rcu_head rcu_head;
+ int index;
+ bool scheduled;
+ struct pending_free pf[2];
+} delayed_free;
/*
* The lockdep classes are in a hash-table as well, for fast lookup:
@@ -742,7 +771,8 @@ static bool assign_lock_key(struct lockdep_map *lock)
}
/*
- * Initialize the lock_classes[] array elements.
+ * Initialize the lock_classes[] array elements, the free_lock_classes list
+ * and also the delayed_free structure.
*/
static void init_data_structures_once(void)
{
@@ -754,7 +784,12 @@ static void init_data_structures_once(void)
initialization_happened = true;
+ init_rcu_head(&delayed_free.rcu_head);
+ INIT_LIST_HEAD(&delayed_free.pf[0].zapped);
+ INIT_LIST_HEAD(&delayed_free.pf[1].zapped);
+
for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ list_add_tail(&lock_classes[i].lock_entry, &free_lock_classes);
INIT_LIST_HEAD(&lock_classes[i].locks_after);
INIT_LIST_HEAD(&lock_classes[i].locks_before);
}
@@ -802,11 +837,10 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
init_data_structures_once();
- /*
- * Allocate a new key from the static array, and add it to
- * the hash:
- */
- if (nr_lock_classes >= MAX_LOCKDEP_KEYS) {
+ /* Allocate a new lock class and add it to the hash. */
+ class = list_first_entry_or_null(&free_lock_classes, typeof(*class),
+ lock_entry);
+ if (!class) {
if (!debug_locks_off_graph_unlock()) {
return NULL;
}
@@ -815,7 +849,7 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
dump_stack();
return NULL;
}
- class = lock_classes + nr_lock_classes++;
+ nr_lock_classes++;
debug_atomic_inc(nr_unused_locks);
class->key = key;
class->name = lock->name;
@@ -829,9 +863,10 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
*/
hlist_add_head_rcu(&class->hash_entry, hash_head);
/*
- * Add it to the global list of classes:
+ * Remove the class from the free list and add it to the global list
+ * of classes.
*/
- list_add_tail(&class->lock_entry, &all_lock_classes);
+ list_move_tail(&class->lock_entry, &all_lock_classes);
if (verbose(class)) {
graph_unlock();
@@ -1871,6 +1906,24 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
struct lock_list this;
int ret;
+ if (!hlock_class(prev)->key || !hlock_class(next)->key) {
+ /*
+ * The warning statements below may trigger a use-after-free
+ * of the class name. It is better to trigger a use-after free
+ * and to have the class name most of the time instead of not
+ * having the class name available.
+ */
+ WARN_ONCE(!debug_locks_silent && !hlock_class(prev)->key,
+ "Detected use-after-free of lock class %px/%s\n",
+ hlock_class(prev),
+ hlock_class(prev)->name);
+ WARN_ONCE(!debug_locks_silent && !hlock_class(next)->key,
+ "Detected use-after-free of lock class %px/%s\n",
+ hlock_class(next),
+ hlock_class(next)->name);
+ return 2;
+ }
+
/*
* Prove that the new <prev> -> <next> dependency would not
* create a circular dependency in the graph. (We do this by
@@ -2253,19 +2306,16 @@ static inline int add_chain_cache(struct task_struct *curr,
}
/*
- * Look up a dependency chain.
+ * Look up a dependency chain. Must be called with either the graph lock or
+ * the RCU read lock held.
*/
static inline struct lock_chain *lookup_chain_cache(u64 chain_key)
{
struct hlist_head *hash_head = chainhashentry(chain_key);
struct lock_chain *chain;
- /*
- * We can walk it lock-free, because entries only get added
- * to the hash:
- */
hlist_for_each_entry_rcu(chain, hash_head, entry) {
- if (chain->chain_key == chain_key) {
+ if (READ_ONCE(chain->chain_key) == chain_key) {
debug_atomic_inc(chain_lookup_hits);
return chain;
}
@@ -3355,6 +3405,11 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
if (nest_lock && !__lock_is_held(nest_lock, -1))
return print_lock_nested_lock_not_held(curr, hlock, ip);
+ if (!debug_locks_silent) {
+ WARN_ON_ONCE(depth && !hlock_class(hlock - 1)->key);
+ WARN_ON_ONCE(!hlock_class(hlock)->key);
+ }
+
if (!validate_chain(curr, lock, hlock, chain_head, chain_key))
return 0;
@@ -4143,14 +4198,92 @@ void lockdep_reset(void)
raw_local_irq_restore(flags);
}
+/* Remove a class from a lock chain. Must be called with the graph lock held. */
+static void remove_class_from_lock_chain(struct lock_chain *chain,
+ struct lock_class *class)
+{
+#ifdef CONFIG_PROVE_LOCKING
+ struct lock_chain *new_chain;
+ u64 chain_key;
+ int i;
+
+ for (i = chain->base; i < chain->base + chain->depth; i++) {
+ if (chain_hlocks[i] != class - lock_classes)
+ continue;
+ /* The code below leaks one chain_hlock[] entry. */
+ if (--chain->depth > 0)
+ memmove(&chain_hlocks[i], &chain_hlocks[i + 1],
+ (chain->base + chain->depth - i) *
+ sizeof(chain_hlocks[0]));
+ /*
+ * Each lock class occurs at most once in a lock chain so once
+ * we found a match we can break out of this loop.
+ */
+ goto recalc;
+ }
+ /* Since the chain has not been modified, return. */
+ return;
+
+recalc:
+ chain_key = 0;
+ for (i = chain->base; i < chain->base + chain->depth; i++)
+ chain_key = iterate_chain_key(chain_key, chain_hlocks[i] + 1);
+ if (chain->depth && chain->chain_key == chain_key)
+ return;
+ /* Overwrite the chain key for concurrent RCU readers. */
+ WRITE_ONCE(chain->chain_key, chain_key);
+ /*
+ * Note: calling hlist_del_rcu() from inside a
+ * hlist_for_each_entry_rcu() loop is safe.
+ */
+ hlist_del_rcu(&chain->entry);
+ if (chain->depth == 0)
+ return;
+ /*
+ * If the modified lock chain matches an existing lock chain, drop
+ * the modified lock chain.
+ */
+ if (lookup_chain_cache(chain_key))
+ return;
+ if (WARN_ON_ONCE(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
+ debug_locks_off();
+ return;
+ }
+ /*
+ * Leak *chain because it is not safe to reinsert it before an RCU
+ * grace period has expired.
+ */
+ new_chain = lock_chains + nr_lock_chains++;
+ *new_chain = *chain;
+ hlist_add_head_rcu(&new_chain->entry, chainhashentry(chain_key));
+#endif
+}
+
+/* Must be called with the graph lock held. */
+static void remove_class_from_lock_chains(struct lock_class *class)
+{
+ struct lock_chain *chain;
+ struct hlist_head *head;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(chainhash_table); i++) {
+ head = chainhash_table + i;
+ hlist_for_each_entry_rcu(chain, head, entry) {
+ remove_class_from_lock_chain(chain, class);
+ }
+ }
+}
+
/*
* Remove all references to a lock class. The caller must hold the graph lock.
*/
-static void zap_class(struct lock_class *class)
+static void zap_class(struct pending_free *pf, struct lock_class *class)
{
struct lock_list *entry;
int i;
+ WARN_ON_ONCE(!class->key);
+
/*
* Remove all dependencies this lock is
* involved in:
@@ -4163,14 +4296,33 @@ static void zap_class(struct lock_class *class)
WRITE_ONCE(entry->class, NULL);
WRITE_ONCE(entry->links_to, NULL);
}
- /*
- * Unhash the class and remove it from the all_lock_classes list:
- */
- hlist_del_rcu(&class->hash_entry);
- list_del(&class->lock_entry);
+ if (list_empty(&class->locks_after) &&
+ list_empty(&class->locks_before)) {
+ list_move_tail(&class->lock_entry, &pf->zapped);
+ hlist_del_rcu(&class->hash_entry);
+ WRITE_ONCE(class->key, NULL);
+ WRITE_ONCE(class->name, NULL);
+ nr_lock_classes--;
+ } else {
+ WARN_ONCE(true, "%s() failed for class %s\n", __func__,
+ class->name);
+ }
- RCU_INIT_POINTER(class->key, NULL);
- RCU_INIT_POINTER(class->name, NULL);
+ remove_class_from_lock_chains(class);
+}
+
+static void reinit_class(struct lock_class *class)
+{
+ void *const p = class;
+ const unsigned int offset = offsetof(struct lock_class, key);
+
+ WARN_ON_ONCE(!class->lock_entry.next);
+ WARN_ON_ONCE(!list_empty(&class->locks_after));
+ WARN_ON_ONCE(!list_empty(&class->locks_before));
+ memset(p + offset, 0, sizeof(*class) - offset);
+ WARN_ON_ONCE(!class->lock_entry.next);
+ WARN_ON_ONCE(!list_empty(&class->locks_after));
+ WARN_ON_ONCE(!list_empty(&class->locks_before));
}
static inline int within(const void *addr, void *start, unsigned long size)
@@ -4178,7 +4330,87 @@ static inline int within(const void *addr, void *start, unsigned long size)
return addr >= start && addr < start + size;
}
-static void __lockdep_free_key_range(void *start, unsigned long size)
+static bool inside_selftest(void)
+{
+ return current == lockdep_selftest_task_struct;
+}
+
+/* The caller must hold the graph lock. */
+static struct pending_free *get_pending_free(void)
+{
+ return delayed_free.pf + delayed_free.index;
+}
+
+static void free_zapped_rcu(struct rcu_head *cb);
+
+/*
+ * Schedule an RCU callback if no RCU callback is pending. Must be called with
+ * the graph lock held.
+ */
+static void call_rcu_zapped(struct pending_free *pf)
+{
+ WARN_ON_ONCE(inside_selftest());
+
+ if (list_empty(&pf->zapped))
+ return;
+
+ if (delayed_free.scheduled)
+ return;
+
+ delayed_free.scheduled = true;
+
+ WARN_ON_ONCE(delayed_free.pf + delayed_free.index != pf);
+ delayed_free.index ^= 1;
+
+ call_rcu(&delayed_free.rcu_head, free_zapped_rcu);
+}
+
+/* The caller must hold the graph lock. May be called from RCU context. */
+static void __free_zapped_classes(struct pending_free *pf)
+{
+ struct lock_class *class;
+
+ list_for_each_entry(class, &pf->zapped, lock_entry)
+ reinit_class(class);
+
+ list_splice_init(&pf->zapped, &free_lock_classes);
+}
+
+static void free_zapped_rcu(struct rcu_head *ch)
+{
+ struct pending_free *pf;
+ unsigned long flags;
+
+ if (WARN_ON_ONCE(ch != &delayed_free.rcu_head))
+ return;
+
+ raw_local_irq_save(flags);
+ if (!graph_lock())
+ goto out_irq;
+
+ /* closed head */
+ pf = delayed_free.pf + (delayed_free.index ^ 1);
+ __free_zapped_classes(pf);
+ delayed_free.scheduled = false;
+
+ /*
+ * If there's anything on the open list, close and start a new callback.
+ */
+ call_rcu_zapped(delayed_free.pf + delayed_free.index);
+
+ graph_unlock();
+out_irq:
+ raw_local_irq_restore(flags);
+}
+
+/*
+ * Remove all lock classes from the class hash table and from the
+ * all_lock_classes list whose key or name is in the address range [start,
+ * start + size). Move these lock classes to the zapped_classes list. Must
+ * be called with the graph lock held.
+ */
+static void __lockdep_free_key_range(struct pending_free *pf, void *start,
+ unsigned long size)
{
struct lock_class *class;
struct hlist_head *head;
@@ -4191,7 +4423,7 @@ static void __lockdep_free_key_range(void *start, unsigned long size)
if (!within(class->key, start, size) &&
!within(class->name, start, size))
continue;
- zap_class(class);
+ zap_class(pf, class);
}
}
}
@@ -4204,8 +4436,9 @@ static void __lockdep_free_key_range(void *start, unsigned long size)
* guaranteed nobody will look up these exact classes -- they're properly dead
* but still allocated.
*/
-void lockdep_free_key_range(void *start, unsigned long size)
+static void lockdep_free_key_range_reg(void *start, unsigned long size)
{
+ struct pending_free *pf;
unsigned long flags;
int locked;
@@ -4213,9 +4446,15 @@ void lockdep_free_key_range(void *start, unsigned long size)
raw_local_irq_save(flags);
locked = graph_lock();
- __lockdep_free_key_range(start, size);
- if (locked)
- graph_unlock();
+ if (!locked)
+ goto out_irq;
+
+ pf = get_pending_free();
+ __lockdep_free_key_range(pf, start, size);
+ call_rcu_zapped(pf);
+
+ graph_unlock();
+out_irq:
raw_local_irq_restore(flags);
/*
@@ -4223,12 +4462,35 @@ void lockdep_free_key_range(void *start, unsigned long size)
* before continuing to free the memory they refer to.
*/
synchronize_rcu();
+}
- /*
- * XXX at this point we could return the resources to the pool;
- * instead we leak them. We would need to change to bitmap allocators
- * instead of the linear allocators we have now.
- */
+/*
+ * Free all lockdep keys in the range [start, start+size). Does not sleep.
+ * Ignores debug_locks. Must only be used by the lockdep selftests.
+ */
+static void lockdep_free_key_range_imm(void *start, unsigned long size)
+{
+ struct pending_free *pf = delayed_free.pf;
+ unsigned long flags;
+
+ init_data_structures_once();
+
+ raw_local_irq_save(flags);
+ arch_spin_lock(&lockdep_lock);
+ __lockdep_free_key_range(pf, start, size);
+ __free_zapped_classes(pf);
+ arch_spin_unlock(&lockdep_lock);
+ raw_local_irq_restore(flags);
+}
+
+void lockdep_free_key_range(void *start, unsigned long size)
+{
+ init_data_structures_once();
+
+ if (inside_selftest())
+ lockdep_free_key_range_imm(start, size);
+ else
+ lockdep_free_key_range_reg(start, size);
}
/*
@@ -4254,7 +4516,8 @@ static bool lock_class_cache_is_registered(struct lockdep_map *lock)
}
/* The caller must hold the graph lock. Does not sleep. */
-static void __lockdep_reset_lock(struct lockdep_map *lock)
+static void __lockdep_reset_lock(struct pending_free *pf,
+ struct lockdep_map *lock)
{
struct lock_class *class;
int j;
@@ -4268,7 +4531,7 @@ static void __lockdep_reset_lock(struct lockdep_map *lock)
*/
class = look_up_lock_class(lock, j);
if (class)
- zap_class(class);
+ zap_class(pf, class);
}
/*
* Debug check: in the end all mapped classes should
@@ -4278,21 +4541,57 @@ static void __lockdep_reset_lock(struct lockdep_map *lock)
debug_locks_off();
}
-void lockdep_reset_lock(struct lockdep_map *lock)
+/*
+ * Remove all information lockdep has about a lock if debug_locks == 1. Free
+ * released data structures from RCU context.
+ */
+static void lockdep_reset_lock_reg(struct lockdep_map *lock)
{
+ struct pending_free *pf;
unsigned long flags;
int locked;
- init_data_structures_once();
-
raw_local_irq_save(flags);
locked = graph_lock();
- __lockdep_reset_lock(lock);
- if (locked)
- graph_unlock();
+ if (!locked)
+ goto out_irq;
+
+ pf = get_pending_free();
+ __lockdep_reset_lock(pf, lock);
+ call_rcu_zapped(pf);
+
+ graph_unlock();
+out_irq:
+ raw_local_irq_restore(flags);
+}
+
+/*
+ * Reset a lock. Does not sleep. Ignores debug_locks. Must only be used by the
+ * lockdep selftests.
+ */
+static void lockdep_reset_lock_imm(struct lockdep_map *lock)
+{
+ struct pending_free *pf = delayed_free.pf;
+ unsigned long flags;
+
+ raw_local_irq_save(flags);
+ arch_spin_lock(&lockdep_lock);
+ __lockdep_reset_lock(pf, lock);
+ __free_zapped_classes(pf);
+ arch_spin_unlock(&lockdep_lock);
raw_local_irq_restore(flags);
}
+void lockdep_reset_lock(struct lockdep_map *lock)
+{
+ init_data_structures_once();
+
+ if (inside_selftest())
+ lockdep_reset_lock_imm(lock);
+ else
+ lockdep_reset_lock_reg(lock);
+}
+
void __init lockdep_init(void)
{
printk("Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar\n");
@@ -4309,7 +4608,8 @@ void __init lockdep_init(void)
(sizeof(lock_classes) +
sizeof(classhash_table) +
sizeof(list_entries) +
- sizeof(chainhash_table)
+ sizeof(chainhash_table) +
+ sizeof(delayed_free)
#ifdef CONFIG_PROVE_LOCKING
+ sizeof(lock_cq)
+ sizeof(lock_chains)
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Free lock classes that are no longer in use
2019-02-14 23:00 ` [PATCH v7 11/23] locking/lockdep: Free lock classes that are no longer in use Bart Van Assche
@ 2019-02-28 7:09 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:09 UTC (permalink / raw)
To: linux-tip-commits
Cc: bvanassche, linux-kernel, longman, peterz, torvalds, will.deacon,
johannes, paulmck, tglx, mingo, hpa, akpm
Commit-ID: a0b0fd53e1e67639b303b15939b9c653dbe7a8c4
Gitweb: https://git.kernel.org/tip/a0b0fd53e1e67639b303b15939b9c653dbe7a8c4
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:46 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:43 +0100
locking/lockdep: Free lock classes that are no longer in use
Instead of leaving lock classes that are no longer in use in the
lock_classes array, reuse entries from that array that are no longer in
use. Maintain a linked list of free lock classes with list head
'free_lock_class'. Only add freed lock classes to the free_lock_classes
list after a grace period to avoid that a lock_classes[] element would
be reused while an RCU reader is accessing it. Since the lockdep
selftests run in a context where sleeping is not allowed and since the
selftests require that lock resetting/zapping works with debug_locks
off, make the behavior of lockdep_free_key_range() and
lockdep_reset_lock() depend on whether or not these are called from
the context of the lockdep selftests.
Thanks to Peter for having shown how to modify get_pending_free()
such that that function does not have to sleep.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-12-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/lockdep.h | 9 +-
kernel/locking/lockdep.c | 396 +++++++++++++++++++++++++++++++++++++++++------
2 files changed, 354 insertions(+), 51 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 66eee1ba0f2a..619ec3f26cdc 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -63,7 +63,8 @@ extern struct lock_class_key __lockdep_no_validate__;
#define LOCKSTAT_POINTS 4
/*
- * The lock-class itself:
+ * The lock-class itself. The order of the structure members matters.
+ * reinit_class() zeroes the key member and all subsequent members.
*/
struct lock_class {
/*
@@ -72,7 +73,9 @@ struct lock_class {
struct hlist_node hash_entry;
/*
- * global list of all lock-classes:
+ * Entry in all_lock_classes when in use. Entry in free_lock_classes
+ * when not in use. Instances that are being freed are on one of the
+ * zapped_classes lists.
*/
struct list_head lock_entry;
@@ -104,7 +107,7 @@ struct lock_class {
unsigned long contention_point[LOCKSTAT_POINTS];
unsigned long contending_point[LOCKSTAT_POINTS];
#endif
-};
+} __no_randomize_layout;
#ifdef CONFIG_LOCK_STAT
struct lock_time {
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index c7ca3a4def7e..8ecf355dd163 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -50,6 +50,7 @@
#include <linux/random.h>
#include <linux/jhash.h>
#include <linux/nmi.h>
+#include <linux/rcupdate.h>
#include <asm/sections.h>
@@ -135,8 +136,8 @@ static struct lock_list list_entries[MAX_LOCKDEP_ENTRIES];
/*
* All data structures here are protected by the global debug_lock.
*
- * Mutex key structs only get allocated, once during bootup, and never
- * get freed - this significantly simplifies the debugging code.
+ * nr_lock_classes is the number of elements of lock_classes[] that is
+ * in use.
*/
unsigned long nr_lock_classes;
#ifndef CONFIG_DEBUG_LOCKDEP
@@ -278,11 +279,39 @@ static inline void lock_release_holdtime(struct held_lock *hlock)
#endif
/*
- * We keep a global list of all lock classes. The list only grows,
- * never shrinks. The list is only accessed with the lockdep
- * spinlock lock held.
+ * We keep a global list of all lock classes. The list is only accessed with
+ * the lockdep spinlock lock held. free_lock_classes is a list with free
+ * elements. These elements are linked together by the lock_entry member in
+ * struct lock_class.
*/
LIST_HEAD(all_lock_classes);
+static LIST_HEAD(free_lock_classes);
+
+/**
+ * struct pending_free - information about data structures about to be freed
+ * @zapped: Head of a list with struct lock_class elements.
+ */
+struct pending_free {
+ struct list_head zapped;
+};
+
+/**
+ * struct delayed_free - data structures used for delayed freeing
+ *
+ * A data structure for delayed freeing of data structures that may be
+ * accessed by RCU readers at the time these were freed.
+ *
+ * @rcu_head: Used to schedule an RCU callback for freeing data structures.
+ * @index: Index of @pf to which freed data structures are added.
+ * @scheduled: Whether or not an RCU callback has been scheduled.
+ * @pf: Array with information about data structures about to be freed.
+ */
+static struct delayed_free {
+ struct rcu_head rcu_head;
+ int index;
+ int scheduled;
+ struct pending_free pf[2];
+} delayed_free;
/*
* The lockdep classes are in a hash-table as well, for fast lookup:
@@ -742,7 +771,8 @@ static bool assign_lock_key(struct lockdep_map *lock)
}
/*
- * Initialize the lock_classes[] array elements.
+ * Initialize the lock_classes[] array elements, the free_lock_classes list
+ * and also the delayed_free structure.
*/
static void init_data_structures_once(void)
{
@@ -754,7 +784,12 @@ static void init_data_structures_once(void)
initialization_happened = true;
+ init_rcu_head(&delayed_free.rcu_head);
+ INIT_LIST_HEAD(&delayed_free.pf[0].zapped);
+ INIT_LIST_HEAD(&delayed_free.pf[1].zapped);
+
for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ list_add_tail(&lock_classes[i].lock_entry, &free_lock_classes);
INIT_LIST_HEAD(&lock_classes[i].locks_after);
INIT_LIST_HEAD(&lock_classes[i].locks_before);
}
@@ -802,11 +837,10 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
init_data_structures_once();
- /*
- * Allocate a new key from the static array, and add it to
- * the hash:
- */
- if (nr_lock_classes >= MAX_LOCKDEP_KEYS) {
+ /* Allocate a new lock class and add it to the hash. */
+ class = list_first_entry_or_null(&free_lock_classes, typeof(*class),
+ lock_entry);
+ if (!class) {
if (!debug_locks_off_graph_unlock()) {
return NULL;
}
@@ -815,7 +849,7 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
dump_stack();
return NULL;
}
- class = lock_classes + nr_lock_classes++;
+ nr_lock_classes++;
debug_atomic_inc(nr_unused_locks);
class->key = key;
class->name = lock->name;
@@ -829,9 +863,10 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
*/
hlist_add_head_rcu(&class->hash_entry, hash_head);
/*
- * Add it to the global list of classes:
+ * Remove the class from the free list and add it to the global list
+ * of classes.
*/
- list_add_tail(&class->lock_entry, &all_lock_classes);
+ list_move_tail(&class->lock_entry, &all_lock_classes);
if (verbose(class)) {
graph_unlock();
@@ -1860,6 +1895,24 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
struct lock_list this;
int ret;
+ if (!hlock_class(prev)->key || !hlock_class(next)->key) {
+ /*
+ * The warning statements below may trigger a use-after-free
+ * of the class name. It is better to trigger a use-after free
+ * and to have the class name most of the time instead of not
+ * having the class name available.
+ */
+ WARN_ONCE(!debug_locks_silent && !hlock_class(prev)->key,
+ "Detected use-after-free of lock class %px/%s\n",
+ hlock_class(prev),
+ hlock_class(prev)->name);
+ WARN_ONCE(!debug_locks_silent && !hlock_class(next)->key,
+ "Detected use-after-free of lock class %px/%s\n",
+ hlock_class(next),
+ hlock_class(next)->name);
+ return 2;
+ }
+
/*
* Prove that the new <prev> -> <next> dependency would not
* create a circular dependency in the graph. (We do this by
@@ -2242,19 +2295,16 @@ static inline int add_chain_cache(struct task_struct *curr,
}
/*
- * Look up a dependency chain.
+ * Look up a dependency chain. Must be called with either the graph lock or
+ * the RCU read lock held.
*/
static inline struct lock_chain *lookup_chain_cache(u64 chain_key)
{
struct hlist_head *hash_head = chainhashentry(chain_key);
struct lock_chain *chain;
- /*
- * We can walk it lock-free, because entries only get added
- * to the hash:
- */
hlist_for_each_entry_rcu(chain, hash_head, entry) {
- if (chain->chain_key == chain_key) {
+ if (READ_ONCE(chain->chain_key) == chain_key) {
debug_atomic_inc(chain_lookup_hits);
return chain;
}
@@ -3337,6 +3387,11 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
if (nest_lock && !__lock_is_held(nest_lock, -1))
return print_lock_nested_lock_not_held(curr, hlock, ip);
+ if (!debug_locks_silent) {
+ WARN_ON_ONCE(depth && !hlock_class(hlock - 1)->key);
+ WARN_ON_ONCE(!hlock_class(hlock)->key);
+ }
+
if (!validate_chain(curr, lock, hlock, chain_head, chain_key))
return 0;
@@ -4131,14 +4186,92 @@ void lockdep_reset(void)
raw_local_irq_restore(flags);
}
+/* Remove a class from a lock chain. Must be called with the graph lock held. */
+static void remove_class_from_lock_chain(struct lock_chain *chain,
+ struct lock_class *class)
+{
+#ifdef CONFIG_PROVE_LOCKING
+ struct lock_chain *new_chain;
+ u64 chain_key;
+ int i;
+
+ for (i = chain->base; i < chain->base + chain->depth; i++) {
+ if (chain_hlocks[i] != class - lock_classes)
+ continue;
+ /* The code below leaks one chain_hlock[] entry. */
+ if (--chain->depth > 0)
+ memmove(&chain_hlocks[i], &chain_hlocks[i + 1],
+ (chain->base + chain->depth - i) *
+ sizeof(chain_hlocks[0]));
+ /*
+ * Each lock class occurs at most once in a lock chain so once
+ * we found a match we can break out of this loop.
+ */
+ goto recalc;
+ }
+ /* Since the chain has not been modified, return. */
+ return;
+
+recalc:
+ chain_key = 0;
+ for (i = chain->base; i < chain->base + chain->depth; i++)
+ chain_key = iterate_chain_key(chain_key, chain_hlocks[i] + 1);
+ if (chain->depth && chain->chain_key == chain_key)
+ return;
+ /* Overwrite the chain key for concurrent RCU readers. */
+ WRITE_ONCE(chain->chain_key, chain_key);
+ /*
+ * Note: calling hlist_del_rcu() from inside a
+ * hlist_for_each_entry_rcu() loop is safe.
+ */
+ hlist_del_rcu(&chain->entry);
+ if (chain->depth == 0)
+ return;
+ /*
+ * If the modified lock chain matches an existing lock chain, drop
+ * the modified lock chain.
+ */
+ if (lookup_chain_cache(chain_key))
+ return;
+ if (WARN_ON_ONCE(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
+ debug_locks_off();
+ return;
+ }
+ /*
+ * Leak *chain because it is not safe to reinsert it before an RCU
+ * grace period has expired.
+ */
+ new_chain = lock_chains + nr_lock_chains++;
+ *new_chain = *chain;
+ hlist_add_head_rcu(&new_chain->entry, chainhashentry(chain_key));
+#endif
+}
+
+/* Must be called with the graph lock held. */
+static void remove_class_from_lock_chains(struct lock_class *class)
+{
+ struct lock_chain *chain;
+ struct hlist_head *head;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(chainhash_table); i++) {
+ head = chainhash_table + i;
+ hlist_for_each_entry_rcu(chain, head, entry) {
+ remove_class_from_lock_chain(chain, class);
+ }
+ }
+}
+
/*
* Remove all references to a lock class. The caller must hold the graph lock.
*/
-static void zap_class(struct lock_class *class)
+static void zap_class(struct pending_free *pf, struct lock_class *class)
{
struct lock_list *entry;
int i;
+ WARN_ON_ONCE(!class->key);
+
/*
* Remove all dependencies this lock is
* involved in:
@@ -4151,14 +4284,33 @@ static void zap_class(struct lock_class *class)
WRITE_ONCE(entry->class, NULL);
WRITE_ONCE(entry->links_to, NULL);
}
- /*
- * Unhash the class and remove it from the all_lock_classes list:
- */
- hlist_del_rcu(&class->hash_entry);
- list_del(&class->lock_entry);
+ if (list_empty(&class->locks_after) &&
+ list_empty(&class->locks_before)) {
+ list_move_tail(&class->lock_entry, &pf->zapped);
+ hlist_del_rcu(&class->hash_entry);
+ WRITE_ONCE(class->key, NULL);
+ WRITE_ONCE(class->name, NULL);
+ nr_lock_classes--;
+ } else {
+ WARN_ONCE(true, "%s() failed for class %s\n", __func__,
+ class->name);
+ }
- RCU_INIT_POINTER(class->key, NULL);
- RCU_INIT_POINTER(class->name, NULL);
+ remove_class_from_lock_chains(class);
+}
+
+static void reinit_class(struct lock_class *class)
+{
+ void *const p = class;
+ const unsigned int offset = offsetof(struct lock_class, key);
+
+ WARN_ON_ONCE(!class->lock_entry.next);
+ WARN_ON_ONCE(!list_empty(&class->locks_after));
+ WARN_ON_ONCE(!list_empty(&class->locks_before));
+ memset(p + offset, 0, sizeof(*class) - offset);
+ WARN_ON_ONCE(!class->lock_entry.next);
+ WARN_ON_ONCE(!list_empty(&class->locks_after));
+ WARN_ON_ONCE(!list_empty(&class->locks_before));
}
static inline int within(const void *addr, void *start, unsigned long size)
@@ -4166,7 +4318,87 @@ static inline int within(const void *addr, void *start, unsigned long size)
return addr >= start && addr < start + size;
}
-static void __lockdep_free_key_range(void *start, unsigned long size)
+static bool inside_selftest(void)
+{
+ return current == lockdep_selftest_task_struct;
+}
+
+/* The caller must hold the graph lock. */
+static struct pending_free *get_pending_free(void)
+{
+ return delayed_free.pf + delayed_free.index;
+}
+
+static void free_zapped_rcu(struct rcu_head *cb);
+
+/*
+ * Schedule an RCU callback if no RCU callback is pending. Must be called with
+ * the graph lock held.
+ */
+static void call_rcu_zapped(struct pending_free *pf)
+{
+ WARN_ON_ONCE(inside_selftest());
+
+ if (list_empty(&pf->zapped))
+ return;
+
+ if (delayed_free.scheduled)
+ return;
+
+ delayed_free.scheduled = true;
+
+ WARN_ON_ONCE(delayed_free.pf + delayed_free.index != pf);
+ delayed_free.index ^= 1;
+
+ call_rcu(&delayed_free.rcu_head, free_zapped_rcu);
+}
+
+/* The caller must hold the graph lock. May be called from RCU context. */
+static void __free_zapped_classes(struct pending_free *pf)
+{
+ struct lock_class *class;
+
+ list_for_each_entry(class, &pf->zapped, lock_entry)
+ reinit_class(class);
+
+ list_splice_init(&pf->zapped, &free_lock_classes);
+}
+
+static void free_zapped_rcu(struct rcu_head *ch)
+{
+ struct pending_free *pf;
+ unsigned long flags;
+
+ if (WARN_ON_ONCE(ch != &delayed_free.rcu_head))
+ return;
+
+ raw_local_irq_save(flags);
+ if (!graph_lock())
+ goto out_irq;
+
+ /* closed head */
+ pf = delayed_free.pf + (delayed_free.index ^ 1);
+ __free_zapped_classes(pf);
+ delayed_free.scheduled = false;
+
+ /*
+ * If there's anything on the open list, close and start a new callback.
+ */
+ call_rcu_zapped(delayed_free.pf + delayed_free.index);
+
+ graph_unlock();
+out_irq:
+ raw_local_irq_restore(flags);
+}
+
+/*
+ * Remove all lock classes from the class hash table and from the
+ * all_lock_classes list whose key or name is in the address range [start,
+ * start + size). Move these lock classes to the zapped_classes list. Must
+ * be called with the graph lock held.
+ */
+static void __lockdep_free_key_range(struct pending_free *pf, void *start,
+ unsigned long size)
{
struct lock_class *class;
struct hlist_head *head;
@@ -4179,7 +4411,7 @@ static void __lockdep_free_key_range(void *start, unsigned long size)
if (!within(class->key, start, size) &&
!within(class->name, start, size))
continue;
- zap_class(class);
+ zap_class(pf, class);
}
}
}
@@ -4192,8 +4424,9 @@ static void __lockdep_free_key_range(void *start, unsigned long size)
* guaranteed nobody will look up these exact classes -- they're properly dead
* but still allocated.
*/
-void lockdep_free_key_range(void *start, unsigned long size)
+static void lockdep_free_key_range_reg(void *start, unsigned long size)
{
+ struct pending_free *pf;
unsigned long flags;
int locked;
@@ -4201,9 +4434,15 @@ void lockdep_free_key_range(void *start, unsigned long size)
raw_local_irq_save(flags);
locked = graph_lock();
- __lockdep_free_key_range(start, size);
- if (locked)
- graph_unlock();
+ if (!locked)
+ goto out_irq;
+
+ pf = get_pending_free();
+ __lockdep_free_key_range(pf, start, size);
+ call_rcu_zapped(pf);
+
+ graph_unlock();
+out_irq:
raw_local_irq_restore(flags);
/*
@@ -4211,12 +4450,35 @@ void lockdep_free_key_range(void *start, unsigned long size)
* before continuing to free the memory they refer to.
*/
synchronize_rcu();
+}
- /*
- * XXX at this point we could return the resources to the pool;
- * instead we leak them. We would need to change to bitmap allocators
- * instead of the linear allocators we have now.
- */
+/*
+ * Free all lockdep keys in the range [start, start+size). Does not sleep.
+ * Ignores debug_locks. Must only be used by the lockdep selftests.
+ */
+static void lockdep_free_key_range_imm(void *start, unsigned long size)
+{
+ struct pending_free *pf = delayed_free.pf;
+ unsigned long flags;
+
+ init_data_structures_once();
+
+ raw_local_irq_save(flags);
+ arch_spin_lock(&lockdep_lock);
+ __lockdep_free_key_range(pf, start, size);
+ __free_zapped_classes(pf);
+ arch_spin_unlock(&lockdep_lock);
+ raw_local_irq_restore(flags);
+}
+
+void lockdep_free_key_range(void *start, unsigned long size)
+{
+ init_data_structures_once();
+
+ if (inside_selftest())
+ lockdep_free_key_range_imm(start, size);
+ else
+ lockdep_free_key_range_reg(start, size);
}
/*
@@ -4242,7 +4504,8 @@ static bool lock_class_cache_is_registered(struct lockdep_map *lock)
}
/* The caller must hold the graph lock. Does not sleep. */
-static void __lockdep_reset_lock(struct lockdep_map *lock)
+static void __lockdep_reset_lock(struct pending_free *pf,
+ struct lockdep_map *lock)
{
struct lock_class *class;
int j;
@@ -4256,7 +4519,7 @@ static void __lockdep_reset_lock(struct lockdep_map *lock)
*/
class = look_up_lock_class(lock, j);
if (class)
- zap_class(class);
+ zap_class(pf, class);
}
/*
* Debug check: in the end all mapped classes should
@@ -4266,21 +4529,57 @@ static void __lockdep_reset_lock(struct lockdep_map *lock)
debug_locks_off();
}
-void lockdep_reset_lock(struct lockdep_map *lock)
+/*
+ * Remove all information lockdep has about a lock if debug_locks == 1. Free
+ * released data structures from RCU context.
+ */
+static void lockdep_reset_lock_reg(struct lockdep_map *lock)
{
+ struct pending_free *pf;
unsigned long flags;
int locked;
- init_data_structures_once();
-
raw_local_irq_save(flags);
locked = graph_lock();
- __lockdep_reset_lock(lock);
- if (locked)
- graph_unlock();
+ if (!locked)
+ goto out_irq;
+
+ pf = get_pending_free();
+ __lockdep_reset_lock(pf, lock);
+ call_rcu_zapped(pf);
+
+ graph_unlock();
+out_irq:
+ raw_local_irq_restore(flags);
+}
+
+/*
+ * Reset a lock. Does not sleep. Ignores debug_locks. Must only be used by the
+ * lockdep selftests.
+ */
+static void lockdep_reset_lock_imm(struct lockdep_map *lock)
+{
+ struct pending_free *pf = delayed_free.pf;
+ unsigned long flags;
+
+ raw_local_irq_save(flags);
+ arch_spin_lock(&lockdep_lock);
+ __lockdep_reset_lock(pf, lock);
+ __free_zapped_classes(pf);
+ arch_spin_unlock(&lockdep_lock);
raw_local_irq_restore(flags);
}
+void lockdep_reset_lock(struct lockdep_map *lock)
+{
+ init_data_structures_once();
+
+ if (inside_selftest())
+ lockdep_reset_lock_imm(lock);
+ else
+ lockdep_reset_lock_reg(lock);
+}
+
void __init lockdep_init(void)
{
printk("Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar\n");
@@ -4297,7 +4596,8 @@ void __init lockdep_init(void)
(sizeof(lock_classes) +
sizeof(classhash_table) +
sizeof(list_entries) +
- sizeof(chainhash_table)
+ sizeof(chainhash_table) +
+ sizeof(delayed_free)
#ifdef CONFIG_PROVE_LOCKING
+ sizeof(lock_cq)
+ sizeof(lock_chains)
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 12/23] locking/lockdep: Reuse list entries that are no longer in use
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (10 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 11/23] locking/lockdep: Free lock classes that are no longer in use Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:09 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 13/23] locking/lockdep: Introduce lockdep_next_lockchain() and lock_chain_count() Bart Van Assche
` (11 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Instead of abandoning elements of list_entries[] that are no longer in
use, make alloc_list_entry() reuse array elements that have been freed.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 6920f406ee91..4308f0b9ecd5 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -45,6 +45,7 @@
#include <linux/hash.h>
#include <linux/ftrace.h>
#include <linux/stringify.h>
+#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/gfp.h>
#include <linux/random.h>
@@ -132,6 +133,7 @@ static inline int debug_locks_off_graph_unlock(void)
unsigned long nr_list_entries;
static struct lock_list list_entries[MAX_LOCKDEP_ENTRIES];
+static DECLARE_BITMAP(list_entries_in_use, MAX_LOCKDEP_ENTRIES);
/*
* All data structures here are protected by the global debug_lock.
@@ -907,7 +909,10 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
*/
static struct lock_list *alloc_list_entry(void)
{
- if (nr_list_entries >= MAX_LOCKDEP_ENTRIES) {
+ int idx = find_first_zero_bit(list_entries_in_use,
+ ARRAY_SIZE(list_entries));
+
+ if (idx >= ARRAY_SIZE(list_entries)) {
if (!debug_locks_off_graph_unlock())
return NULL;
@@ -915,7 +920,9 @@ static struct lock_list *alloc_list_entry(void)
dump_stack();
return NULL;
}
- return list_entries + nr_list_entries++;
+ nr_list_entries++;
+ __set_bit(idx, list_entries_in_use);
+ return list_entries + idx;
}
/*
@@ -1019,7 +1026,7 @@ static inline void mark_lock_accessed(struct lock_list *lock,
unsigned long nr;
nr = lock - list_entries;
- WARN_ON(nr >= nr_list_entries); /* Out-of-bounds, input fail */
+ WARN_ON(nr >= ARRAY_SIZE(list_entries)); /* Out-of-bounds, input fail */
lock->parent = parent;
lock->class->dep_gen_id = lockdep_dependency_gen_id;
}
@@ -1029,7 +1036,7 @@ static inline unsigned long lock_accessed(struct lock_list *lock)
unsigned long nr;
nr = lock - list_entries;
- WARN_ON(nr >= nr_list_entries); /* Out-of-bounds, input fail */
+ WARN_ON(nr >= ARRAY_SIZE(list_entries)); /* Out-of-bounds, input fail */
return lock->class->dep_gen_id == lockdep_dependency_gen_id;
}
@@ -4288,13 +4295,13 @@ static void zap_class(struct pending_free *pf, struct lock_class *class)
* Remove all dependencies this lock is
* involved in:
*/
- for (i = 0, entry = list_entries; i < nr_list_entries; i++, entry++) {
+ for_each_set_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) {
+ entry = list_entries + i;
if (entry->class != class && entry->links_to != class)
continue;
+ __clear_bit(i, list_entries_in_use);
+ nr_list_entries--;
list_del_rcu(&entry->entry);
- /* Clear .class and .links_to to avoid double removal. */
- WRITE_ONCE(entry->class, NULL);
- WRITE_ONCE(entry->links_to, NULL);
}
if (list_empty(&class->locks_after) &&
list_empty(&class->locks_before)) {
@@ -4608,6 +4615,7 @@ void __init lockdep_init(void)
(sizeof(lock_classes) +
sizeof(classhash_table) +
sizeof(list_entries) +
+ sizeof(list_entries_in_use) +
sizeof(chainhash_table) +
sizeof(delayed_free)
#ifdef CONFIG_PROVE_LOCKING
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Reuse list entries that are no longer in use
2019-02-14 23:00 ` [PATCH v7 12/23] locking/lockdep: Reuse list entries " Bart Van Assche
@ 2019-02-28 7:09 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:09 UTC (permalink / raw)
To: linux-tip-commits
Cc: will.deacon, akpm, torvalds, linux-kernel, longman, peterz,
johannes, mingo, bvanassche, paulmck, tglx, hpa
Commit-ID: ace35a7ac493d4284a57ad807579011bebba891c
Gitweb: https://git.kernel.org/tip/ace35a7ac493d4284a57ad807579011bebba891c
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:47 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:44 +0100
locking/lockdep: Reuse list entries that are no longer in use
Instead of abandoning elements of list_entries[] that are no longer in
use, make alloc_list_entry() reuse array elements that have been freed.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-13-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 8ecf355dd163..2c6d0b67e7b6 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -45,6 +45,7 @@
#include <linux/hash.h>
#include <linux/ftrace.h>
#include <linux/stringify.h>
+#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/gfp.h>
#include <linux/random.h>
@@ -132,6 +133,7 @@ static inline int debug_locks_off_graph_unlock(void)
unsigned long nr_list_entries;
static struct lock_list list_entries[MAX_LOCKDEP_ENTRIES];
+static DECLARE_BITMAP(list_entries_in_use, MAX_LOCKDEP_ENTRIES);
/*
* All data structures here are protected by the global debug_lock.
@@ -907,7 +909,10 @@ out_set_class_cache:
*/
static struct lock_list *alloc_list_entry(void)
{
- if (nr_list_entries >= MAX_LOCKDEP_ENTRIES) {
+ int idx = find_first_zero_bit(list_entries_in_use,
+ ARRAY_SIZE(list_entries));
+
+ if (idx >= ARRAY_SIZE(list_entries)) {
if (!debug_locks_off_graph_unlock())
return NULL;
@@ -915,7 +920,9 @@ static struct lock_list *alloc_list_entry(void)
dump_stack();
return NULL;
}
- return list_entries + nr_list_entries++;
+ nr_list_entries++;
+ __set_bit(idx, list_entries_in_use);
+ return list_entries + idx;
}
/*
@@ -1019,7 +1026,7 @@ static inline void mark_lock_accessed(struct lock_list *lock,
unsigned long nr;
nr = lock - list_entries;
- WARN_ON(nr >= nr_list_entries); /* Out-of-bounds, input fail */
+ WARN_ON(nr >= ARRAY_SIZE(list_entries)); /* Out-of-bounds, input fail */
lock->parent = parent;
lock->class->dep_gen_id = lockdep_dependency_gen_id;
}
@@ -1029,7 +1036,7 @@ static inline unsigned long lock_accessed(struct lock_list *lock)
unsigned long nr;
nr = lock - list_entries;
- WARN_ON(nr >= nr_list_entries); /* Out-of-bounds, input fail */
+ WARN_ON(nr >= ARRAY_SIZE(list_entries)); /* Out-of-bounds, input fail */
return lock->class->dep_gen_id == lockdep_dependency_gen_id;
}
@@ -4276,13 +4283,13 @@ static void zap_class(struct pending_free *pf, struct lock_class *class)
* Remove all dependencies this lock is
* involved in:
*/
- for (i = 0, entry = list_entries; i < nr_list_entries; i++, entry++) {
+ for_each_set_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) {
+ entry = list_entries + i;
if (entry->class != class && entry->links_to != class)
continue;
+ __clear_bit(i, list_entries_in_use);
+ nr_list_entries--;
list_del_rcu(&entry->entry);
- /* Clear .class and .links_to to avoid double removal. */
- WRITE_ONCE(entry->class, NULL);
- WRITE_ONCE(entry->links_to, NULL);
}
if (list_empty(&class->locks_after) &&
list_empty(&class->locks_before)) {
@@ -4596,6 +4603,7 @@ void __init lockdep_init(void)
(sizeof(lock_classes) +
sizeof(classhash_table) +
sizeof(list_entries) +
+ sizeof(list_entries_in_use) +
sizeof(chainhash_table) +
sizeof(delayed_free)
#ifdef CONFIG_PROVE_LOCKING
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 13/23] locking/lockdep: Introduce lockdep_next_lockchain() and lock_chain_count()
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (11 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 12/23] locking/lockdep: Reuse list entries " Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:10 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 14/23] locking/lockdep: Fix a comment in add_chain_cache() Bart Van Assche
` (10 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
This patch does not change any functionality but makes the next patch in
this series easier to read.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 16 +++++++++++++++-
kernel/locking/lockdep_internals.h | 3 ++-
kernel/locking/lockdep_proc.c | 12 ++++++------
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 4308f0b9ecd5..9c10fcf422f4 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2107,7 +2107,7 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
return 0;
}
-unsigned long nr_lock_chains;
+static unsigned long nr_lock_chains;
struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
int nr_chain_hlocks;
static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
@@ -2241,6 +2241,20 @@ static int check_no_collision(struct task_struct *curr,
return 1;
}
+/*
+ * Given an index that is >= -1, return the index of the next lock chain.
+ * Return -2 if there is no next lock chain.
+ */
+long lockdep_next_lockchain(long i)
+{
+ return i + 1 < nr_lock_chains ? i + 1 : -2;
+}
+
+unsigned long lock_chain_count(void)
+{
+ return nr_lock_chains;
+}
+
/*
* Adds a dependency chain into chain hashtable. And must be called with
* graph_lock held.
diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 88c847a41c8a..ba8a4ac7bd04 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -96,7 +96,8 @@ struct lock_class *lock_chain_get_class(struct lock_chain *chain, int i);
extern unsigned long nr_lock_classes;
extern unsigned long nr_list_entries;
-extern unsigned long nr_lock_chains;
+long lockdep_next_lockchain(long i);
+unsigned long lock_chain_count(void);
extern int nr_chain_hlocks;
extern unsigned long nr_stack_trace_entries;
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index 3d31f9b0059e..9c49ec645d8b 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -104,18 +104,18 @@ static const struct seq_operations lockdep_ops = {
#ifdef CONFIG_PROVE_LOCKING
static void *lc_start(struct seq_file *m, loff_t *pos)
{
+ if (*pos < 0)
+ return NULL;
+
if (*pos == 0)
return SEQ_START_TOKEN;
- if (*pos - 1 < nr_lock_chains)
- return lock_chains + (*pos - 1);
-
- return NULL;
+ return lock_chains + (*pos - 1);
}
static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
{
- (*pos)++;
+ *pos = lockdep_next_lockchain(*pos - 1) + 1;
return lc_start(m, pos);
}
@@ -268,7 +268,7 @@ static int lockdep_stats_show(struct seq_file *m, void *v)
#ifdef CONFIG_PROVE_LOCKING
seq_printf(m, " dependency chains: %11lu [max: %lu]\n",
- nr_lock_chains, MAX_LOCKDEP_CHAINS);
+ lock_chain_count(), MAX_LOCKDEP_CHAINS);
seq_printf(m, " dependency chain hlocks: %11d [max: %lu]\n",
nr_chain_hlocks, MAX_LOCKDEP_CHAIN_HLOCKS);
#endif
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Introduce lockdep_next_lockchain() and lock_chain_count()
2019-02-14 23:00 ` [PATCH v7 13/23] locking/lockdep: Introduce lockdep_next_lockchain() and lock_chain_count() Bart Van Assche
@ 2019-02-28 7:10 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: paulmck, linux-kernel, torvalds, longman, akpm, peterz,
will.deacon, bvanassche, mingo, hpa, johannes, tglx
Commit-ID: 2212684adff79e2704a2792ff46682afb9246fc8
Gitweb: https://git.kernel.org/tip/2212684adff79e2704a2792ff46682afb9246fc8
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:48 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:44 +0100
locking/lockdep: Introduce lockdep_next_lockchain() and lock_chain_count()
This patch does not change any functionality but makes the next patch in
this series easier to read.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-14-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 16 +++++++++++++++-
kernel/locking/lockdep_internals.h | 3 ++-
kernel/locking/lockdep_proc.c | 12 ++++++------
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 2c6d0b67e7b6..753a9b758266 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2096,7 +2096,7 @@ out_bug:
return 0;
}
-unsigned long nr_lock_chains;
+static unsigned long nr_lock_chains;
struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
int nr_chain_hlocks;
static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
@@ -2230,6 +2230,20 @@ static int check_no_collision(struct task_struct *curr,
return 1;
}
+/*
+ * Given an index that is >= -1, return the index of the next lock chain.
+ * Return -2 if there is no next lock chain.
+ */
+long lockdep_next_lockchain(long i)
+{
+ return i + 1 < nr_lock_chains ? i + 1 : -2;
+}
+
+unsigned long lock_chain_count(void)
+{
+ return nr_lock_chains;
+}
+
/*
* Adds a dependency chain into chain hashtable. And must be called with
* graph_lock held.
diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 2ebb9d0ea91c..d4c197425f68 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -100,7 +100,8 @@ struct lock_class *lock_chain_get_class(struct lock_chain *chain, int i);
extern unsigned long nr_lock_classes;
extern unsigned long nr_list_entries;
-extern unsigned long nr_lock_chains;
+long lockdep_next_lockchain(long i);
+unsigned long lock_chain_count(void);
extern int nr_chain_hlocks;
extern unsigned long nr_stack_trace_entries;
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index 3d31f9b0059e..9c49ec645d8b 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -104,18 +104,18 @@ static const struct seq_operations lockdep_ops = {
#ifdef CONFIG_PROVE_LOCKING
static void *lc_start(struct seq_file *m, loff_t *pos)
{
+ if (*pos < 0)
+ return NULL;
+
if (*pos == 0)
return SEQ_START_TOKEN;
- if (*pos - 1 < nr_lock_chains)
- return lock_chains + (*pos - 1);
-
- return NULL;
+ return lock_chains + (*pos - 1);
}
static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
{
- (*pos)++;
+ *pos = lockdep_next_lockchain(*pos - 1) + 1;
return lc_start(m, pos);
}
@@ -268,7 +268,7 @@ static int lockdep_stats_show(struct seq_file *m, void *v)
#ifdef CONFIG_PROVE_LOCKING
seq_printf(m, " dependency chains: %11lu [max: %lu]\n",
- nr_lock_chains, MAX_LOCKDEP_CHAINS);
+ lock_chain_count(), MAX_LOCKDEP_CHAINS);
seq_printf(m, " dependency chain hlocks: %11d [max: %lu]\n",
nr_chain_hlocks, MAX_LOCKDEP_CHAIN_HLOCKS);
#endif
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 14/23] locking/lockdep: Fix a comment in add_chain_cache()
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (12 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 13/23] locking/lockdep: Introduce lockdep_next_lockchain() and lock_chain_count() Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:11 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 15/23] locking/lockdep: Reuse lock chains that have been freed Bart Van Assche
` (9 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Reflect that add_chain_cache() is always called with the graph lock held.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 9c10fcf422f4..e983db2a2032 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2277,7 +2277,7 @@ static inline int add_chain_cache(struct task_struct *curr,
*/
/*
- * We might need to take the graph lock, ensure we've got IRQs
+ * The caller must hold the graph lock, ensure we've got IRQs
* disabled to make this an IRQ-safe lock.. for recursion reasons
* lockdep won't complain about its own locking errors.
*/
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Fix a comment in add_chain_cache()
2019-02-14 23:00 ` [PATCH v7 14/23] locking/lockdep: Fix a comment in add_chain_cache() Bart Van Assche
@ 2019-02-28 7:11 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:11 UTC (permalink / raw)
To: linux-tip-commits
Cc: peterz, johannes, bvanassche, linux-kernel, torvalds, paulmck,
tglx, longman, akpm, mingo, will.deacon, hpa
Commit-ID: 527af3ea273b2cf0c017a2c90090b3c94af8aba4
Gitweb: https://git.kernel.org/tip/527af3ea273b2cf0c017a2c90090b3c94af8aba4
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:49 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:45 +0100
locking/lockdep: Fix a comment in add_chain_cache()
Reflect that add_chain_cache() is always called with the graph lock held.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-15-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 753a9b758266..ec0cb794f70d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2266,7 +2266,7 @@ static inline int add_chain_cache(struct task_struct *curr,
*/
/*
- * We might need to take the graph lock, ensure we've got IRQs
+ * The caller must hold the graph lock, ensure we've got IRQs
* disabled to make this an IRQ-safe lock.. for recursion reasons
* lockdep won't complain about its own locking errors.
*/
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 15/23] locking/lockdep: Reuse lock chains that have been freed
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (13 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 14/23] locking/lockdep: Fix a comment in add_chain_cache() Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:11 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 16/23] locking/lockdep: Check data structure consistency Bart Van Assche
` (8 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
A previous patch introduced a lock chain leak. Fix that leak by reusing
lock chains that have been freed.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 57 ++++++++++++++++++++++++++--------------
1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index e983db2a2032..2ee173820909 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -292,9 +292,12 @@ static LIST_HEAD(free_lock_classes);
/**
* struct pending_free - information about data structures about to be freed
* @zapped: Head of a list with struct lock_class elements.
+ * @lock_chains_being_freed: Bitmap that indicates which lock_chains[] elements
+ * are about to be freed.
*/
struct pending_free {
struct list_head zapped;
+ DECLARE_BITMAP(lock_chains_being_freed, MAX_LOCKDEP_CHAINS);
};
/**
@@ -2107,8 +2110,8 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next)
return 0;
}
-static unsigned long nr_lock_chains;
struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
+static DECLARE_BITMAP(lock_chains_in_use, MAX_LOCKDEP_CHAINS);
int nr_chain_hlocks;
static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
@@ -2247,12 +2250,25 @@ static int check_no_collision(struct task_struct *curr,
*/
long lockdep_next_lockchain(long i)
{
- return i + 1 < nr_lock_chains ? i + 1 : -2;
+ i = find_next_bit(lock_chains_in_use, ARRAY_SIZE(lock_chains), i + 1);
+ return i < ARRAY_SIZE(lock_chains) ? i : -2;
}
unsigned long lock_chain_count(void)
{
- return nr_lock_chains;
+ return bitmap_weight(lock_chains_in_use, ARRAY_SIZE(lock_chains));
+}
+
+/* Must be called with the graph lock held. */
+static struct lock_chain *alloc_lock_chain(void)
+{
+ int idx = find_first_zero_bit(lock_chains_in_use,
+ ARRAY_SIZE(lock_chains));
+
+ if (unlikely(idx >= ARRAY_SIZE(lock_chains)))
+ return NULL;
+ __set_bit(idx, lock_chains_in_use);
+ return lock_chains + idx;
}
/*
@@ -2271,11 +2287,6 @@ static inline int add_chain_cache(struct task_struct *curr,
struct lock_chain *chain;
int i, j;
- /*
- * Allocate a new chain entry from the static array, and add
- * it to the hash:
- */
-
/*
* The caller must hold the graph lock, ensure we've got IRQs
* disabled to make this an IRQ-safe lock.. for recursion reasons
@@ -2284,7 +2295,8 @@ static inline int add_chain_cache(struct task_struct *curr,
if (DEBUG_LOCKS_WARN_ON(!irqs_disabled()))
return 0;
- if (unlikely(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
+ chain = alloc_lock_chain();
+ if (!chain) {
if (!debug_locks_off_graph_unlock())
return 0;
@@ -2292,7 +2304,6 @@ static inline int add_chain_cache(struct task_struct *curr,
dump_stack();
return 0;
}
- chain = lock_chains + nr_lock_chains++;
chain->chain_key = chain_key;
chain->irq_context = hlock->irq_context;
i = get_first_held_lock(curr, hlock);
@@ -4220,7 +4231,8 @@ void lockdep_reset(void)
}
/* Remove a class from a lock chain. Must be called with the graph lock held. */
-static void remove_class_from_lock_chain(struct lock_chain *chain,
+static void remove_class_from_lock_chain(struct pending_free *pf,
+ struct lock_chain *chain,
struct lock_class *class)
{
#ifdef CONFIG_PROVE_LOCKING
@@ -4258,6 +4270,7 @@ static void remove_class_from_lock_chain(struct lock_chain *chain,
* hlist_for_each_entry_rcu() loop is safe.
*/
hlist_del_rcu(&chain->entry);
+ __set_bit(chain - lock_chains, pf->lock_chains_being_freed);
if (chain->depth == 0)
return;
/*
@@ -4266,22 +4279,19 @@ static void remove_class_from_lock_chain(struct lock_chain *chain,
*/
if (lookup_chain_cache(chain_key))
return;
- if (WARN_ON_ONCE(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
+ new_chain = alloc_lock_chain();
+ if (WARN_ON_ONCE(!new_chain)) {
debug_locks_off();
return;
}
- /*
- * Leak *chain because it is not safe to reinsert it before an RCU
- * grace period has expired.
- */
- new_chain = lock_chains + nr_lock_chains++;
*new_chain = *chain;
hlist_add_head_rcu(&new_chain->entry, chainhashentry(chain_key));
#endif
}
/* Must be called with the graph lock held. */
-static void remove_class_from_lock_chains(struct lock_class *class)
+static void remove_class_from_lock_chains(struct pending_free *pf,
+ struct lock_class *class)
{
struct lock_chain *chain;
struct hlist_head *head;
@@ -4290,7 +4300,7 @@ static void remove_class_from_lock_chains(struct lock_class *class)
for (i = 0; i < ARRAY_SIZE(chainhash_table); i++) {
head = chainhash_table + i;
hlist_for_each_entry_rcu(chain, head, entry) {
- remove_class_from_lock_chain(chain, class);
+ remove_class_from_lock_chain(pf, chain, class);
}
}
}
@@ -4329,7 +4339,7 @@ static void zap_class(struct pending_free *pf, struct lock_class *class)
class->name);
}
- remove_class_from_lock_chains(class);
+ remove_class_from_lock_chains(pf, class);
}
static void reinit_class(struct lock_class *class)
@@ -4395,6 +4405,12 @@ static void __free_zapped_classes(struct pending_free *pf)
reinit_class(class);
list_splice_init(&pf->zapped, &free_lock_classes);
+
+#ifdef CONFIG_PROVE_LOCKING
+ bitmap_andnot(lock_chains_in_use, lock_chains_in_use,
+ pf->lock_chains_being_freed, ARRAY_SIZE(lock_chains));
+ bitmap_clear(pf->lock_chains_being_freed, 0, ARRAY_SIZE(lock_chains));
+#endif
}
static void free_zapped_rcu(struct rcu_head *ch)
@@ -4635,6 +4651,7 @@ void __init lockdep_init(void)
#ifdef CONFIG_PROVE_LOCKING
+ sizeof(lock_cq)
+ sizeof(lock_chains)
+ + sizeof(lock_chains_in_use)
+ sizeof(chain_hlocks)
#endif
) / 1024
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Reuse lock chains that have been freed
2019-02-14 23:00 ` [PATCH v7 15/23] locking/lockdep: Reuse lock chains that have been freed Bart Van Assche
@ 2019-02-28 7:11 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:11 UTC (permalink / raw)
To: linux-tip-commits
Cc: johannes, mingo, will.deacon, torvalds, peterz, bvanassche,
linux-kernel, hpa, longman, tglx, paulmck, akpm
Commit-ID: de4643a77356a77bce73f64275b125b4b71a69cf
Gitweb: https://git.kernel.org/tip/de4643a77356a77bce73f64275b125b4b71a69cf
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:50 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:45 +0100
locking/lockdep: Reuse lock chains that have been freed
A previous patch introduced a lock chain leak. Fix that leak by reusing
lock chains that have been freed.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-16-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 57 +++++++++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ec0cb794f70d..0bb204464afe 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -292,9 +292,12 @@ static LIST_HEAD(free_lock_classes);
/**
* struct pending_free - information about data structures about to be freed
* @zapped: Head of a list with struct lock_class elements.
+ * @lock_chains_being_freed: Bitmap that indicates which lock_chains[] elements
+ * are about to be freed.
*/
struct pending_free {
struct list_head zapped;
+ DECLARE_BITMAP(lock_chains_being_freed, MAX_LOCKDEP_CHAINS);
};
/**
@@ -2096,8 +2099,8 @@ out_bug:
return 0;
}
-static unsigned long nr_lock_chains;
struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
+static DECLARE_BITMAP(lock_chains_in_use, MAX_LOCKDEP_CHAINS);
int nr_chain_hlocks;
static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
@@ -2236,12 +2239,25 @@ static int check_no_collision(struct task_struct *curr,
*/
long lockdep_next_lockchain(long i)
{
- return i + 1 < nr_lock_chains ? i + 1 : -2;
+ i = find_next_bit(lock_chains_in_use, ARRAY_SIZE(lock_chains), i + 1);
+ return i < ARRAY_SIZE(lock_chains) ? i : -2;
}
unsigned long lock_chain_count(void)
{
- return nr_lock_chains;
+ return bitmap_weight(lock_chains_in_use, ARRAY_SIZE(lock_chains));
+}
+
+/* Must be called with the graph lock held. */
+static struct lock_chain *alloc_lock_chain(void)
+{
+ int idx = find_first_zero_bit(lock_chains_in_use,
+ ARRAY_SIZE(lock_chains));
+
+ if (unlikely(idx >= ARRAY_SIZE(lock_chains)))
+ return NULL;
+ __set_bit(idx, lock_chains_in_use);
+ return lock_chains + idx;
}
/*
@@ -2260,11 +2276,6 @@ static inline int add_chain_cache(struct task_struct *curr,
struct lock_chain *chain;
int i, j;
- /*
- * Allocate a new chain entry from the static array, and add
- * it to the hash:
- */
-
/*
* The caller must hold the graph lock, ensure we've got IRQs
* disabled to make this an IRQ-safe lock.. for recursion reasons
@@ -2273,7 +2284,8 @@ static inline int add_chain_cache(struct task_struct *curr,
if (DEBUG_LOCKS_WARN_ON(!irqs_disabled()))
return 0;
- if (unlikely(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
+ chain = alloc_lock_chain();
+ if (!chain) {
if (!debug_locks_off_graph_unlock())
return 0;
@@ -2281,7 +2293,6 @@ static inline int add_chain_cache(struct task_struct *curr,
dump_stack();
return 0;
}
- chain = lock_chains + nr_lock_chains++;
chain->chain_key = chain_key;
chain->irq_context = hlock->irq_context;
i = get_first_held_lock(curr, hlock);
@@ -4208,7 +4219,8 @@ void lockdep_reset(void)
}
/* Remove a class from a lock chain. Must be called with the graph lock held. */
-static void remove_class_from_lock_chain(struct lock_chain *chain,
+static void remove_class_from_lock_chain(struct pending_free *pf,
+ struct lock_chain *chain,
struct lock_class *class)
{
#ifdef CONFIG_PROVE_LOCKING
@@ -4246,6 +4258,7 @@ recalc:
* hlist_for_each_entry_rcu() loop is safe.
*/
hlist_del_rcu(&chain->entry);
+ __set_bit(chain - lock_chains, pf->lock_chains_being_freed);
if (chain->depth == 0)
return;
/*
@@ -4254,22 +4267,19 @@ recalc:
*/
if (lookup_chain_cache(chain_key))
return;
- if (WARN_ON_ONCE(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
+ new_chain = alloc_lock_chain();
+ if (WARN_ON_ONCE(!new_chain)) {
debug_locks_off();
return;
}
- /*
- * Leak *chain because it is not safe to reinsert it before an RCU
- * grace period has expired.
- */
- new_chain = lock_chains + nr_lock_chains++;
*new_chain = *chain;
hlist_add_head_rcu(&new_chain->entry, chainhashentry(chain_key));
#endif
}
/* Must be called with the graph lock held. */
-static void remove_class_from_lock_chains(struct lock_class *class)
+static void remove_class_from_lock_chains(struct pending_free *pf,
+ struct lock_class *class)
{
struct lock_chain *chain;
struct hlist_head *head;
@@ -4278,7 +4288,7 @@ static void remove_class_from_lock_chains(struct lock_class *class)
for (i = 0; i < ARRAY_SIZE(chainhash_table); i++) {
head = chainhash_table + i;
hlist_for_each_entry_rcu(chain, head, entry) {
- remove_class_from_lock_chain(chain, class);
+ remove_class_from_lock_chain(pf, chain, class);
}
}
}
@@ -4317,7 +4327,7 @@ static void zap_class(struct pending_free *pf, struct lock_class *class)
class->name);
}
- remove_class_from_lock_chains(class);
+ remove_class_from_lock_chains(pf, class);
}
static void reinit_class(struct lock_class *class)
@@ -4383,6 +4393,12 @@ static void __free_zapped_classes(struct pending_free *pf)
reinit_class(class);
list_splice_init(&pf->zapped, &free_lock_classes);
+
+#ifdef CONFIG_PROVE_LOCKING
+ bitmap_andnot(lock_chains_in_use, lock_chains_in_use,
+ pf->lock_chains_being_freed, ARRAY_SIZE(lock_chains));
+ bitmap_clear(pf->lock_chains_being_freed, 0, ARRAY_SIZE(lock_chains));
+#endif
}
static void free_zapped_rcu(struct rcu_head *ch)
@@ -4623,6 +4639,7 @@ void __init lockdep_init(void)
#ifdef CONFIG_PROVE_LOCKING
+ sizeof(lock_cq)
+ sizeof(lock_chains)
+ + sizeof(lock_chains_in_use)
+ sizeof(chain_hlocks)
#endif
) / 1024
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 16/23] locking/lockdep: Check data structure consistency
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (14 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 15/23] locking/lockdep: Reuse lock chains that have been freed Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:12 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 17/23] locking/lockdep: Verify whether lock objects are small enough to be used as class keys Bart Van Assche
` (7 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Debugging lockdep data structure inconsistencies is challenging. Add
code that verifies data structure consistency at runtime. That code is
disabled by default because it is very CPU intensive.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 167 +++++++++++++++++++++++++++++++++++++++
1 file changed, 167 insertions(+)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 2ee173820909..f5df97812dfa 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -74,6 +74,8 @@ module_param(lock_stat, int, 0644);
#define lock_stat 0
#endif
+static bool check_data_structure_consistency;
+
/*
* lockdep_lock: protects the lockdep graph, the hashes and the
* class/list/hash allocators.
@@ -775,6 +777,168 @@ static bool assign_lock_key(struct lockdep_map *lock)
return true;
}
+/* Check whether element @e occurs in list @h */
+static bool in_list(struct list_head *e, struct list_head *h)
+{
+ struct list_head *f;
+
+ list_for_each(f, h) {
+ if (e == f)
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * Check whether entry @e occurs in any of the locks_after or locks_before
+ * lists.
+ */
+static bool in_any_class_list(struct list_head *e)
+{
+ struct lock_class *class;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ class = &lock_classes[i];
+ if (in_list(e, &class->locks_after) ||
+ in_list(e, &class->locks_before))
+ return true;
+ }
+ return false;
+}
+
+static bool class_lock_list_valid(struct lock_class *c, struct list_head *h)
+{
+ struct lock_list *e;
+
+ list_for_each_entry(e, h, entry) {
+ if (e->links_to != c) {
+ printk(KERN_INFO "class %s: mismatch for lock entry %ld; class %s <> %s",
+ c->name ? : "(?)",
+ (unsigned long)(e - list_entries),
+ e->links_to && e->links_to->name ?
+ e->links_to->name : "(?)",
+ e->class && e->class->name ? e->class->name :
+ "(?)");
+ return false;
+ }
+ }
+ return true;
+}
+
+static u16 chain_hlocks[];
+
+static bool check_lock_chain_key(struct lock_chain *chain)
+{
+#ifdef CONFIG_PROVE_LOCKING
+ u64 chain_key = 0;
+ int i;
+
+ for (i = chain->base; i < chain->base + chain->depth; i++)
+ chain_key = iterate_chain_key(chain_key, chain_hlocks[i] + 1);
+ /*
+ * The 'unsigned long long' casts avoid that a compiler warning
+ * is reported when building tools/lib/lockdep.
+ */
+ if (chain->chain_key != chain_key)
+ printk(KERN_INFO "chain %lld: key %#llx <> %#llx\n",
+ (unsigned long long)(chain - lock_chains),
+ (unsigned long long)chain->chain_key,
+ (unsigned long long)chain_key);
+ return chain->chain_key == chain_key;
+#else
+ return true;
+#endif
+}
+
+static bool in_any_zapped_class_list(struct lock_class *class)
+{
+ struct pending_free *pf;
+ int i;
+
+ for (i = 0, pf = delayed_free.pf; i < ARRAY_SIZE(delayed_free.pf);
+ i++, pf++)
+ if (in_list(&class->lock_entry, &pf->zapped))
+ return true;
+
+ return false;
+}
+
+static bool check_data_structures(void)
+{
+ struct lock_class *class;
+ struct lock_chain *chain;
+ struct hlist_head *head;
+ struct lock_list *e;
+ int i;
+
+ /* Check whether all classes occur in a lock list. */
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ class = &lock_classes[i];
+ if (!in_list(&class->lock_entry, &all_lock_classes) &&
+ !in_list(&class->lock_entry, &free_lock_classes) &&
+ !in_any_zapped_class_list(class)) {
+ printk(KERN_INFO "class %px/%s is not in any class list\n",
+ class, class->name ? : "(?)");
+ return false;
+ return false;
+ }
+ }
+
+ /* Check whether all classes have valid lock lists. */
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ class = &lock_classes[i];
+ if (!class_lock_list_valid(class, &class->locks_before))
+ return false;
+ if (!class_lock_list_valid(class, &class->locks_after))
+ return false;
+ }
+
+ /* Check the chain_key of all lock chains. */
+ for (i = 0; i < ARRAY_SIZE(chainhash_table); i++) {
+ head = chainhash_table + i;
+ hlist_for_each_entry_rcu(chain, head, entry) {
+ if (!check_lock_chain_key(chain))
+ return false;
+ }
+ }
+
+ /*
+ * Check whether all list entries that are in use occur in a class
+ * lock list.
+ */
+ for_each_set_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) {
+ e = list_entries + i;
+ if (!in_any_class_list(&e->entry)) {
+ printk(KERN_INFO "list entry %d is not in any class list; class %s <> %s\n",
+ (unsigned int)(e - list_entries),
+ e->class->name ? : "(?)",
+ e->links_to->name ? : "(?)");
+ return false;
+ }
+ }
+
+ /*
+ * Check whether all list entries that are not in use do not occur in
+ * a class lock list.
+ */
+ for_each_clear_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) {
+ e = list_entries + i;
+ if (in_any_class_list(&e->entry)) {
+ printk(KERN_INFO "list entry %d occurs in a class list; class %s <> %s\n",
+ (unsigned int)(e - list_entries),
+ e->class && e->class->name ? e->class->name :
+ "(?)",
+ e->links_to && e->links_to->name ?
+ e->links_to->name : "(?)");
+ return false;
+ }
+ }
+
+ return true;
+}
+
/*
* Initialize the lock_classes[] array elements, the free_lock_classes list
* and also the delayed_free structure.
@@ -4401,6 +4565,9 @@ static void __free_zapped_classes(struct pending_free *pf)
{
struct lock_class *class;
+ if (check_data_structure_consistency)
+ WARN_ON_ONCE(!check_data_structures());
+
list_for_each_entry(class, &pf->zapped, lock_entry)
reinit_class(class);
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Check data structure consistency
2019-02-14 23:00 ` [PATCH v7 16/23] locking/lockdep: Check data structure consistency Bart Van Assche
@ 2019-02-28 7:12 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: torvalds, will.deacon, tglx, linux-kernel, paulmck, longman,
akpm, johannes, bvanassche, hpa, mingo, peterz
Commit-ID: b526b2e39a53b312f5a6867ce57824247aa0ce8b
Gitweb: https://git.kernel.org/tip/b526b2e39a53b312f5a6867ce57824247aa0ce8b
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:51 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:45 +0100
locking/lockdep: Check data structure consistency
Debugging lockdep data structure inconsistencies is challenging. Add
code that verifies data structure consistency at runtime. That code is
disabled by default because it is very CPU intensive.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-17-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 167 insertions(+)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 0bb204464afe..630be9ac6253 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -74,6 +74,8 @@ module_param(lock_stat, int, 0644);
#define lock_stat 0
#endif
+static bool check_data_structure_consistency;
+
/*
* lockdep_lock: protects the lockdep graph, the hashes and the
* class/list/hash allocators.
@@ -775,6 +777,168 @@ static bool assign_lock_key(struct lockdep_map *lock)
return true;
}
+/* Check whether element @e occurs in list @h */
+static bool in_list(struct list_head *e, struct list_head *h)
+{
+ struct list_head *f;
+
+ list_for_each(f, h) {
+ if (e == f)
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * Check whether entry @e occurs in any of the locks_after or locks_before
+ * lists.
+ */
+static bool in_any_class_list(struct list_head *e)
+{
+ struct lock_class *class;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ class = &lock_classes[i];
+ if (in_list(e, &class->locks_after) ||
+ in_list(e, &class->locks_before))
+ return true;
+ }
+ return false;
+}
+
+static bool class_lock_list_valid(struct lock_class *c, struct list_head *h)
+{
+ struct lock_list *e;
+
+ list_for_each_entry(e, h, entry) {
+ if (e->links_to != c) {
+ printk(KERN_INFO "class %s: mismatch for lock entry %ld; class %s <> %s",
+ c->name ? : "(?)",
+ (unsigned long)(e - list_entries),
+ e->links_to && e->links_to->name ?
+ e->links_to->name : "(?)",
+ e->class && e->class->name ? e->class->name :
+ "(?)");
+ return false;
+ }
+ }
+ return true;
+}
+
+static u16 chain_hlocks[];
+
+static bool check_lock_chain_key(struct lock_chain *chain)
+{
+#ifdef CONFIG_PROVE_LOCKING
+ u64 chain_key = 0;
+ int i;
+
+ for (i = chain->base; i < chain->base + chain->depth; i++)
+ chain_key = iterate_chain_key(chain_key, chain_hlocks[i] + 1);
+ /*
+ * The 'unsigned long long' casts avoid that a compiler warning
+ * is reported when building tools/lib/lockdep.
+ */
+ if (chain->chain_key != chain_key)
+ printk(KERN_INFO "chain %lld: key %#llx <> %#llx\n",
+ (unsigned long long)(chain - lock_chains),
+ (unsigned long long)chain->chain_key,
+ (unsigned long long)chain_key);
+ return chain->chain_key == chain_key;
+#else
+ return true;
+#endif
+}
+
+static bool in_any_zapped_class_list(struct lock_class *class)
+{
+ struct pending_free *pf;
+ int i;
+
+ for (i = 0, pf = delayed_free.pf; i < ARRAY_SIZE(delayed_free.pf);
+ i++, pf++)
+ if (in_list(&class->lock_entry, &pf->zapped))
+ return true;
+
+ return false;
+}
+
+static bool check_data_structures(void)
+{
+ struct lock_class *class;
+ struct lock_chain *chain;
+ struct hlist_head *head;
+ struct lock_list *e;
+ int i;
+
+ /* Check whether all classes occur in a lock list. */
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ class = &lock_classes[i];
+ if (!in_list(&class->lock_entry, &all_lock_classes) &&
+ !in_list(&class->lock_entry, &free_lock_classes) &&
+ !in_any_zapped_class_list(class)) {
+ printk(KERN_INFO "class %px/%s is not in any class list\n",
+ class, class->name ? : "(?)");
+ return false;
+ return false;
+ }
+ }
+
+ /* Check whether all classes have valid lock lists. */
+ for (i = 0; i < ARRAY_SIZE(lock_classes); i++) {
+ class = &lock_classes[i];
+ if (!class_lock_list_valid(class, &class->locks_before))
+ return false;
+ if (!class_lock_list_valid(class, &class->locks_after))
+ return false;
+ }
+
+ /* Check the chain_key of all lock chains. */
+ for (i = 0; i < ARRAY_SIZE(chainhash_table); i++) {
+ head = chainhash_table + i;
+ hlist_for_each_entry_rcu(chain, head, entry) {
+ if (!check_lock_chain_key(chain))
+ return false;
+ }
+ }
+
+ /*
+ * Check whether all list entries that are in use occur in a class
+ * lock list.
+ */
+ for_each_set_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) {
+ e = list_entries + i;
+ if (!in_any_class_list(&e->entry)) {
+ printk(KERN_INFO "list entry %d is not in any class list; class %s <> %s\n",
+ (unsigned int)(e - list_entries),
+ e->class->name ? : "(?)",
+ e->links_to->name ? : "(?)");
+ return false;
+ }
+ }
+
+ /*
+ * Check whether all list entries that are not in use do not occur in
+ * a class lock list.
+ */
+ for_each_clear_bit(i, list_entries_in_use, ARRAY_SIZE(list_entries)) {
+ e = list_entries + i;
+ if (in_any_class_list(&e->entry)) {
+ printk(KERN_INFO "list entry %d occurs in a class list; class %s <> %s\n",
+ (unsigned int)(e - list_entries),
+ e->class && e->class->name ? e->class->name :
+ "(?)",
+ e->links_to && e->links_to->name ?
+ e->links_to->name : "(?)");
+ return false;
+ }
+ }
+
+ return true;
+}
+
/*
* Initialize the lock_classes[] array elements, the free_lock_classes list
* and also the delayed_free structure.
@@ -4389,6 +4553,9 @@ static void __free_zapped_classes(struct pending_free *pf)
{
struct lock_class *class;
+ if (check_data_structure_consistency)
+ WARN_ON_ONCE(!check_data_structures());
+
list_for_each_entry(class, &pf->zapped, lock_entry)
reinit_class(class);
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 17/23] locking/lockdep: Verify whether lock objects are small enough to be used as class keys
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (15 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 16/23] locking/lockdep: Check data structure consistency Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:13 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 18/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (6 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
kernel/locking/lockdep.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index f5df97812dfa..93216e195b4f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -758,6 +758,17 @@ static bool assign_lock_key(struct lockdep_map *lock)
{
unsigned long can_addr, addr = (unsigned long)lock;
+#ifdef __KERNEL__
+ /*
+ * lockdep_free_key_range() assumes that struct lock_class_key
+ * objects do not overlap. Since we use the address of lock
+ * objects as class key for static objects, check whether the
+ * size of lock_class_key objects does not exceed the size of
+ * the smallest lock object.
+ */
+ BUILD_BUG_ON(sizeof(struct lock_class_key) > sizeof(raw_spinlock_t));
+#endif
+
if (__is_kernel_percpu_address(addr, &can_addr))
lock->key = (void *)can_addr;
else if (__is_module_percpu_address(addr, &can_addr))
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Verify whether lock objects are small enough to be used as class keys
2019-02-14 23:00 ` [PATCH v7 17/23] locking/lockdep: Verify whether lock objects are small enough to be used as class keys Bart Van Assche
@ 2019-02-28 7:13 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: bvanassche, mingo, linux-kernel, hpa, longman, akpm, tglx,
johannes, torvalds, paulmck, will.deacon, peterz
Commit-ID: 4bf508621855613ca2ac782f70c3171e0e8bb011
Gitweb: https://git.kernel.org/tip/4bf508621855613ca2ac782f70c3171e0e8bb011
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:52 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:46 +0100
locking/lockdep: Verify whether lock objects are small enough to be used as class keys
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-18-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/locking/lockdep.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 630be9ac6253..84427441824e 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -758,6 +758,17 @@ static bool assign_lock_key(struct lockdep_map *lock)
{
unsigned long can_addr, addr = (unsigned long)lock;
+#ifdef __KERNEL__
+ /*
+ * lockdep_free_key_range() assumes that struct lock_class_key
+ * objects do not overlap. Since we use the address of lock
+ * objects as class key for static objects, check whether the
+ * size of lock_class_key objects does not exceed the size of
+ * the smallest lock object.
+ */
+ BUILD_BUG_ON(sizeof(struct lock_class_key) > sizeof(raw_spinlock_t));
+#endif
+
if (__is_kernel_percpu_address(addr, &can_addr))
lock->key = (void *)can_addr;
else if (__is_module_percpu_address(addr, &can_addr))
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 18/23] locking/lockdep: Add support for dynamic keys
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (16 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 17/23] locking/lockdep: Verify whether lock objects are small enough to be used as class keys Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-26 17:17 ` Peter Zijlstra
2019-02-28 7:13 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 19/23] kernel/workqueue: Use dynamic lockdep keys for workqueues Bart Van Assche
` (5 subsequent siblings)
23 siblings, 2 replies; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
A shortcoming of the current lockdep implementation is that it requires
lock keys to be allocated statically. That forces all instances of lock
objects that occur in a given data structure to share a lock key. Since
lock dependency analysis groups lock objects per key sharing lock keys
can cause false positive lockdep reports. Make it possible to avoid
such false positive reports by allowing lock keys to be allocated
dynamically. Require that dynamically allocated lock keys are
registered before use by calling lockdep_register_key(). Complain about
attempts to register the same lock key pointer twice without calling
lockdep_unregister_key() between successive registration calls.
The purpose of the new lock_keys_hash[] data structure that keeps
track of all dynamic keys is twofold:
- Verify whether the lockdep_register_key() and lockdep_unregister_key()
functions are used correctly.
- Avoid that lockdep_init_map() complains when encountering a dynamically
allocated key.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/lockdep.h | 21 ++++++-
kernel/locking/lockdep.c | 122 ++++++++++++++++++++++++++++++++++++---
2 files changed, 132 insertions(+), 11 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 619ec3f26cdc..43fb35bd7baf 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -46,15 +46,19 @@ extern int lock_stat;
#define NR_LOCKDEP_CACHING_CLASSES 2
/*
- * Lock-classes are keyed via unique addresses, by embedding the
- * lockclass-key into the kernel (or module) .data section. (For
- * static locks we use the lock address itself as the key.)
+ * A lockdep key is associated with each lock object. For static locks we use
+ * the lock address itself as the key. Dynamically allocated lock objects can
+ * have a statically or dynamically allocated key. Dynamically allocated lock
+ * keys must be registered before being used and must be unregistered before
+ * the key memory is freed.
*/
struct lockdep_subclass_key {
char __one_byte;
} __attribute__ ((__packed__));
+/* hash_entry is used to keep track of dynamically allocated keys. */
struct lock_class_key {
+ struct hlist_node hash_entry;
struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES];
};
@@ -273,6 +277,9 @@ extern void lockdep_set_selftest_task(struct task_struct *task);
extern void lockdep_off(void);
extern void lockdep_on(void);
+extern void lockdep_register_key(struct lock_class_key *key);
+extern void lockdep_unregister_key(struct lock_class_key *key);
+
/*
* These methods are used by specific locking variants (spinlocks,
* rwlocks, mutexes and rwsems) to pass init/acquire/release events
@@ -434,6 +441,14 @@ static inline void lockdep_set_selftest_task(struct task_struct *task)
*/
struct lock_class_key { };
+static inline void lockdep_register_key(struct lock_class_key *key)
+{
+}
+
+static inline void lockdep_unregister_key(struct lock_class_key *key)
+{
+}
+
/*
* The lockdep_map takes no space if lockdep is disabled:
*/
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 93216e195b4f..d594866df6f6 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -143,6 +143,9 @@ static DECLARE_BITMAP(list_entries_in_use, MAX_LOCKDEP_ENTRIES);
* nr_lock_classes is the number of elements of lock_classes[] that is
* in use.
*/
+#define KEYHASH_BITS (MAX_LOCKDEP_KEYS_BITS - 1)
+#define KEYHASH_SIZE (1UL << KEYHASH_BITS)
+static struct hlist_head lock_keys_hash[KEYHASH_SIZE];
unsigned long nr_lock_classes;
#ifndef CONFIG_DEBUG_LOCKDEP
static
@@ -641,7 +644,7 @@ static int very_verbose(struct lock_class *class)
* Is this the address of a static object:
*/
#ifdef __KERNEL__
-static int static_obj(void *obj)
+static int static_obj(const void *obj)
{
unsigned long start = (unsigned long) &_stext,
end = (unsigned long) &_end,
@@ -975,6 +978,71 @@ static void init_data_structures_once(void)
}
}
+static inline struct hlist_head *keyhashentry(const struct lock_class_key *key)
+{
+ unsigned long hash = hash_long((uintptr_t)key, KEYHASH_BITS);
+
+ return lock_keys_hash + hash;
+}
+
+/* Register a dynamically allocated key. */
+void lockdep_register_key(struct lock_class_key *key)
+{
+ struct hlist_head *hash_head;
+ struct lock_class_key *k;
+ unsigned long flags;
+
+ if (WARN_ON_ONCE(static_obj(key)))
+ return;
+ hash_head = keyhashentry(key);
+
+ raw_local_irq_save(flags);
+ if (!graph_lock())
+ goto restore_irqs;
+ hlist_for_each_entry_rcu(k, hash_head, hash_entry) {
+ if (WARN_ON_ONCE(k == key))
+ goto out_unlock;
+ }
+ hlist_add_head_rcu(&key->hash_entry, hash_head);
+out_unlock:
+ graph_unlock();
+restore_irqs:
+ raw_local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(lockdep_register_key);
+
+/* Check whether a key has been registered as a dynamic key. */
+static bool is_dynamic_key(const struct lock_class_key *key)
+{
+ struct hlist_head *hash_head;
+ struct lock_class_key *k;
+ bool found = false;
+
+ if (WARN_ON_ONCE(static_obj(key)))
+ return false;
+
+ /*
+ * If lock debugging is disabled lock_keys_hash[] may contain
+ * pointers to memory that has already been freed. Avoid triggering
+ * a use-after-free in that case by returning early.
+ */
+ if (!debug_locks)
+ return true;
+
+ hash_head = keyhashentry(key);
+
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(k, hash_head, hash_entry) {
+ if (k == key) {
+ found = true;
+ break;
+ }
+ }
+ rcu_read_unlock();
+
+ return found;
+}
+
/*
* Register a lock's class in the hash-table, if the class is not present
* yet. Otherwise we look it up. We cache the result in the lock object
@@ -996,7 +1064,7 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
if (!lock->key) {
if (!assign_lock_key(lock))
return NULL;
- } else if (!static_obj(lock->key)) {
+ } else if (!static_obj(lock->key) && !is_dynamic_key(lock->key)) {
return NULL;
}
@@ -3396,13 +3464,13 @@ void lockdep_init_map(struct lockdep_map *lock, const char *name,
if (DEBUG_LOCKS_WARN_ON(!key))
return;
/*
- * Sanity check, the lock-class key must be persistent:
+ * Sanity check, the lock-class key must either have been allocated
+ * statically or must have been registered as a dynamic key.
*/
- if (!static_obj(key)) {
- printk("BUG: key %px not in .data!\n", key);
- /*
- * What it says above ^^^^^, I suggest you read it.
- */
+ if (!static_obj(key) && !is_dynamic_key(key)) {
+ if (debug_locks)
+ printk(KERN_ERR "BUG: key %px has not been registered!\n",
+ key);
DEBUG_LOCKS_WARN_ON(1);
return;
}
@@ -4807,6 +4875,44 @@ void lockdep_reset_lock(struct lockdep_map *lock)
lockdep_reset_lock_reg(lock);
}
+/* Unregister a dynamically allocated key. */
+void lockdep_unregister_key(struct lock_class_key *key)
+{
+ struct hlist_head *hash_head = keyhashentry(key);
+ struct lock_class_key *k;
+ struct pending_free *pf;
+ unsigned long flags;
+ bool found = false;
+
+ might_sleep();
+
+ if (WARN_ON_ONCE(static_obj(key)))
+ return;
+
+ raw_local_irq_save(flags);
+ if (!graph_lock())
+ goto out_irq;
+
+ pf = get_pending_free();
+ hlist_for_each_entry_rcu(k, hash_head, hash_entry) {
+ if (k == key) {
+ hlist_del_rcu(&k->hash_entry);
+ found = true;
+ break;
+ }
+ }
+ WARN_ON_ONCE(!found);
+ __lockdep_free_key_range(pf, key, 1);
+ call_rcu_zapped(pf);
+ graph_unlock();
+out_irq:
+ raw_local_irq_restore(flags);
+
+ /* Wait until is_dynamic_key() has finished accessing k->hash_entry. */
+ synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(lockdep_unregister_key);
+
void __init lockdep_init(void)
{
printk("Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar\n");
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v7 18/23] locking/lockdep: Add support for dynamic keys
2019-02-14 23:00 ` [PATCH v7 18/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
@ 2019-02-26 17:17 ` Peter Zijlstra
2019-02-28 7:13 ` [tip:locking/core] " tip-bot for Bart Van Assche
1 sibling, 0 replies; 59+ messages in thread
From: Peter Zijlstra @ 2019-02-26 17:17 UTC (permalink / raw)
To: Bart Van Assche
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Johannes Berg
On Thu, Feb 14, 2019 at 03:00:53PM -0800, Bart Van Assche wrote:
> +/* hash_entry is used to keep track of dynamically allocated keys. */
> struct lock_class_key {
> + struct hlist_node hash_entry;
> struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES];
> };
I think we can make that:
struct lock_class_key {
union {
struct hlist_node hash_entry;
struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES];
};
};
I've added a patch to that effect at the end. IIRC we never actually
store anything in the subkeys, we just use the address.
^ permalink raw reply [flat|nested] 59+ messages in thread* [tip:locking/core] locking/lockdep: Add support for dynamic keys
2019-02-14 23:00 ` [PATCH v7 18/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
2019-02-26 17:17 ` Peter Zijlstra
@ 2019-02-28 7:13 ` tip-bot for Bart Van Assche
1 sibling, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, johannes, peterz, longman, akpm, torvalds, bvanassche,
paulmck, tglx, linux-kernel, will.deacon, mingo
Commit-ID: 108c14858b9ea224686e476c8f5ec345a0df9e27
Gitweb: https://git.kernel.org/tip/108c14858b9ea224686e476c8f5ec345a0df9e27
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:53 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:47 +0100
locking/lockdep: Add support for dynamic keys
A shortcoming of the current lockdep implementation is that it requires
lock keys to be allocated statically. That forces all instances of lock
objects that occur in a given data structure to share a lock key. Since
lock dependency analysis groups lock objects per key sharing lock keys
can cause false positive lockdep reports. Make it possible to avoid
such false positive reports by allowing lock keys to be allocated
dynamically. Require that dynamically allocated lock keys are
registered before use by calling lockdep_register_key(). Complain about
attempts to register the same lock key pointer twice without calling
lockdep_unregister_key() between successive registration calls.
The purpose of the new lock_keys_hash[] data structure that keeps
track of all dynamic keys is twofold:
- Verify whether the lockdep_register_key() and lockdep_unregister_key()
functions are used correctly.
- Avoid that lockdep_init_map() complains when encountering a dynamically
allocated key.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-19-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/lockdep.h | 21 ++++++--
kernel/locking/lockdep.c | 121 +++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 131 insertions(+), 11 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 619ec3f26cdc..43fb35bd7baf 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -46,15 +46,19 @@ extern int lock_stat;
#define NR_LOCKDEP_CACHING_CLASSES 2
/*
- * Lock-classes are keyed via unique addresses, by embedding the
- * lockclass-key into the kernel (or module) .data section. (For
- * static locks we use the lock address itself as the key.)
+ * A lockdep key is associated with each lock object. For static locks we use
+ * the lock address itself as the key. Dynamically allocated lock objects can
+ * have a statically or dynamically allocated key. Dynamically allocated lock
+ * keys must be registered before being used and must be unregistered before
+ * the key memory is freed.
*/
struct lockdep_subclass_key {
char __one_byte;
} __attribute__ ((__packed__));
+/* hash_entry is used to keep track of dynamically allocated keys. */
struct lock_class_key {
+ struct hlist_node hash_entry;
struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES];
};
@@ -273,6 +277,9 @@ extern void lockdep_set_selftest_task(struct task_struct *task);
extern void lockdep_off(void);
extern void lockdep_on(void);
+extern void lockdep_register_key(struct lock_class_key *key);
+extern void lockdep_unregister_key(struct lock_class_key *key);
+
/*
* These methods are used by specific locking variants (spinlocks,
* rwlocks, mutexes and rwsems) to pass init/acquire/release events
@@ -434,6 +441,14 @@ static inline void lockdep_set_selftest_task(struct task_struct *task)
*/
struct lock_class_key { };
+static inline void lockdep_register_key(struct lock_class_key *key)
+{
+}
+
+static inline void lockdep_unregister_key(struct lock_class_key *key)
+{
+}
+
/*
* The lockdep_map takes no space if lockdep is disabled:
*/
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 84427441824e..c73bc4334bee 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -143,6 +143,9 @@ static DECLARE_BITMAP(list_entries_in_use, MAX_LOCKDEP_ENTRIES);
* nr_lock_classes is the number of elements of lock_classes[] that is
* in use.
*/
+#define KEYHASH_BITS (MAX_LOCKDEP_KEYS_BITS - 1)
+#define KEYHASH_SIZE (1UL << KEYHASH_BITS)
+static struct hlist_head lock_keys_hash[KEYHASH_SIZE];
unsigned long nr_lock_classes;
#ifndef CONFIG_DEBUG_LOCKDEP
static
@@ -641,7 +644,7 @@ static int very_verbose(struct lock_class *class)
* Is this the address of a static object:
*/
#ifdef __KERNEL__
-static int static_obj(void *obj)
+static int static_obj(const void *obj)
{
unsigned long start = (unsigned long) &_stext,
end = (unsigned long) &_end,
@@ -975,6 +978,71 @@ static void init_data_structures_once(void)
}
}
+static inline struct hlist_head *keyhashentry(const struct lock_class_key *key)
+{
+ unsigned long hash = hash_long((uintptr_t)key, KEYHASH_BITS);
+
+ return lock_keys_hash + hash;
+}
+
+/* Register a dynamically allocated key. */
+void lockdep_register_key(struct lock_class_key *key)
+{
+ struct hlist_head *hash_head;
+ struct lock_class_key *k;
+ unsigned long flags;
+
+ if (WARN_ON_ONCE(static_obj(key)))
+ return;
+ hash_head = keyhashentry(key);
+
+ raw_local_irq_save(flags);
+ if (!graph_lock())
+ goto restore_irqs;
+ hlist_for_each_entry_rcu(k, hash_head, hash_entry) {
+ if (WARN_ON_ONCE(k == key))
+ goto out_unlock;
+ }
+ hlist_add_head_rcu(&key->hash_entry, hash_head);
+out_unlock:
+ graph_unlock();
+restore_irqs:
+ raw_local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(lockdep_register_key);
+
+/* Check whether a key has been registered as a dynamic key. */
+static bool is_dynamic_key(const struct lock_class_key *key)
+{
+ struct hlist_head *hash_head;
+ struct lock_class_key *k;
+ bool found = false;
+
+ if (WARN_ON_ONCE(static_obj(key)))
+ return false;
+
+ /*
+ * If lock debugging is disabled lock_keys_hash[] may contain
+ * pointers to memory that has already been freed. Avoid triggering
+ * a use-after-free in that case by returning early.
+ */
+ if (!debug_locks)
+ return true;
+
+ hash_head = keyhashentry(key);
+
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(k, hash_head, hash_entry) {
+ if (k == key) {
+ found = true;
+ break;
+ }
+ }
+ rcu_read_unlock();
+
+ return found;
+}
+
/*
* Register a lock's class in the hash-table, if the class is not present
* yet. Otherwise we look it up. We cache the result in the lock object
@@ -996,7 +1064,7 @@ register_lock_class(struct lockdep_map *lock, unsigned int subclass, int force)
if (!lock->key) {
if (!assign_lock_key(lock))
return NULL;
- } else if (!static_obj(lock->key)) {
+ } else if (!static_obj(lock->key) && !is_dynamic_key(lock->key)) {
return NULL;
}
@@ -3378,13 +3446,12 @@ void lockdep_init_map(struct lockdep_map *lock, const char *name,
if (DEBUG_LOCKS_WARN_ON(!key))
return;
/*
- * Sanity check, the lock-class key must be persistent:
+ * Sanity check, the lock-class key must either have been allocated
+ * statically or must have been registered as a dynamic key.
*/
- if (!static_obj(key)) {
- printk("BUG: key %px not in .data!\n", key);
- /*
- * What it says above ^^^^^, I suggest you read it.
- */
+ if (!static_obj(key) && !is_dynamic_key(key)) {
+ if (debug_locks)
+ printk(KERN_ERR "BUG: key %px has not been registered!\n", key);
DEBUG_LOCKS_WARN_ON(1);
return;
}
@@ -4795,6 +4862,44 @@ void lockdep_reset_lock(struct lockdep_map *lock)
lockdep_reset_lock_reg(lock);
}
+/* Unregister a dynamically allocated key. */
+void lockdep_unregister_key(struct lock_class_key *key)
+{
+ struct hlist_head *hash_head = keyhashentry(key);
+ struct lock_class_key *k;
+ struct pending_free *pf;
+ unsigned long flags;
+ bool found = false;
+
+ might_sleep();
+
+ if (WARN_ON_ONCE(static_obj(key)))
+ return;
+
+ raw_local_irq_save(flags);
+ if (!graph_lock())
+ goto out_irq;
+
+ pf = get_pending_free();
+ hlist_for_each_entry_rcu(k, hash_head, hash_entry) {
+ if (k == key) {
+ hlist_del_rcu(&k->hash_entry);
+ found = true;
+ break;
+ }
+ }
+ WARN_ON_ONCE(!found);
+ __lockdep_free_key_range(pf, key, 1);
+ call_rcu_zapped(pf);
+ graph_unlock();
+out_irq:
+ raw_local_irq_restore(flags);
+
+ /* Wait until is_dynamic_key() has finished accessing k->hash_entry. */
+ synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(lockdep_unregister_key);
+
void __init lockdep_init(void)
{
printk("Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar\n");
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 19/23] kernel/workqueue: Use dynamic lockdep keys for workqueues
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (17 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 18/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:14 ` [tip:locking/core] " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 20/23] locking/spinlock: Introduce spin_lock_init_key() Bart Van Assche
` (4 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche
Commit 87915adc3f0a ("workqueue: re-add lockdep dependencies for flushing")
improved deadlock checking in the workqueue implementation. Unfortunately
that patch also introduced a few false positive lockdep complaints. This
patch suppresses these false positives by allocating the workqueue mutex
lockdep key dynamically. An example of a false positive lockdep complaint
suppressed by this report can be found below. The root cause of the
lockdep complaint shown below is that the direct I/O code can call
alloc_workqueue() from inside a work item created by another
alloc_workqueue() call and that both workqueues share the same lockdep
key. This patch avoids that that lockdep complaint is triggered by
allocating the work queue lockdep keys dynamically. In other words, this
patch guarantees that a unique lockdep key is associated with each work
queue mutex.
======================================================
WARNING: possible circular locking dependency detected
4.19.0-dbg+ #1 Not tainted
------------------------------------------------------
fio/4129 is trying to acquire lock:
00000000a01cfe1a ((wq_completion)"dio/%s"sb->s_id){+.+.}, at: flush_workqueue+0xd0/0x970
but task is already holding lock:
00000000a0acecf9 (&sb->s_type->i_mutex_key#14){+.+.}, at: ext4_file_write_iter+0x154/0x710
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (&sb->s_type->i_mutex_key#14){+.+.}:
down_write+0x3d/0x80
__generic_file_fsync+0x77/0xf0
ext4_sync_file+0x3c9/0x780
vfs_fsync_range+0x66/0x100
dio_complete+0x2f5/0x360
dio_aio_complete_work+0x1c/0x20
process_one_work+0x481/0x9f0
worker_thread+0x63/0x5a0
kthread+0x1cf/0x1f0
ret_from_fork+0x24/0x30
-> #1 ((work_completion)(&dio->complete_work)){+.+.}:
process_one_work+0x447/0x9f0
worker_thread+0x63/0x5a0
kthread+0x1cf/0x1f0
ret_from_fork+0x24/0x30
-> #0 ((wq_completion)"dio/%s"sb->s_id){+.+.}:
lock_acquire+0xc5/0x200
flush_workqueue+0xf3/0x970
drain_workqueue+0xec/0x220
destroy_workqueue+0x23/0x350
sb_init_dio_done_wq+0x6a/0x80
do_blockdev_direct_IO+0x1f33/0x4be0
__blockdev_direct_IO+0x79/0x86
ext4_direct_IO+0x5df/0xbb0
generic_file_direct_write+0x119/0x220
__generic_file_write_iter+0x131/0x2d0
ext4_file_write_iter+0x3fa/0x710
aio_write+0x235/0x330
io_submit_one+0x510/0xeb0
__x64_sys_io_submit+0x122/0x340
do_syscall_64+0x71/0x220
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
(wq_completion)"dio/%s"sb->s_id --> (work_completion)(&dio->complete_work) --> &sb->s_type->i_mutex_key#14
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&sb->s_type->i_mutex_key#14);
lock((work_completion)(&dio->complete_work));
lock(&sb->s_type->i_mutex_key#14);
lock((wq_completion)"dio/%s"sb->s_id);
*** DEADLOCK ***
1 lock held by fio/4129:
#0: 00000000a0acecf9 (&sb->s_type->i_mutex_key#14){+.+.}, at: ext4_file_write_iter+0x154/0x710
stack backtrace:
CPU: 3 PID: 4129 Comm: fio Not tainted 4.19.0-dbg+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
dump_stack+0x86/0xc5
print_circular_bug.isra.32+0x20a/0x218
__lock_acquire+0x1c68/0x1cf0
lock_acquire+0xc5/0x200
flush_workqueue+0xf3/0x970
drain_workqueue+0xec/0x220
destroy_workqueue+0x23/0x350
sb_init_dio_done_wq+0x6a/0x80
do_blockdev_direct_IO+0x1f33/0x4be0
__blockdev_direct_IO+0x79/0x86
ext4_direct_IO+0x5df/0xbb0
generic_file_direct_write+0x119/0x220
__generic_file_write_iter+0x131/0x2d0
ext4_file_write_iter+0x3fa/0x710
aio_write+0x235/0x330
io_submit_one+0x510/0xeb0
__x64_sys_io_submit+0x122/0x340
do_syscall_64+0x71/0x220
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/workqueue.h | 28 +++----------------
kernel/workqueue.c | 59 +++++++++++++++++++++++++++++++++------
2 files changed, 54 insertions(+), 33 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 60d673e15632..d9a1a480e920 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -390,43 +390,23 @@ extern struct workqueue_struct *system_freezable_wq;
extern struct workqueue_struct *system_power_efficient_wq;
extern struct workqueue_struct *system_freezable_power_efficient_wq;
-extern struct workqueue_struct *
-__alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active,
- struct lock_class_key *key, const char *lock_name, ...) __printf(1, 6);
-
/**
* alloc_workqueue - allocate a workqueue
* @fmt: printf format for the name of the workqueue
* @flags: WQ_* flags
* @max_active: max in-flight work items, 0 for default
- * @args...: args for @fmt
+ * remaining args: args for @fmt
*
* Allocate a workqueue with the specified parameters. For detailed
* information on WQ_* flags, please refer to
* Documentation/core-api/workqueue.rst.
*
- * The __lock_name macro dance is to guarantee that single lock_class_key
- * doesn't end up with different namesm, which isn't allowed by lockdep.
- *
* RETURNS:
* Pointer to the allocated workqueue on success, %NULL on failure.
*/
-#ifdef CONFIG_LOCKDEP
-#define alloc_workqueue(fmt, flags, max_active, args...) \
-({ \
- static struct lock_class_key __key; \
- const char *__lock_name; \
- \
- __lock_name = "(wq_completion)"#fmt#args; \
- \
- __alloc_workqueue_key((fmt), (flags), (max_active), \
- &__key, __lock_name, ##args); \
-})
-#else
-#define alloc_workqueue(fmt, flags, max_active, args...) \
- __alloc_workqueue_key((fmt), (flags), (max_active), \
- NULL, NULL, ##args)
-#endif
+struct workqueue_struct *alloc_workqueue(const char *fmt,
+ unsigned int flags,
+ int max_active, ...);
/**
* alloc_ordered_workqueue - allocate an ordered workqueue
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index fc5d23d752a5..e163e7a7f5e5 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -259,6 +259,8 @@ struct workqueue_struct {
struct wq_device *wq_dev; /* I: for sysfs interface */
#endif
#ifdef CONFIG_LOCKDEP
+ char *lock_name;
+ struct lock_class_key key;
struct lockdep_map lockdep_map;
#endif
char name[WQ_NAME_LEN]; /* I: workqueue name */
@@ -3337,11 +3339,49 @@ static int init_worker_pool(struct worker_pool *pool)
return 0;
}
+#ifdef CONFIG_LOCKDEP
+static void wq_init_lockdep(struct workqueue_struct *wq)
+{
+ char *lock_name;
+
+ lockdep_register_key(&wq->key);
+ lock_name = kasprintf(GFP_KERNEL, "%s%s", "(wq_completion)", wq->name);
+ if (!lock_name)
+ lock_name = wq->name;
+ lockdep_init_map(&wq->lockdep_map, lock_name, &wq->key, 0);
+}
+
+static void wq_unregister_lockdep(struct workqueue_struct *wq)
+{
+ lockdep_unregister_key(&wq->key);
+}
+
+static void wq_free_lockdep(struct workqueue_struct *wq)
+{
+ if (wq->lock_name != wq->name)
+ kfree(wq->lock_name);
+}
+#else
+static void wq_init_lockdep(struct workqueue_struct *wq)
+{
+}
+
+static void wq_unregister_lockdep(struct workqueue_struct *wq)
+{
+}
+
+static void wq_free_lockdep(struct workqueue_struct *wq)
+{
+}
+#endif
+
static void rcu_free_wq(struct rcu_head *rcu)
{
struct workqueue_struct *wq =
container_of(rcu, struct workqueue_struct, rcu);
+ wq_free_lockdep(wq);
+
if (!(wq->flags & WQ_UNBOUND))
free_percpu(wq->cpu_pwqs);
else
@@ -3532,8 +3572,10 @@ static void pwq_unbound_release_workfn(struct work_struct *work)
* If we're the last pwq going away, @wq is already dead and no one
* is gonna access it anymore. Schedule RCU free.
*/
- if (is_last)
+ if (is_last) {
+ wq_unregister_lockdep(wq);
call_rcu(&wq->rcu, rcu_free_wq);
+ }
}
/**
@@ -4067,11 +4109,9 @@ static int init_rescuer(struct workqueue_struct *wq)
return 0;
}
-struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
- unsigned int flags,
- int max_active,
- struct lock_class_key *key,
- const char *lock_name, ...)
+struct workqueue_struct *alloc_workqueue(const char *fmt,
+ unsigned int flags,
+ int max_active, ...)
{
size_t tbl_size = 0;
va_list args;
@@ -4106,7 +4146,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
goto err_free_wq;
}
- va_start(args, lock_name);
+ va_start(args, max_active);
vsnprintf(wq->name, sizeof(wq->name), fmt, args);
va_end(args);
@@ -4123,7 +4163,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
INIT_LIST_HEAD(&wq->flusher_overflow);
INIT_LIST_HEAD(&wq->maydays);
- lockdep_init_map(&wq->lockdep_map, lock_name, key, 0);
+ wq_init_lockdep(wq);
INIT_LIST_HEAD(&wq->list);
if (alloc_and_link_pwqs(wq) < 0)
@@ -4161,7 +4201,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
destroy_workqueue(wq);
return NULL;
}
-EXPORT_SYMBOL_GPL(__alloc_workqueue_key);
+EXPORT_SYMBOL_GPL(alloc_workqueue);
/**
* destroy_workqueue - safely terminate a workqueue
@@ -4214,6 +4254,7 @@ void destroy_workqueue(struct workqueue_struct *wq)
kthread_stop(wq->rescuer->task);
if (!(wq->flags & WQ_UNBOUND)) {
+ wq_unregister_lockdep(wq);
/*
* The base ref is never dropped on per-cpu pwqs. Directly
* schedule RCU free.
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] kernel/workqueue: Use dynamic lockdep keys for workqueues
2019-02-14 23:00 ` [PATCH v7 19/23] kernel/workqueue: Use dynamic lockdep keys for workqueues Bart Van Assche
@ 2019-02-28 7:14 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:14 UTC (permalink / raw)
To: linux-tip-commits
Cc: johannes.berg, bvanassche, linux-kernel, will.deacon, peterz,
paulmck, tj, akpm, tglx, mingo, longman, hpa, torvalds
Commit-ID: 669de8bda87b92ab9a2fc663b3f5743c2ad1ae9f
Gitweb: https://git.kernel.org/tip/669de8bda87b92ab9a2fc663b3f5743c2ad1ae9f
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:54 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:47 +0100
kernel/workqueue: Use dynamic lockdep keys for workqueues
The following commit:
87915adc3f0a ("workqueue: re-add lockdep dependencies for flushing")
improved deadlock checking in the workqueue implementation. Unfortunately
that patch also introduced a few false positive lockdep complaints.
This patch suppresses these false positives by allocating the workqueue mutex
lockdep key dynamically.
An example of a false positive lockdep complaint suppressed by this patch
can be found below. The root cause of the lockdep complaint shown below
is that the direct I/O code can call alloc_workqueue() from inside a work
item created by another alloc_workqueue() call and that both workqueues
share the same lockdep key. This patch avoids that that lockdep complaint
is triggered by allocating the work queue lockdep keys dynamically.
In other words, this patch guarantees that a unique lockdep key is
associated with each work queue mutex.
======================================================
WARNING: possible circular locking dependency detected
4.19.0-dbg+ #1 Not tainted
fio/4129 is trying to acquire lock:
00000000a01cfe1a ((wq_completion)"dio/%s"sb->s_id){+.+.}, at: flush_workqueue+0xd0/0x970
but task is already holding lock:
00000000a0acecf9 (&sb->s_type->i_mutex_key#14){+.+.}, at: ext4_file_write_iter+0x154/0x710
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (&sb->s_type->i_mutex_key#14){+.+.}:
down_write+0x3d/0x80
__generic_file_fsync+0x77/0xf0
ext4_sync_file+0x3c9/0x780
vfs_fsync_range+0x66/0x100
dio_complete+0x2f5/0x360
dio_aio_complete_work+0x1c/0x20
process_one_work+0x481/0x9f0
worker_thread+0x63/0x5a0
kthread+0x1cf/0x1f0
ret_from_fork+0x24/0x30
-> #1 ((work_completion)(&dio->complete_work)){+.+.}:
process_one_work+0x447/0x9f0
worker_thread+0x63/0x5a0
kthread+0x1cf/0x1f0
ret_from_fork+0x24/0x30
-> #0 ((wq_completion)"dio/%s"sb->s_id){+.+.}:
lock_acquire+0xc5/0x200
flush_workqueue+0xf3/0x970
drain_workqueue+0xec/0x220
destroy_workqueue+0x23/0x350
sb_init_dio_done_wq+0x6a/0x80
do_blockdev_direct_IO+0x1f33/0x4be0
__blockdev_direct_IO+0x79/0x86
ext4_direct_IO+0x5df/0xbb0
generic_file_direct_write+0x119/0x220
__generic_file_write_iter+0x131/0x2d0
ext4_file_write_iter+0x3fa/0x710
aio_write+0x235/0x330
io_submit_one+0x510/0xeb0
__x64_sys_io_submit+0x122/0x340
do_syscall_64+0x71/0x220
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
(wq_completion)"dio/%s"sb->s_id --> (work_completion)(&dio->complete_work) --> &sb->s_type->i_mutex_key#14
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&sb->s_type->i_mutex_key#14);
lock((work_completion)(&dio->complete_work));
lock(&sb->s_type->i_mutex_key#14);
lock((wq_completion)"dio/%s"sb->s_id);
*** DEADLOCK ***
1 lock held by fio/4129:
#0: 00000000a0acecf9 (&sb->s_type->i_mutex_key#14){+.+.}, at: ext4_file_write_iter+0x154/0x710
stack backtrace:
CPU: 3 PID: 4129 Comm: fio Not tainted 4.19.0-dbg+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
dump_stack+0x86/0xc5
print_circular_bug.isra.32+0x20a/0x218
__lock_acquire+0x1c68/0x1cf0
lock_acquire+0xc5/0x200
flush_workqueue+0xf3/0x970
drain_workqueue+0xec/0x220
destroy_workqueue+0x23/0x350
sb_init_dio_done_wq+0x6a/0x80
do_blockdev_direct_IO+0x1f33/0x4be0
__blockdev_direct_IO+0x79/0x86
ext4_direct_IO+0x5df/0xbb0
generic_file_direct_write+0x119/0x220
__generic_file_write_iter+0x131/0x2d0
ext4_file_write_iter+0x3fa/0x710
aio_write+0x235/0x330
io_submit_one+0x510/0xeb0
__x64_sys_io_submit+0x122/0x340
do_syscall_64+0x71/0x220
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Link: https://lkml.kernel.org/r/20190214230058.196511-20-bvanassche@acm.org
[ Reworked the changelog a bit. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/workqueue.h | 28 ++++------------------
kernel/workqueue.c | 59 +++++++++++++++++++++++++++++++++++++++--------
2 files changed, 54 insertions(+), 33 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 60d673e15632..d9a1a480e920 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -390,43 +390,23 @@ extern struct workqueue_struct *system_freezable_wq;
extern struct workqueue_struct *system_power_efficient_wq;
extern struct workqueue_struct *system_freezable_power_efficient_wq;
-extern struct workqueue_struct *
-__alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active,
- struct lock_class_key *key, const char *lock_name, ...) __printf(1, 6);
-
/**
* alloc_workqueue - allocate a workqueue
* @fmt: printf format for the name of the workqueue
* @flags: WQ_* flags
* @max_active: max in-flight work items, 0 for default
- * @args...: args for @fmt
+ * remaining args: args for @fmt
*
* Allocate a workqueue with the specified parameters. For detailed
* information on WQ_* flags, please refer to
* Documentation/core-api/workqueue.rst.
*
- * The __lock_name macro dance is to guarantee that single lock_class_key
- * doesn't end up with different namesm, which isn't allowed by lockdep.
- *
* RETURNS:
* Pointer to the allocated workqueue on success, %NULL on failure.
*/
-#ifdef CONFIG_LOCKDEP
-#define alloc_workqueue(fmt, flags, max_active, args...) \
-({ \
- static struct lock_class_key __key; \
- const char *__lock_name; \
- \
- __lock_name = "(wq_completion)"#fmt#args; \
- \
- __alloc_workqueue_key((fmt), (flags), (max_active), \
- &__key, __lock_name, ##args); \
-})
-#else
-#define alloc_workqueue(fmt, flags, max_active, args...) \
- __alloc_workqueue_key((fmt), (flags), (max_active), \
- NULL, NULL, ##args)
-#endif
+struct workqueue_struct *alloc_workqueue(const char *fmt,
+ unsigned int flags,
+ int max_active, ...);
/**
* alloc_ordered_workqueue - allocate an ordered workqueue
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index fc5d23d752a5..e163e7a7f5e5 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -259,6 +259,8 @@ struct workqueue_struct {
struct wq_device *wq_dev; /* I: for sysfs interface */
#endif
#ifdef CONFIG_LOCKDEP
+ char *lock_name;
+ struct lock_class_key key;
struct lockdep_map lockdep_map;
#endif
char name[WQ_NAME_LEN]; /* I: workqueue name */
@@ -3337,11 +3339,49 @@ static int init_worker_pool(struct worker_pool *pool)
return 0;
}
+#ifdef CONFIG_LOCKDEP
+static void wq_init_lockdep(struct workqueue_struct *wq)
+{
+ char *lock_name;
+
+ lockdep_register_key(&wq->key);
+ lock_name = kasprintf(GFP_KERNEL, "%s%s", "(wq_completion)", wq->name);
+ if (!lock_name)
+ lock_name = wq->name;
+ lockdep_init_map(&wq->lockdep_map, lock_name, &wq->key, 0);
+}
+
+static void wq_unregister_lockdep(struct workqueue_struct *wq)
+{
+ lockdep_unregister_key(&wq->key);
+}
+
+static void wq_free_lockdep(struct workqueue_struct *wq)
+{
+ if (wq->lock_name != wq->name)
+ kfree(wq->lock_name);
+}
+#else
+static void wq_init_lockdep(struct workqueue_struct *wq)
+{
+}
+
+static void wq_unregister_lockdep(struct workqueue_struct *wq)
+{
+}
+
+static void wq_free_lockdep(struct workqueue_struct *wq)
+{
+}
+#endif
+
static void rcu_free_wq(struct rcu_head *rcu)
{
struct workqueue_struct *wq =
container_of(rcu, struct workqueue_struct, rcu);
+ wq_free_lockdep(wq);
+
if (!(wq->flags & WQ_UNBOUND))
free_percpu(wq->cpu_pwqs);
else
@@ -3532,8 +3572,10 @@ static void pwq_unbound_release_workfn(struct work_struct *work)
* If we're the last pwq going away, @wq is already dead and no one
* is gonna access it anymore. Schedule RCU free.
*/
- if (is_last)
+ if (is_last) {
+ wq_unregister_lockdep(wq);
call_rcu(&wq->rcu, rcu_free_wq);
+ }
}
/**
@@ -4067,11 +4109,9 @@ static int init_rescuer(struct workqueue_struct *wq)
return 0;
}
-struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
- unsigned int flags,
- int max_active,
- struct lock_class_key *key,
- const char *lock_name, ...)
+struct workqueue_struct *alloc_workqueue(const char *fmt,
+ unsigned int flags,
+ int max_active, ...)
{
size_t tbl_size = 0;
va_list args;
@@ -4106,7 +4146,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
goto err_free_wq;
}
- va_start(args, lock_name);
+ va_start(args, max_active);
vsnprintf(wq->name, sizeof(wq->name), fmt, args);
va_end(args);
@@ -4123,7 +4163,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
INIT_LIST_HEAD(&wq->flusher_overflow);
INIT_LIST_HEAD(&wq->maydays);
- lockdep_init_map(&wq->lockdep_map, lock_name, key, 0);
+ wq_init_lockdep(wq);
INIT_LIST_HEAD(&wq->list);
if (alloc_and_link_pwqs(wq) < 0)
@@ -4161,7 +4201,7 @@ err_destroy:
destroy_workqueue(wq);
return NULL;
}
-EXPORT_SYMBOL_GPL(__alloc_workqueue_key);
+EXPORT_SYMBOL_GPL(alloc_workqueue);
/**
* destroy_workqueue - safely terminate a workqueue
@@ -4214,6 +4254,7 @@ void destroy_workqueue(struct workqueue_struct *wq)
kthread_stop(wq->rescuer->task);
if (!(wq->flags & WQ_UNBOUND)) {
+ wq_unregister_lockdep(wq);
/*
* The base ref is never dropped on per-cpu pwqs. Directly
* schedule RCU free.
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 20/23] locking/spinlock: Introduce spin_lock_init_key()
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (18 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 19/23] kernel/workqueue: Use dynamic lockdep keys for workqueues Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint Bart Van Assche
` (3 subsequent siblings)
23 siblings, 0 replies; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Ingo Molnar
Some code uses nested locking of different spinlocks that share a
lock class. That results in false positives because spin_lock_init()
forces these instances to share a lock class. Make it possible to
avoid these false positives by allowing spinlock users to specify
the lock class at runtime.
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/spinlock.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index e089157dcf97..09b3e27ed21d 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -99,10 +99,19 @@ do { \
\
__raw_spin_lock_init((lock), #lock, &__key); \
} while (0)
+#define raw_spin_lock_init_key(lock, key) \
+ __raw_spin_lock_init((lock), #lock, key)
#else
+
# define raw_spin_lock_init(lock) \
do { *(lock) = __RAW_SPIN_LOCK_UNLOCKED(lock); } while (0)
+static inline void raw_spin_lock_init_key(struct raw_spinlock *lock,
+ struct lock_class_key *key)
+{
+ *(lock) = __RAW_SPIN_LOCK_UNLOCKED(lock);
+}
+
#endif
#define raw_spin_is_locked(lock) arch_spin_is_locked(&(lock)->raw_lock)
@@ -324,6 +333,12 @@ do { \
raw_spin_lock_init(&(_lock)->rlock); \
} while (0)
+#define spin_lock_init_key(_lock, _key) \
+do { \
+ spinlock_check(_lock); \
+ raw_spin_lock_init_key(&(_lock)->rlock, _key); \
+} while (0)
+
static __always_inline void spin_lock(spinlock_t *lock)
{
raw_spin_lock(&lock->rlock);
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (19 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 20/23] locking/spinlock: Introduce spin_lock_init_key() Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-15 2:26 ` Ming Lei
2019-02-26 17:24 ` Peter Zijlstra
2019-02-14 23:00 ` [PATCH v7 22/23] lockdep tests: Fix run_tests.sh Bart Van Assche
` (2 subsequent siblings)
23 siblings, 2 replies; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Jens Axboe, Ming Lei, Theodore Ts'o
Avoid that running test nvme/012 from the blktests suite triggers the
following false positive lockdep complaint:
============================================
WARNING: possible recursive locking detected
5.0.0-rc3-xfstests-00015-g1236f7d60242 #841 Not tainted
--------------------------------------------
ksoftirqd/1/16 is trying to acquire lock:
000000000282032e (&(&fq->mq_flush_lock)->rlock){..-.}, at: flush_end_io+0x4e/0x1d0
but task is already holding lock:
00000000cbadcbc2 (&(&fq->mq_flush_lock)->rlock){..-.}, at: flush_end_io+0x4e/0x1d0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&(&fq->mq_flush_lock)->rlock);
lock(&(&fq->mq_flush_lock)->rlock);
*** DEADLOCK ***
May be due to missing lock nesting notation
1 lock held by ksoftirqd/1/16:
#0: 00000000cbadcbc2 (&(&fq->mq_flush_lock)->rlock){..-.}, at: flush_end_io+0x4e/0x1d0
stack backtrace:
CPU: 1 PID: 16 Comm: ksoftirqd/1 Not tainted 5.0.0-rc3-xfstests-00015-g1236f7d60242 #841
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
dump_stack+0x67/0x90
__lock_acquire.cold.45+0x2b4/0x313
lock_acquire+0x98/0x160
_raw_spin_lock_irqsave+0x3b/0x80
flush_end_io+0x4e/0x1d0
blk_mq_complete_request+0x76/0x110
nvmet_req_complete+0x15/0x110 [nvmet]
nvmet_bio_done+0x27/0x50 [nvmet]
blk_update_request+0xd7/0x2d0
blk_mq_end_request+0x1a/0x100
blk_flush_complete_seq+0xe5/0x350
flush_end_io+0x12f/0x1d0
blk_done_softirq+0x9f/0xd0
__do_softirq+0xca/0x440
run_ksoftirqd+0x24/0x50
smpboot_thread_fn+0x113/0x1e0
kthread+0x121/0x140
ret_from_fork+0x3a/0x50
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
block/blk-flush.c | 5 ++++-
block/blk.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 6e0f2d97fc6d..86c86c76c087 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -70,6 +70,7 @@
#include <linux/blkdev.h>
#include <linux/gfp.h>
#include <linux/blk-mq.h>
+#include <linux/lockdep.h>
#include "blk.h"
#include "blk-mq.h"
@@ -472,7 +473,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(struct request_queue *q,
if (!fq)
goto fail;
- spin_lock_init(&fq->mq_flush_lock);
+ lockdep_register_key(&fq->key);
+ spin_lock_init_key(&fq->mq_flush_lock, &fq->key);
rq_sz = round_up(rq_sz + cmd_size, cache_line_size());
fq->flush_rq = kzalloc_node(rq_sz, flags, node);
@@ -497,6 +499,7 @@ void blk_free_flush_queue(struct blk_flush_queue *fq)
if (!fq)
return;
+ lockdep_unregister_key(&fq->key);
kfree(fq->flush_rq);
kfree(fq);
}
diff --git a/block/blk.h b/block/blk.h
index 848278c52030..10f5e19aa4a1 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -28,6 +28,7 @@ struct blk_flush_queue {
* at the same time
*/
struct request *orig_rq;
+ struct lock_class_key key;
spinlock_t mq_flush_lock;
};
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-14 23:00 ` [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint Bart Van Assche
@ 2019-02-15 2:26 ` Ming Lei
2019-02-15 16:08 ` Bart Van Assche
2019-02-26 18:08 ` Peter Zijlstra
2019-02-26 17:24 ` Peter Zijlstra
1 sibling, 2 replies; 59+ messages in thread
From: Ming Lei @ 2019-02-15 2:26 UTC (permalink / raw)
To: Bart Van Assche
Cc: peterz, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Thu, Feb 14, 2019 at 03:00:56PM -0800, Bart Van Assche wrote:
> Avoid that running test nvme/012 from the blktests suite triggers the
> following false positive lockdep complaint:
>
> ============================================
> WARNING: possible recursive locking detected
> 5.0.0-rc3-xfstests-00015-g1236f7d60242 #841 Not tainted
> --------------------------------------------
> ksoftirqd/1/16 is trying to acquire lock:
> 000000000282032e (&(&fq->mq_flush_lock)->rlock){..-.}, at: flush_end_io+0x4e/0x1d0
>
> but task is already holding lock:
> 00000000cbadcbc2 (&(&fq->mq_flush_lock)->rlock){..-.}, at: flush_end_io+0x4e/0x1d0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&(&fq->mq_flush_lock)->rlock);
> lock(&(&fq->mq_flush_lock)->rlock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 1 lock held by ksoftirqd/1/16:
> #0: 00000000cbadcbc2 (&(&fq->mq_flush_lock)->rlock){..-.}, at: flush_end_io+0x4e/0x1d0
>
> stack backtrace:
> CPU: 1 PID: 16 Comm: ksoftirqd/1 Not tainted 5.0.0-rc3-xfstests-00015-g1236f7d60242 #841
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> dump_stack+0x67/0x90
> __lock_acquire.cold.45+0x2b4/0x313
> lock_acquire+0x98/0x160
> _raw_spin_lock_irqsave+0x3b/0x80
> flush_end_io+0x4e/0x1d0
> blk_mq_complete_request+0x76/0x110
> nvmet_req_complete+0x15/0x110 [nvmet]
> nvmet_bio_done+0x27/0x50 [nvmet]
> blk_update_request+0xd7/0x2d0
> blk_mq_end_request+0x1a/0x100
> blk_flush_complete_seq+0xe5/0x350
> flush_end_io+0x12f/0x1d0
> blk_done_softirq+0x9f/0xd0
> __do_softirq+0xca/0x440
> run_ksoftirqd+0x24/0x50
> smpboot_thread_fn+0x113/0x1e0
> kthread+0x121/0x140
> ret_from_fork+0x3a/0x50
>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> block/blk-flush.c | 5 ++++-
> block/blk.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-flush.c b/block/blk-flush.c
> index 6e0f2d97fc6d..86c86c76c087 100644
> --- a/block/blk-flush.c
> +++ b/block/blk-flush.c
> @@ -70,6 +70,7 @@
> #include <linux/blkdev.h>
> #include <linux/gfp.h>
> #include <linux/blk-mq.h>
> +#include <linux/lockdep.h>
>
> #include "blk.h"
> #include "blk-mq.h"
> @@ -472,7 +473,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(struct request_queue *q,
> if (!fq)
> goto fail;
>
> - spin_lock_init(&fq->mq_flush_lock);
> + lockdep_register_key(&fq->key);
> + spin_lock_init_key(&fq->mq_flush_lock, &fq->key);
>
> rq_sz = round_up(rq_sz + cmd_size, cache_line_size());
> fq->flush_rq = kzalloc_node(rq_sz, flags, node);
> @@ -497,6 +499,7 @@ void blk_free_flush_queue(struct blk_flush_queue *fq)
> if (!fq)
> return;
>
> + lockdep_unregister_key(&fq->key);
> kfree(fq->flush_rq);
> kfree(fq);
> }
> diff --git a/block/blk.h b/block/blk.h
> index 848278c52030..10f5e19aa4a1 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -28,6 +28,7 @@ struct blk_flush_queue {
> * at the same time
> */
> struct request *orig_rq;
> + struct lock_class_key key;
> spinlock_t mq_flush_lock;
> };
>
Hi Bart,
Did you look at the following comment?
https://marc.info/?l=linux-block&m=155014828206209&w=2
There might be lots of blk_flush_queue instance which is allocated
for each hctx, then lots of class key slot may be wasted.
So I suggest to use one nvmet_loop_flush_lock_key for this particular issue,
something like the following patch:
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 4aac1b4a8112..ec4248c12ed9 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -524,7 +524,9 @@ static const struct nvme_ctrl_ops nvme_loop_ctrl_ops = {
static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
{
- int ret;
+ static struct lock_class_key nvme_loop_flush_lock_key;
+ int ret, i;
+ struct blk_mq_hw_ctx *hctx;
ret = nvme_loop_init_io_queues(ctrl);
if (ret)
@@ -553,6 +555,10 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
goto out_free_tagset;
}
+ queue_for_each_hw_ctx(ctrl->ctrl.connect_q, hctx, i)
+ lockdep_set_class(&hctx->fq->mq_flush_lock,
+ &nvme_loop_flush_lock_key);
+
ret = nvme_loop_connect_io_queues(ctrl);
if (ret)
goto out_cleanup_connect_q;
--
Ming
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-15 2:26 ` Ming Lei
@ 2019-02-15 16:08 ` Bart Van Assche
2019-02-17 13:23 ` Ming Lei
2019-02-26 18:08 ` Peter Zijlstra
1 sibling, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-15 16:08 UTC (permalink / raw)
To: Ming Lei
Cc: peterz, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Fri, 2019-02-15 at 10:26 +0800, Ming Lei wrote:
> There might be lots of blk_flush_queue instance which is allocated
> for each hctx, then lots of class key slot may be wasted.
>
> So I suggest to use one nvmet_loop_flush_lock_key for this particular issue,
> something like the following patch:
>
> diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
> index 4aac1b4a8112..ec4248c12ed9 100644
> --- a/drivers/nvme/target/loop.c
> +++ b/drivers/nvme/target/loop.c
> @@ -524,7 +524,9 @@ static const struct nvme_ctrl_ops nvme_loop_ctrl_ops = {
>
> static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
> {
> - int ret;
> + static struct lock_class_key nvme_loop_flush_lock_key;
> + int ret, i;
> + struct blk_mq_hw_ctx *hctx;
>
> ret = nvme_loop_init_io_queues(ctrl);
> if (ret)
> @@ -553,6 +555,10 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
> goto out_free_tagset;
> }
>
> + queue_for_each_hw_ctx(ctrl->ctrl.connect_q, hctx, i)
> + lockdep_set_class(&hctx->fq->mq_flush_lock,
> + &nvme_loop_flush_lock_key);
> +
> ret = nvme_loop_connect_io_queues(ctrl);
> if (ret)
> goto out_cleanup_connect_q;
Hi Ming,
Thanks for your feedback.
Are you aware that sizeof(struct lock_class_key) is zero if lockdep is
disabled? Does this alleviate your concern?
I'm not enthusiast about your patch. I don't think that block layer users
should touch the lock class key of the flush queue. That's a key that should
be set by the block layer core.
Anyway, let's drop patches 20/23 and 21/23 from this series and let's
continue this discussion on the linux-block mailing list after agreement has
been reached about the "dynamic lockdep key" approach.
Bart.
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-15 16:08 ` Bart Van Assche
@ 2019-02-17 13:23 ` Ming Lei
0 siblings, 0 replies; 59+ messages in thread
From: Ming Lei @ 2019-02-17 13:23 UTC (permalink / raw)
To: Bart Van Assche
Cc: peterz, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Fri, Feb 15, 2019 at 08:08:08AM -0800, Bart Van Assche wrote:
> On Fri, 2019-02-15 at 10:26 +0800, Ming Lei wrote:
> > There might be lots of blk_flush_queue instance which is allocated
> > for each hctx, then lots of class key slot may be wasted.
> >
> > So I suggest to use one nvmet_loop_flush_lock_key for this particular issue,
> > something like the following patch:
> >
> > diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
> > index 4aac1b4a8112..ec4248c12ed9 100644
> > --- a/drivers/nvme/target/loop.c
> > +++ b/drivers/nvme/target/loop.c
> > @@ -524,7 +524,9 @@ static const struct nvme_ctrl_ops nvme_loop_ctrl_ops = {
> >
> > static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
> > {
> > - int ret;
> > + static struct lock_class_key nvme_loop_flush_lock_key;
> > + int ret, i;
> > + struct blk_mq_hw_ctx *hctx;
> >
> > ret = nvme_loop_init_io_queues(ctrl);
> > if (ret)
> > @@ -553,6 +555,10 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
> > goto out_free_tagset;
> > }
> >
> > + queue_for_each_hw_ctx(ctrl->ctrl.connect_q, hctx, i)
> > + lockdep_set_class(&hctx->fq->mq_flush_lock,
> > + &nvme_loop_flush_lock_key);
> > +
> > ret = nvme_loop_connect_io_queues(ctrl);
> > if (ret)
> > goto out_cleanup_connect_q;
>
> Hi Ming,
>
> Thanks for your feedback.
>
> Are you aware that sizeof(struct lock_class_key) is zero if lockdep is
> disabled?
Yes.
> Does this alleviate your concern?
No, I mean in case of CONFIG_LOCKDP.
1) MAX_LOCKDEP_KEYS is defined as 8k-1
2) lock validation runs as graph search algorithm actually, and each lock
class acts as one graph vertex.
So more lock class will make the lock validation much slower, also lock
class key may be overflow.
>
> I'm not enthusiast about your patch. I don't think that block layer users
> should touch the lock class key of the flush queue. That's a key that should
> be set by the block layer core.
Why? lockdep_set_class is used tree-wide actually.
Thanks,
Ming
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-15 2:26 ` Ming Lei
2019-02-15 16:08 ` Bart Van Assche
@ 2019-02-26 18:08 ` Peter Zijlstra
2019-02-27 1:35 ` Ming Lei
1 sibling, 1 reply; 59+ messages in thread
From: Peter Zijlstra @ 2019-02-26 18:08 UTC (permalink / raw)
To: Ming Lei
Cc: Bart Van Assche, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Fri, Feb 15, 2019 at 10:26:59AM +0800, Ming Lei wrote:
> There might be lots of blk_flush_queue instance which is allocated
> for each hctx, then lots of class key slot may be wasted.
What is 'lots' ? for someone who doesn't really know all that much about
the block layer.
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-26 18:08 ` Peter Zijlstra
@ 2019-02-27 1:35 ` Ming Lei
2019-02-27 14:24 ` Peter Zijlstra
0 siblings, 1 reply; 59+ messages in thread
From: Ming Lei @ 2019-02-27 1:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Bart Van Assche, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Tue, Feb 26, 2019 at 07:08:02PM +0100, Peter Zijlstra wrote:
> On Fri, Feb 15, 2019 at 10:26:59AM +0800, Ming Lei wrote:
> > There might be lots of blk_flush_queue instance which is allocated
> > for each hctx, then lots of class key slot may be wasted.
>
> What is 'lots' ? for someone who doesn't really know all that much about
> the block layer.
Each hw queue has one instance of blk_flush_queue, and one device may
has lots of hw queues(may be > all possible cpus, such as nvme), and there
may be lots of block devices in one system.
Suppose one system has 10 NVMe hosts, 8 disks attached to each host, and
256 CPU cores in the system, there can be 10 * 8 * 256 = 20K instances of
blk_flush_queue.
Not mention there are other block devices(loop, nbd, scsi, ...) in the system.
That is why I suggest to use one single lock class for addressing this
nvme loop specific issue:
https://marc.info/?l=linux-kernel&m=155019765724564&w=2
Thanks,
Ming
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-27 1:35 ` Ming Lei
@ 2019-02-27 14:24 ` Peter Zijlstra
2019-02-27 15:53 ` Ming Lei
0 siblings, 1 reply; 59+ messages in thread
From: Peter Zijlstra @ 2019-02-27 14:24 UTC (permalink / raw)
To: Ming Lei
Cc: Bart Van Assche, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Wed, Feb 27, 2019 at 09:35:56AM +0800, Ming Lei wrote:
> On Tue, Feb 26, 2019 at 07:08:02PM +0100, Peter Zijlstra wrote:
> > On Fri, Feb 15, 2019 at 10:26:59AM +0800, Ming Lei wrote:
> > > There might be lots of blk_flush_queue instance which is allocated
> > > for each hctx, then lots of class key slot may be wasted.
> >
> > What is 'lots' ? for someone who doesn't really know all that much about
> > the block layer.
>
> Each hw queue has one instance of blk_flush_queue, and one device may
> has lots of hw queues(may be > all possible cpus, such as nvme), and there
> may be lots of block devices in one system.
>
> Suppose one system has 10 NVMe hosts, 8 disks attached to each host, and
> 256 CPU cores in the system, there can be 10 * 8 * 256 = 20K instances of
> blk_flush_queue.
>
> Not mention there are other block devices(loop, nbd, scsi, ...) in the system.
>
> That is why I suggest to use one single lock class for addressing this
> nvme loop specific issue:
>
> https://marc.info/?l=linux-kernel&m=155019765724564&w=2
Right; that is rather a lot. But what causes the recursion, and thus how
is it specific to NVME ?
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-27 14:24 ` Peter Zijlstra
@ 2019-02-27 15:53 ` Ming Lei
0 siblings, 0 replies; 59+ messages in thread
From: Ming Lei @ 2019-02-27 15:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Bart Van Assche, mingo, will.deacon, tj, longman, johannes.berg,
linux-kernel, Jens Axboe, Theodore Ts'o
On Wed, Feb 27, 2019 at 03:24:51PM +0100, Peter Zijlstra wrote:
> On Wed, Feb 27, 2019 at 09:35:56AM +0800, Ming Lei wrote:
> > On Tue, Feb 26, 2019 at 07:08:02PM +0100, Peter Zijlstra wrote:
> > > On Fri, Feb 15, 2019 at 10:26:59AM +0800, Ming Lei wrote:
> > > > There might be lots of blk_flush_queue instance which is allocated
> > > > for each hctx, then lots of class key slot may be wasted.
> > >
> > > What is 'lots' ? for someone who doesn't really know all that much about
> > > the block layer.
> >
> > Each hw queue has one instance of blk_flush_queue, and one device may
> > has lots of hw queues(may be > all possible cpus, such as nvme), and there
> > may be lots of block devices in one system.
> >
> > Suppose one system has 10 NVMe hosts, 8 disks attached to each host, and
> > 256 CPU cores in the system, there can be 10 * 8 * 256 = 20K instances of
> > blk_flush_queue.
> >
> > Not mention there are other block devices(loop, nbd, scsi, ...) in the system.
> >
> > That is why I suggest to use one single lock class for addressing this
> > nvme loop specific issue:
> >
> > https://marc.info/?l=linux-kernel&m=155019765724564&w=2
>
> Right; that is rather a lot. But what causes the recursion, and thus how
> is it specific to NVME ?
The recursion is nvme-loop specific, see the following link:
https://marc.info/?l=linux-block&m=155003205030566&w=2
Thanks,
Ming
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-14 23:00 ` [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint Bart Van Assche
2019-02-15 2:26 ` Ming Lei
@ 2019-02-26 17:24 ` Peter Zijlstra
2019-02-26 17:48 ` Bart Van Assche
1 sibling, 1 reply; 59+ messages in thread
From: Peter Zijlstra @ 2019-02-26 17:24 UTC (permalink / raw)
To: Bart Van Assche
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Jens Axboe, Ming Lei, Theodore Ts'o
On Thu, Feb 14, 2019 at 03:00:56PM -0800, Bart Van Assche wrote:
> @@ -472,7 +473,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(struct request_queue *q,
> if (!fq)
> goto fail;
>
> - spin_lock_init(&fq->mq_flush_lock);
> + lockdep_register_key(&fq->key);
> + spin_lock_init_key(&fq->mq_flush_lock, &fq->key);
What's wrong with:
spin_lock_init(&fq->wq_flush_lock);
lockdep_register_key(&fq->key);
lockdep_set_class(&fq->wq_flush_lock, &fq->key);
?
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint
2019-02-26 17:24 ` Peter Zijlstra
@ 2019-02-26 17:48 ` Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: Bart Van Assche @ 2019-02-26 17:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Jens Axboe, Ming Lei, Theodore Ts'o
On Tue, 2019-02-26 at 18:24 +0100, Peter Zijlstra wrote:
> On Thu, Feb 14, 2019 at 03:00:56PM -0800, Bart Van Assche wrote:
> > @@ -472,7 +473,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(struct request_queue *q,
> > if (!fq)
> > goto fail;
> >
> > - spin_lock_init(&fq->mq_flush_lock);
> > + lockdep_register_key(&fq->key);
> > + spin_lock_init_key(&fq->mq_flush_lock, &fq->key);
>
> What's wrong with:
>
> spin_lock_init(&fq->wq_flush_lock);
> lockdep_register_key(&fq->key);
> lockdep_set_class(&fq->wq_flush_lock, &fq->key);
>
> ?
Hi Peter,
That's an approach that I had not yet considered. I'm fine with the
lockdep_set_class() version.
Bart.
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v7 22/23] lockdep tests: Fix run_tests.sh
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (20 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 21/23] block: Avoid that flushing triggers a lockdep complaint Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:15 ` [tip:locking/core] lockdep/lib/tests: " tip-bot for Bart Van Assche
2019-02-14 23:00 ` [PATCH v7 23/23] lockdep tests: Test dynamic key registration Bart Van Assche
2019-02-21 22:02 ` [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Apparently the execute bits were set for the tests/*.sh scripts on my
test setup but these are not set in the kernel tree. Fix this by adding
the interpreter path in front of the script paths.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Fixes: 5ecb8e94b494 ("tools/lib/lockdep/tests: Improve testing accuracy") # v5.0-rc1
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
tools/lib/lockdep/run_tests.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/lib/lockdep/run_tests.sh b/tools/lib/lockdep/run_tests.sh
index c8fbd0306960..11f425662b43 100755
--- a/tools/lib/lockdep/run_tests.sh
+++ b/tools/lib/lockdep/run_tests.sh
@@ -11,7 +11,7 @@ find tests -name '*.c' | sort | while read -r i; do
testname=$(basename "$i" .c)
echo -ne "$testname... "
if gcc -o "tests/$testname" -pthread "$i" liblockdep.a -Iinclude -D__USE_LIBLOCKDEP &&
- timeout 1 "tests/$testname" 2>&1 | "tests/${testname}.sh"; then
+ timeout 1 "tests/$testname" 2>&1 | /bin/bash "tests/${testname}.sh"; then
echo "PASSED!"
else
echo "FAILED!"
@@ -24,7 +24,7 @@ find tests -name '*.c' | sort | while read -r i; do
echo -ne "(PRELOAD) $testname... "
if gcc -o "tests/$testname" -pthread -Iinclude "$i" &&
timeout 1 ./lockdep "tests/$testname" 2>&1 |
- "tests/${testname}.sh"; then
+ /bin/bash "tests/${testname}.sh"; then
echo "PASSED!"
else
echo "FAILED!"
@@ -37,7 +37,7 @@ find tests -name '*.c' | sort | while read -r i; do
echo -ne "(PRELOAD + Valgrind) $testname... "
if gcc -o "tests/$testname" -pthread -Iinclude "$i" &&
{ timeout 10 valgrind --read-var-info=yes ./lockdep "./tests/$testname" >& "tests/${testname}.vg.out"; true; } &&
- "tests/${testname}.sh" < "tests/${testname}.vg.out" &&
+ /bin/bash "tests/${testname}.sh" < "tests/${testname}.vg.out" &&
! grep -Eq '(^==[0-9]*== (Invalid |Uninitialised ))|Mismatched free|Source and destination overlap| UME ' "tests/${testname}.vg.out"; then
echo "PASSED!"
else
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] lockdep/lib/tests: Fix run_tests.sh
2019-02-14 23:00 ` [PATCH v7 22/23] lockdep tests: Fix run_tests.sh Bart Van Assche
@ 2019-02-28 7:15 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:15 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, will.deacon, linux-kernel, bvanassche, paulmck, longman,
torvalds, tglx, akpm, peterz, johannes, hpa
Commit-ID: d93ac78bf7b37db36fa00225f8e9a14c7ed1b2ba
Gitweb: https://git.kernel.org/tip/d93ac78bf7b37db36fa00225f8e9a14c7ed1b2ba
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:57 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:48 +0100
lockdep/lib/tests: Fix run_tests.sh
Apparently the execute bits were set for the tests/*.sh scripts on my
test setup but these are not set in the kernel tree. Fix this by adding
the interpreter path in front of the script paths.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Fixes: 5ecb8e94b494 ("tools/lib/lockdep/tests: Improve testing accuracy") # v5.0-rc1
Link: https://lkml.kernel.org/r/20190214230058.196511-23-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
tools/lib/lockdep/run_tests.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/lib/lockdep/run_tests.sh b/tools/lib/lockdep/run_tests.sh
index c8fbd0306960..11f425662b43 100755
--- a/tools/lib/lockdep/run_tests.sh
+++ b/tools/lib/lockdep/run_tests.sh
@@ -11,7 +11,7 @@ find tests -name '*.c' | sort | while read -r i; do
testname=$(basename "$i" .c)
echo -ne "$testname... "
if gcc -o "tests/$testname" -pthread "$i" liblockdep.a -Iinclude -D__USE_LIBLOCKDEP &&
- timeout 1 "tests/$testname" 2>&1 | "tests/${testname}.sh"; then
+ timeout 1 "tests/$testname" 2>&1 | /bin/bash "tests/${testname}.sh"; then
echo "PASSED!"
else
echo "FAILED!"
@@ -24,7 +24,7 @@ find tests -name '*.c' | sort | while read -r i; do
echo -ne "(PRELOAD) $testname... "
if gcc -o "tests/$testname" -pthread -Iinclude "$i" &&
timeout 1 ./lockdep "tests/$testname" 2>&1 |
- "tests/${testname}.sh"; then
+ /bin/bash "tests/${testname}.sh"; then
echo "PASSED!"
else
echo "FAILED!"
@@ -37,7 +37,7 @@ find tests -name '*.c' | sort | while read -r i; do
echo -ne "(PRELOAD + Valgrind) $testname... "
if gcc -o "tests/$testname" -pthread -Iinclude "$i" &&
{ timeout 10 valgrind --read-var-info=yes ./lockdep "./tests/$testname" >& "tests/${testname}.vg.out"; true; } &&
- "tests/${testname}.sh" < "tests/${testname}.vg.out" &&
+ /bin/bash "tests/${testname}.sh" < "tests/${testname}.vg.out" &&
! grep -Eq '(^==[0-9]*== (Invalid |Uninitialised ))|Mismatched free|Source and destination overlap| UME ' "tests/${testname}.vg.out"; then
echo "PASSED!"
else
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v7 23/23] lockdep tests: Test dynamic key registration
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (21 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 22/23] lockdep tests: Fix run_tests.sh Bart Van Assche
@ 2019-02-14 23:00 ` Bart Van Assche
2019-02-28 7:15 ` [tip:locking/core] lockdep/lib/tests: " tip-bot for Bart Van Assche
2019-02-21 22:02 ` [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-14 23:00 UTC (permalink / raw)
To: peterz
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel,
Bart Van Assche, Johannes Berg
Make sure that the lockdep_register_key() and lockdep_unregister_key()
code is tested when running the lockdep tests.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
tools/lib/lockdep/include/liblockdep/common.h | 2 ++
tools/lib/lockdep/include/liblockdep/mutex.h | 11 ++++++-----
tools/lib/lockdep/tests/ABBA.c | 9 +++++++++
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/tools/lib/lockdep/include/liblockdep/common.h b/tools/lib/lockdep/include/liblockdep/common.h
index d640a9761f09..a81d91d4fc78 100644
--- a/tools/lib/lockdep/include/liblockdep/common.h
+++ b/tools/lib/lockdep/include/liblockdep/common.h
@@ -45,6 +45,8 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
void lock_release(struct lockdep_map *lock, int nested,
unsigned long ip);
void lockdep_reset_lock(struct lockdep_map *lock);
+void lockdep_register_key(struct lock_class_key *key);
+void lockdep_unregister_key(struct lock_class_key *key);
extern void debug_check_no_locks_freed(const void *from, unsigned long len);
#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
diff --git a/tools/lib/lockdep/include/liblockdep/mutex.h b/tools/lib/lockdep/include/liblockdep/mutex.h
index 2073d4e1f2f0..783dd0df06f9 100644
--- a/tools/lib/lockdep/include/liblockdep/mutex.h
+++ b/tools/lib/lockdep/include/liblockdep/mutex.h
@@ -7,6 +7,7 @@
struct liblockdep_pthread_mutex {
pthread_mutex_t mutex;
+ struct lock_class_key key;
struct lockdep_map dep_map;
};
@@ -27,11 +28,10 @@ static inline int __mutex_init(liblockdep_pthread_mutex_t *lock,
return pthread_mutex_init(&lock->mutex, __mutexattr);
}
-#define liblockdep_pthread_mutex_init(mutex, mutexattr) \
-({ \
- static struct lock_class_key __key; \
- \
- __mutex_init((mutex), #mutex, &__key, (mutexattr)); \
+#define liblockdep_pthread_mutex_init(mutex, mutexattr) \
+({ \
+ lockdep_register_key(&(mutex)->key); \
+ __mutex_init((mutex), #mutex, &(mutex)->key, (mutexattr)); \
})
static inline int liblockdep_pthread_mutex_lock(liblockdep_pthread_mutex_t *lock)
@@ -55,6 +55,7 @@ static inline int liblockdep_pthread_mutex_trylock(liblockdep_pthread_mutex_t *l
static inline int liblockdep_pthread_mutex_destroy(liblockdep_pthread_mutex_t *lock)
{
lockdep_reset_lock(&lock->dep_map);
+ lockdep_unregister_key(&lock->key);
return pthread_mutex_destroy(&lock->mutex);
}
diff --git a/tools/lib/lockdep/tests/ABBA.c b/tools/lib/lockdep/tests/ABBA.c
index 623313f54720..543789bc3e37 100644
--- a/tools/lib/lockdep/tests/ABBA.c
+++ b/tools/lib/lockdep/tests/ABBA.c
@@ -14,4 +14,13 @@ void main(void)
pthread_mutex_destroy(&b);
pthread_mutex_destroy(&a);
+
+ pthread_mutex_init(&a, NULL);
+ pthread_mutex_init(&b, NULL);
+
+ LOCK_UNLOCK_2(a, b);
+ LOCK_UNLOCK_2(b, a);
+
+ pthread_mutex_destroy(&b);
+ pthread_mutex_destroy(&a);
}
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 59+ messages in thread* [tip:locking/core] lockdep/lib/tests: Test dynamic key registration
2019-02-14 23:00 ` [PATCH v7 23/23] lockdep tests: Test dynamic key registration Bart Van Assche
@ 2019-02-28 7:15 ` tip-bot for Bart Van Assche
0 siblings, 0 replies; 59+ messages in thread
From: tip-bot for Bart Van Assche @ 2019-02-28 7:15 UTC (permalink / raw)
To: linux-tip-commits
Cc: torvalds, longman, mingo, johannes, bvanassche, will.deacon, hpa,
akpm, linux-kernel, peterz, paulmck, tglx
Commit-ID: f214737b75b0ee79763b5c058b9d5e83d711348d
Gitweb: https://git.kernel.org/tip/f214737b75b0ee79763b5c058b9d5e83d711348d
Author: Bart Van Assche <bvanassche@acm.org>
AuthorDate: Thu, 14 Feb 2019 15:00:58 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 28 Feb 2019 07:55:48 +0100
lockdep/lib/tests: Test dynamic key registration
Make sure that the lockdep_register_key() and lockdep_unregister_key()
code is tested when running the lockdep tests.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <longman@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: johannes.berg@intel.com
Cc: tj@kernel.org
Link: https://lkml.kernel.org/r/20190214230058.196511-24-bvanassche@acm.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
tools/lib/lockdep/include/liblockdep/common.h | 2 ++
tools/lib/lockdep/include/liblockdep/mutex.h | 11 ++++++-----
tools/lib/lockdep/tests/ABBA.c | 9 +++++++++
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/tools/lib/lockdep/include/liblockdep/common.h b/tools/lib/lockdep/include/liblockdep/common.h
index d640a9761f09..a81d91d4fc78 100644
--- a/tools/lib/lockdep/include/liblockdep/common.h
+++ b/tools/lib/lockdep/include/liblockdep/common.h
@@ -45,6 +45,8 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
void lock_release(struct lockdep_map *lock, int nested,
unsigned long ip);
void lockdep_reset_lock(struct lockdep_map *lock);
+void lockdep_register_key(struct lock_class_key *key);
+void lockdep_unregister_key(struct lock_class_key *key);
extern void debug_check_no_locks_freed(const void *from, unsigned long len);
#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
diff --git a/tools/lib/lockdep/include/liblockdep/mutex.h b/tools/lib/lockdep/include/liblockdep/mutex.h
index 2073d4e1f2f0..783dd0df06f9 100644
--- a/tools/lib/lockdep/include/liblockdep/mutex.h
+++ b/tools/lib/lockdep/include/liblockdep/mutex.h
@@ -7,6 +7,7 @@
struct liblockdep_pthread_mutex {
pthread_mutex_t mutex;
+ struct lock_class_key key;
struct lockdep_map dep_map;
};
@@ -27,11 +28,10 @@ static inline int __mutex_init(liblockdep_pthread_mutex_t *lock,
return pthread_mutex_init(&lock->mutex, __mutexattr);
}
-#define liblockdep_pthread_mutex_init(mutex, mutexattr) \
-({ \
- static struct lock_class_key __key; \
- \
- __mutex_init((mutex), #mutex, &__key, (mutexattr)); \
+#define liblockdep_pthread_mutex_init(mutex, mutexattr) \
+({ \
+ lockdep_register_key(&(mutex)->key); \
+ __mutex_init((mutex), #mutex, &(mutex)->key, (mutexattr)); \
})
static inline int liblockdep_pthread_mutex_lock(liblockdep_pthread_mutex_t *lock)
@@ -55,6 +55,7 @@ static inline int liblockdep_pthread_mutex_trylock(liblockdep_pthread_mutex_t *l
static inline int liblockdep_pthread_mutex_destroy(liblockdep_pthread_mutex_t *lock)
{
lockdep_reset_lock(&lock->dep_map);
+ lockdep_unregister_key(&lock->key);
return pthread_mutex_destroy(&lock->mutex);
}
diff --git a/tools/lib/lockdep/tests/ABBA.c b/tools/lib/lockdep/tests/ABBA.c
index 623313f54720..543789bc3e37 100644
--- a/tools/lib/lockdep/tests/ABBA.c
+++ b/tools/lib/lockdep/tests/ABBA.c
@@ -14,4 +14,13 @@ void main(void)
pthread_mutex_destroy(&b);
pthread_mutex_destroy(&a);
+
+ pthread_mutex_init(&a, NULL);
+ pthread_mutex_init(&b, NULL);
+
+ LOCK_UNLOCK_2(a, b);
+ LOCK_UNLOCK_2(b, a);
+
+ pthread_mutex_destroy(&b);
+ pthread_mutex_destroy(&a);
}
^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys
2019-02-14 23:00 [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
` (22 preceding siblings ...)
2019-02-14 23:00 ` [PATCH v7 23/23] lockdep tests: Test dynamic key registration Bart Van Assche
@ 2019-02-21 22:02 ` Bart Van Assche
2019-02-22 16:26 ` Peter Zijlstra
23 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-21 22:02 UTC (permalink / raw)
To: peterz; +Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel
On Thu, 2019-02-14 at 15:00 -0800, Bart Van Assche wrote:
> A known shortcoming of the current lockdep implementation is that it requires
> lock keys to be allocated statically. This forces certain unrelated
> synchronization objects to share keys and this key sharing can cause false
> positive deadlock reports. This patch series adds support for dynamic keys in
> the lockdep code and eliminates a class of false positive reports from the
> workqueue implementation.
>
> Please consider these patches for kernel v5.1.
Hi Peter and Ingo,
Do you have any feedback about this patch series that you would like to share?
If none of you has the time to do a full review of this patch series before the
v5.1 merge window opens: how about queuing only the first ten patches of this
patch series for kernel v5.1? The first ten patches of this series are small and
easy to review.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys
2019-02-21 22:02 ` [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys Bart Van Assche
@ 2019-02-22 16:26 ` Peter Zijlstra
2019-02-22 17:20 ` Bart Van Assche
0 siblings, 1 reply; 59+ messages in thread
From: Peter Zijlstra @ 2019-02-22 16:26 UTC (permalink / raw)
To: Bart Van Assche
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel
On Thu, Feb 21, 2019 at 02:02:05PM -0800, Bart Van Assche wrote:
> On Thu, 2019-02-14 at 15:00 -0800, Bart Van Assche wrote:
> > A known shortcoming of the current lockdep implementation is that it requires
> > lock keys to be allocated statically. This forces certain unrelated
> > synchronization objects to share keys and this key sharing can cause false
> > positive deadlock reports. This patch series adds support for dynamic keys in
> > the lockdep code and eliminates a class of false positive reports from the
> > workqueue implementation.
> >
> > Please consider these patches for kernel v5.1.
>
> Hi Peter and Ingo,
>
> Do you have any feedback about this patch series that you would like to share?
I've gone over all and I think it looks ok now; I'll give it another
round tomorrow^Wmonday and then queue bits.
So far the only changes I've made are the below. I'm not entirely sure
on the unconditional validity check on DEBUG_LOCKDEP, maybe I'll add a
boot param for that.
---
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -75,8 +75,6 @@ module_param(lock_stat, int, 0644);
#define lock_stat 0
#endif
-static bool check_data_structure_consistency;
-
/*
* lockdep_lock: protects the lockdep graph, the hashes and the
* class/list/hash allocators.
@@ -792,6 +790,8 @@ static bool assign_lock_key(struct lockd
return true;
}
+#ifdef CONFIG_DEBUG_LOCKDEP
+
/* Check whether element @e occurs in list @h */
static bool in_list(struct list_head *e, struct list_head *h)
{
@@ -856,15 +856,15 @@ static bool check_lock_chain_key(struct
* The 'unsigned long long' casts avoid that a compiler warning
* is reported when building tools/lib/lockdep.
*/
- if (chain->chain_key != chain_key)
+ if (chain->chain_key != chain_key) {
printk(KERN_INFO "chain %lld: key %#llx <> %#llx\n",
(unsigned long long)(chain - lock_chains),
(unsigned long long)chain->chain_key,
(unsigned long long)chain_key);
- return chain->chain_key == chain_key;
-#else
- return true;
+ return false;
+ }
#endif
+ return true;
}
static bool in_any_zapped_class_list(struct lock_class *class)
@@ -872,10 +872,10 @@ static bool in_any_zapped_class_list(str
struct pending_free *pf;
int i;
- for (i = 0, pf = delayed_free.pf; i < ARRAY_SIZE(delayed_free.pf);
- i++, pf++)
+ for (i = 0, pf = delayed_free.pf; i < ARRAY_SIZE(delayed_free.pf); i++, pf++) {
if (in_list(&class->lock_entry, &pf->zapped))
return true;
+ }
return false;
}
@@ -897,7 +897,6 @@ static bool check_data_structures(void)
printk(KERN_INFO "class %px/%s is not in any class list\n",
class, class->name ? : "(?)");
return false;
- return false;
}
}
@@ -954,6 +953,12 @@ static bool check_data_structures(void)
return true;
}
+#else /* CONFIG_DEBUG_LOCKDEP */
+
+static inline bool check_data_structures(void) { return true; }
+
+#endif /* CONFIG_DEBUG_LOCKDEP */
+
/*
* Initialize the lock_classes[] array elements, the free_lock_classes list
* and also the delayed_free structure.
@@ -4480,10 +4485,11 @@ static void remove_class_from_lock_chain
if (chain_hlocks[i] != class - lock_classes)
continue;
/* The code below leaks one chain_hlock[] entry. */
- if (--chain->depth > 0)
+ if (--chain->depth > 0) {
memmove(&chain_hlocks[i], &chain_hlocks[i + 1],
(chain->base + chain->depth - i) *
sizeof(chain_hlocks[0]));
+ }
/*
* Each lock class occurs at most once in a lock chain so once
* we found a match we can break out of this loop.
@@ -4637,8 +4643,7 @@ static void __free_zapped_classes(struct
{
struct lock_class *class;
- if (check_data_structure_consistency)
- WARN_ON_ONCE(!check_data_structures());
+ WARN_ON_ONCE(!check_data_structures());
list_for_each_entry(class, &pf->zapped, lock_entry)
reinit_class(class);
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys
2019-02-22 16:26 ` Peter Zijlstra
@ 2019-02-22 17:20 ` Bart Van Assche
2019-02-22 22:13 ` Peter Zijlstra
0 siblings, 1 reply; 59+ messages in thread
From: Bart Van Assche @ 2019-02-22 17:20 UTC (permalink / raw)
To: Peter Zijlstra
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel
On 2/22/19 8:26 AM, Peter Zijlstra wrote:
> I've gone over all and I think it looks ok now; I'll give it another
> round tomorrow^Wmonday and then queue bits.
>
> So far the only changes I've made are the below. I'm not entirely sure
> on the unconditional validity check on DEBUG_LOCKDEP, maybe I'll add a
> boot param for that.
Hi Peter,
Please keep in mind that calling check_data_structures() slows down the
kernel a lot and also that it makes systems much less responsive. This
is because the consistency checks disable interrupts for a long time, up
to several seconds. That is why I think it should be possible to enable
and disable the consistency checks independent of other kernel config
options.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v7 00/23] locking/lockdep: Add support for dynamic keys
2019-02-22 17:20 ` Bart Van Assche
@ 2019-02-22 22:13 ` Peter Zijlstra
0 siblings, 0 replies; 59+ messages in thread
From: Peter Zijlstra @ 2019-02-22 22:13 UTC (permalink / raw)
To: Bart Van Assche
Cc: mingo, will.deacon, tj, longman, johannes.berg, linux-kernel
On Fri, Feb 22, 2019 at 09:20:34AM -0800, Bart Van Assche wrote:
> On 2/22/19 8:26 AM, Peter Zijlstra wrote:
> > I've gone over all and I think it looks ok now; I'll give it another
> > round tomorrow^Wmonday and then queue bits.
> >
> > So far the only changes I've made are the below. I'm not entirely sure
> > on the unconditional validity check on DEBUG_LOCKDEP, maybe I'll add a
> > boot param for that.
>
> Hi Peter,
>
> Please keep in mind that calling check_data_structures() slows down the
> kernel a lot and also that it makes systems much less responsive. This is
> because the consistency checks disable interrupts for a long time, up to
> several seconds. That is why I think it should be possible to enable and
> disable the consistency checks independent of other kernel config options.
Ah, yes. So I had expected expensive, but hadn't tried it yet.
I'll make it a boot option.
^ permalink raw reply [flat|nested] 59+ messages in thread