Skip to contentSkip to conversation

locking/lockdep: Track number of zapped classes & report abuse

PATCH In review Waiman Long 2 participants 6 messages Nov 29, 2018 – Dec 5, 2018 Locking primitives

lore
Share & tools
lore

Waiman Long

author reviewer
#0 lore raw

cover locking/lockdep: Track number of zapped classes & report abuse

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(-)
Signature · 1 line
1.8.3.1
author reviewer
#1 lore raw

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

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.

---
 kernel/locking/lockdep.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
kernel/locking/lockdep.c +8 -8
index 1efada2..9e0d36b 100644
@@ -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
Signature · 1 line
1.8.3.1
author reviewer
#2 lore raw

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

In reply to 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.

---
 kernel/locking/lockdep.c           | 26 ++++++++++++++++++++++++++
 kernel/locking/lockdep_internals.h |  1 +
 kernel/locking/lockdep_proc.c      |  2 ++
 3 files changed, 29 insertions(+)
kernel/locking/lockdep.c +26 -0
index 9e0d36b..7136da4 100644
@@ -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)
kernel/locking/lockdep_internals.h +1 -0
index 88c847a..e719d3d 100644
@@ -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;
kernel/locking/lockdep_proc.c +2 -0
index 3d31f9b..04823e6 100644
@@ -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",
Signature · 1 line
1.8.3.1
maintainer
#3 lore raw
In reply to Waiman Long
Quoted 8 lines · 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,

Quoted 11 lines
[ 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/[email protected]

author reviewer
#4 lore raw
Quoted 20 lines · Peter Zijlstra
Quoted 8 lines · 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,

Quoted 11 lines
[ 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] --------------------------------------------------------
Quoted 3 lines

Have a look here:

https://lkml.kernel.org/r/[email protected]

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

Cheers,
Longman

author reviewer
#5 lore raw
Quoted 21 lines · Waiman Long
Quoted 20 lines · Peter Zijlstra
Quoted 19 lines · 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] --------------------------------------------------------
Quoted 8 lines
Quoted 3 lines

Have a look here:

https://lkml.kernel.org/r/[email protected]

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