[PATCH 0/2] locking/lockdep: Track number of zapped classes & report abuse

[PATCH 0/2] locking/lockdep: Track number of zapped classes & report abuse

[Waiman Long]

When a kernel module is repeatedly load and unload, it will eventually
exhaust the lockdep entries resulting in a bug message. This is a use
case that the current lockdep code cannot support.

This patchset tracks the number of zapped classes and print a warning if
too many lockdep entries are wasted because of too many module unloading.
For example,

[ 2490.651531] BUG: MAX_LOCKDEP_KEYS too low!
[ 2490.669925] turning off the locking correctness validator.
[ 2490.669925] Please attach the output of /proc/lock_stat to the bug report
[ 2490.669926] ========================================================
[ 2490.669927] WARNING: 6499 out of 8191 locks have been destroyed
[ 2490.669927] through kernel module unload operations.
[ 2490.669928] The corresponding lockdep entries are not reusable.
[ 2490.669928] The system might have run out of lockdep entries because
[ 2490.669929] of repeated kernel module load and unload operations.
[ 2490.669929] Lockdep cannot support this particular use case.
[ 2490.669930] --------------------------------------------------------

Waiman Long (2):
  locking/lockdep: Annotate #else/#endif's that are far from #if
  locking/lockdep: Track number of zapped classes & report abuse

 kernel/locking/lockdep.c           | 42 ++++++++++++++++++++++++++++++--------
 kernel/locking/lockdep_internals.h |  1 +
 kernel/locking/lockdep_proc.c      |  2 ++
 3 files changed, 37 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] locking/lockdep: Annotate #else/#endif's that are far from #if

[Waiman Long]

There are some #else and #endif preprocessor directives that are hard
to see where the corresponding #if*'s are when they are more than a
page away. To make the code easier to read, those #else and #endif's
are now properly annotated.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/locking/lockdep.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 1efada2..9e0d36b 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -267,11 +267,11 @@ static void lock_release_holdtime(struct held_lock *hlock)
 	else
 		lock_time_inc(&stats->write_holdtime, holdtime);
 }
-#else
+#else /* CONFIG_LOCK_STAT */
 static inline void lock_release_holdtime(struct held_lock *hlock)
 {
 }
-#endif
+#endif /* CONFIG_LOCK_STAT */
 
 /*
  * We keep a global list of all lock classes. The list only grows,
@@ -1699,7 +1699,7 @@ static void inc_chains(void)
 	}
 }
 
-#else
+#else /* defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_PROVE_LOCKING) */
 
 static inline int
 check_prev_add_irq(struct task_struct *curr, struct held_lock *prev,
@@ -1713,7 +1713,7 @@ static inline void inc_chains(void)
 	nr_process_chains++;
 }
 
-#endif
+#endif /* defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_PROVE_LOCKING) */
 
 static void
 print_deadlock_scenario(struct held_lock *nxt,
@@ -2115,7 +2115,7 @@ static void print_collision(struct task_struct *curr,
 	pr_warn("\nstack backtrace:\n");
 	dump_stack();
 }
-#endif
+#endif /* CONFIG_DEBUG_LOCKDEP */
 
 /*
  * Checks whether the chain and the current held locks are consistent
@@ -2357,14 +2357,14 @@ static int validate_chain(struct task_struct *curr, struct lockdep_map *lock,
 
 	return 1;
 }
-#else
+#else /* CONFIG_PROVE_LOCKING */
 static inline int validate_chain(struct task_struct *curr,
 	       	struct lockdep_map *lock, struct held_lock *hlock,
 		int chain_head, u64 chain_key)
 {
 	return 1;
 }
-#endif
+#endif /* CONFIG_PROVE_LOCKING */
 
 /*
  * We are building curr_chain_key incrementally, so double-check
@@ -4100,7 +4100,7 @@ void lock_acquired(struct lockdep_map *lock, unsigned long ip)
 	raw_local_irq_restore(flags);
 }
 EXPORT_SYMBOL_GPL(lock_acquired);
-#endif
+#endif /* CONFIG_LOCK_STAT */
 
 /*
  * Used by the testsuite, sanitize the validator state
-- 
1.8.3.1

[PATCH 2/2] locking/lockdep: Track number of zapped classes & report abuse

[Waiman Long]

When a kernel module is unloaded, all the lock classes associated with
that kernel module will be zapped. Unfortunately the corresponding
lockdep entries in stack_trace[], list_entries[], lock_classes[],
lock_chains[] and chain_hlocks[] are not reusable without greatly
complicating the existing code.

As a result, an application that does repeated kernel module load and
unload operations may exhaust all the entries in one of the lockdep
arrays leading to a bug message. Lockdep cannot currently support this
particular use case and we need to let users aware of that.

The number of zapped lock classes will now be tracked. If one of the
lockdep arrays is out of entries and the number of zapped classes is
a significant portion (1/4) of the total, a warning message will be
printed to notify users that we cannot supported repeated kernel module
load and unload operations.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/locking/lockdep.c           | 26 ++++++++++++++++++++++++++
 kernel/locking/lockdep_internals.h |  1 +
 kernel/locking/lockdep_proc.c      |  2 ++
 3 files changed, 29 insertions(+)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 9e0d36b..7136da4 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -138,6 +138,7 @@ static inline int debug_locks_off_graph_unlock(void)
  * get freed - this significantly simplifies the debugging code.
  */
 unsigned long nr_lock_classes;
+unsigned long nr_zapped_classes;
 struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
 
 static inline struct lock_class *hlock_class(struct held_lock *hlock)
@@ -372,6 +373,25 @@ static int verbose(struct lock_class *class)
 }
 
 /*
+ * Check the number of zapped classes and print a warning if it is
+ * more than 1/4 of the total.
+ */
+static void check_zapped_classes(void)
+{
+	if (nr_zapped_classes < nr_lock_classes/4)
+		return;
+	pr_warn("========================================================\n");
+	pr_warn("WARNING: %ld out of %ld locks have been destroyed\n",
+		nr_zapped_classes, nr_lock_classes);
+	pr_warn("through kernel module unload operations.\n");
+	pr_warn("The corresponding lockdep entries are not reusable.\n");
+	pr_warn("The system might have run out of lockdep entries because\n");
+	pr_warn("of repeated kernel module load and unload operations.\n");
+	pr_warn("Lockdep cannot support this particular use case.\n");
+	pr_warn("--------------------------------------------------------\n");
+}
+
+/*
  * Stack-trace: tightly packed array of stack backtrace
  * addresses. Protected by the graph_lock.
  */
@@ -417,6 +437,7 @@ static int save_trace(struct stack_trace *trace)
 			return 0;
 
 		print_lockdep_off("BUG: MAX_STACK_TRACE_ENTRIES too low!");
+		check_zapped_classes();
 		dump_stack();
 
 		return 0;
@@ -781,6 +802,7 @@ static bool assign_lock_key(struct lockdep_map *lock)
 		}
 
 		print_lockdep_off("BUG: MAX_LOCKDEP_KEYS too low!");
+		check_zapped_classes();
 		dump_stack();
 		return NULL;
 	}
@@ -847,6 +869,7 @@ static struct lock_list *alloc_list_entry(void)
 			return NULL;
 
 		print_lockdep_off("BUG: MAX_LOCKDEP_ENTRIES too low!");
+		check_zapped_classes();
 		dump_stack();
 		return NULL;
 	}
@@ -2183,6 +2206,7 @@ static inline int add_chain_cache(struct task_struct *curr,
 			return 0;
 
 		print_lockdep_off("BUG: MAX_LOCKDEP_CHAINS too low!");
+		check_zapped_classes();
 		dump_stack();
 		return 0;
 	}
@@ -2217,6 +2241,7 @@ static inline int add_chain_cache(struct task_struct *curr,
 			return 0;
 
 		print_lockdep_off("BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!");
+		check_zapped_classes();
 		dump_stack();
 		return 0;
 	}
@@ -4146,6 +4171,7 @@ static void zap_class(struct lock_class *class)
 
 	RCU_INIT_POINTER(class->key, NULL);
 	RCU_INIT_POINTER(class->name, NULL);
+	nr_zapped_classes++;
 }
 
 static inline int within(const void *addr, void *start, unsigned long size)
diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 88c847a..e719d3d 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -95,6 +95,7 @@ extern void get_usage_chars(struct lock_class *class,
 struct lock_class *lock_chain_get_class(struct lock_chain *chain, int i);
 
 extern unsigned long nr_lock_classes;
+extern unsigned long nr_zapped_classes;
 extern unsigned long nr_list_entries;
 extern unsigned long nr_lock_chains;
 extern int nr_chain_hlocks;
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index 3d31f9b..04823e6 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -314,6 +314,8 @@ static int lockdep_stats_show(struct seq_file *m, void *v)
 	seq_printf(m, " irq-read-unsafe locks:         %11lu\n",
 			nr_irq_read_unsafe);
 
+	seq_printf(m, " zapped locks:                  %11lu\n",
+			nr_zapped_classes);
 	seq_printf(m, " uncategorized locks:           %11lu\n",
 			nr_uncategorized);
 	seq_printf(m, " unused locks:                  %11lu\n",
-- 
1.8.3.1

Re: [PATCH 0/2] locking/lockdep: Track number of zapped classes & report abuse

[Peter Zijlstra]

On Thu, Nov 29, 2018 at 05:41:35PM -0500, Waiman Long wrote:
> When a kernel module is repeatedly load and unload, it will eventually
> exhaust the lockdep entries resulting in a bug message. This is a use
> case that the current lockdep code cannot support.
> 
> This patchset tracks the number of zapped classes and print a warning if
> too many lockdep entries are wasted because of too many module unloading.
> For example,
> 
> [ 2490.651531] BUG: MAX_LOCKDEP_KEYS too low!
> [ 2490.669925] turning off the locking correctness validator.
> [ 2490.669925] Please attach the output of /proc/lock_stat to the bug report
> [ 2490.669926] ========================================================
> [ 2490.669927] WARNING: 6499 out of 8191 locks have been destroyed
> [ 2490.669927] through kernel module unload operations.
> [ 2490.669928] The corresponding lockdep entries are not reusable.
> [ 2490.669928] The system might have run out of lockdep entries because
> [ 2490.669929] of repeated kernel module load and unload operations.
> [ 2490.669929] Lockdep cannot support this particular use case.
> [ 2490.669930] --------------------------------------------------------

Have a look here:

  https://lkml.kernel.org/r/20181128234325.110011-1-bvanassche@acm.org

Re: [PATCH 0/2] locking/lockdep: Track number of zapped classes & report abuse

[Waiman Long]

On 11/29/2018 05:48 PM, Peter Zijlstra wrote:
> On Thu, Nov 29, 2018 at 05:41:35PM -0500, Waiman Long wrote:
>> When a kernel module is repeatedly load and unload, it will eventually
>> exhaust the lockdep entries resulting in a bug message. This is a use
>> case that the current lockdep code cannot support.
>>
>> This patchset tracks the number of zapped classes and print a warning if
>> too many lockdep entries are wasted because of too many module unloading.
>> For example,
>>
>> [ 2490.651531] BUG: MAX_LOCKDEP_KEYS too low!
>> [ 2490.669925] turning off the locking correctness validator.
>> [ 2490.669925] Please attach the output of /proc/lock_stat to the bug report
>> [ 2490.669926] ========================================================
>> [ 2490.669927] WARNING: 6499 out of 8191 locks have been destroyed
>> [ 2490.669927] through kernel module unload operations.
>> [ 2490.669928] The corresponding lockdep entries are not reusable.
>> [ 2490.669928] The system might have run out of lockdep entries because
>> [ 2490.669929] of repeated kernel module load and unload operations.
>> [ 2490.669929] Lockdep cannot support this particular use case.
>> [ 2490.669930] --------------------------------------------------------
> Have a look here:
>
>   https://lkml.kernel.org/r/20181128234325.110011-1-bvanassche@acm.org

Thanks for the pointer, I will take a look at that.

Cheers,
Longman

Re: [PATCH 0/2] locking/lockdep: Track number of zapped classes & report abuse

[Waiman Long]

On 11/30/2018 09:38 AM, Waiman Long wrote:
> On 11/29/2018 05:48 PM, Peter Zijlstra wrote:
>> On Thu, Nov 29, 2018 at 05:41:35PM -0500, Waiman Long wrote:
>>> When a kernel module is repeatedly load and unload, it will eventually
>>> exhaust the lockdep entries resulting in a bug message. This is a use
>>> case that the current lockdep code cannot support.
>>>
>>> This patchset tracks the number of zapped classes and print a warning if
>>> too many lockdep entries are wasted because of too many module unloading.
>>> For example,
>>>
>>> [ 2490.651531] BUG: MAX_LOCKDEP_KEYS too low!
>>> [ 2490.669925] turning off the locking correctness validator.
>>> [ 2490.669925] Please attach the output of /proc/lock_stat to the bug report
>>> [ 2490.669926] ========================================================
>>> [ 2490.669927] WARNING: 6499 out of 8191 locks have been destroyed
>>> [ 2490.669927] through kernel module unload operations.
>>> [ 2490.669928] The corresponding lockdep entries are not reusable.
>>> [ 2490.669928] The system might have run out of lockdep entries because
>>> [ 2490.669929] of repeated kernel module load and unload operations.
>>> [ 2490.669929] Lockdep cannot support this particular use case.
>>> [ 2490.669930] --------------------------------------------------------
>> Have a look here:
>>
>>   https://lkml.kernel.org/r/20181128234325.110011-1-bvanassche@acm.org
> Thanks for the pointer, I will take a look at that.
>
> Cheers,
> Longman
>
I have finished reviewing Bart's v2 patch. It enables the reuse of
lock_classes[] and list_entries[] entries. However, the stack_trace[],
lock_chains[] and chain_hlocks[] entries will still be exhausted over
time. So it doesn't completely solve the issue that I am looking at.

As a side note, an alternative way of solving the workqueue lockdep
problem may be to mark the lockdep key as special that it will hash the
actual lock address as part of the key so that each unique lock of the
same key will have its own unique lock class. That may be able to fix
the issue as well. Just a thought.

Cheers,
Longman