Skip to content

Commit

Permalink
dept: Implement Dept(Dependency Tracker)
Browse files Browse the repository at this point in the history
CURRENT STATUS
--------------
Lockdep tracks acquisition order of locks in order to detect deadlock,
and IRQ and IRQ enable/disable state as well to take accident
acquisitions into account.

Lockdep should be turned off once it detects and reports a deadlock
since the data structure and algorithm are not reusable after detection
because of the complex design.

PROBLEM
-------
*Waits* and their *events* that never reach eventually cause deadlock.
However, Lockdep is only interested in lock acquisition order, forcing
to emulate lock acqusition even for just waits and events that have
nothing to do with real lock.

Even worse, no one likes Lockdep's false positive detection because that
prevents further one that might be more valuable. That's why all the
kernel developers are sensitive to Lockdep's false positive.

Besides those, by tracking acquisition order, it cannot correctly deal
with read lock and cross-event e.g. wait_for_completion()/complete() for
deadlock detection. Lockdep is no longer a good tool for that purpose.

SOLUTION
--------
Again, *waits* and their *events* that never reach eventually cause
deadlock. The new solution, Dept(DEPendency Tracker), focuses on waits
and events themselves. Dept tracks waits and events and report it if
any event would be never reachable.

Dept does:
   . Works with read lock in the right way.
   . Works with any wait and event e.i. cross-event.
   . Continue to work even after reporting multiple times.
   . Provides simple and intuitive APIs.
   . Does exactly what dependency checker should do.

Q & A
-----
Q. Is this the first try ever to address the problem?
A. No. Cross-release feature (b09be67 locking/lockdep: Implement
   the 'crossrelease' feature) addressed it 2 years ago that was a
   Lockdep extension and merged but reverted shortly because:

   Cross-release started to report valuable hidden problems but started
   to give report false positive reports as well. For sure, no one
   likes Lockdep's false positive reports since it makes Lockdep stop,
   preventing reporting further real problems.

Q. Why not Dept was developed as an extension of Lockdep?
A. Lockdep definitely includes all the efforts great developers have
   made for a long time so as to be quite stable enough. But I had to
   design and implement newly because of the following:

   1) Lockdep was designed to track lock acquisition order. The APIs and
      implementation do not fit on wait-event model.
   2) Lockdep is turned off on detection including false positive. Which
      is terrible and prevents developing any extension for stronger
      detection.

Q. Do you intend to totally replace Lockdep?
A. No. Lockdep also checks if lock usage is correct. Of course, the
   dependency check routine should be replaced but the other functions
   should be still there.

Q. Do you mean the dependency check routine should be replaced right
   away?
A. No. I admit Lockdep is stable enough thanks to great efforts kernel
   developers have made. Lockdep and Dept, both should be in the kernel
   until Dept gets considered stable.

Q. Stronger detection capability would give more false positive report.
   Which was a big problem when cross-release was introduced. Is it ok
   with Dept?
A. It's ok. Dept allows multiple reporting thanks to simple and quite
   generalized design. Of course, false positive reports should be fixed
   anyway but it's no longer as a critical problem as it was.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
  • Loading branch information
lgebyungchulpark authored and intel-lab-lkp committed Jan 9, 2023
1 parent a82b5dd commit fe11f4e
Show file tree
Hide file tree
Showing 16 changed files with 3,633 additions and 0 deletions.
573 changes: 573 additions & 0 deletions include/linux/dept.h

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions include/linux/hardirq.h
Expand Up @@ -5,6 +5,7 @@
#include <linux/context_tracking_state.h>
#include <linux/preempt.h>
#include <linux/lockdep.h>
#include <linux/dept.h>
#include <linux/ftrace_irq.h>
#include <linux/sched.h>
#include <linux/vtime.h>
Expand Down Expand Up @@ -106,6 +107,7 @@ void irq_exit_rcu(void);
*/
#define __nmi_enter() \
do { \
dept_off(); \
lockdep_off(); \
arch_nmi_enter(); \
BUG_ON(in_nmi() == NMI_MASK); \
Expand All @@ -128,6 +130,7 @@ void irq_exit_rcu(void);
__preempt_count_sub(NMI_OFFSET + HARDIRQ_OFFSET); \
arch_nmi_exit(); \
lockdep_on(); \
dept_on(); \
} while (0)

#define nmi_exit() \
Expand Down
3 changes: 3 additions & 0 deletions include/linux/sched.h
Expand Up @@ -37,6 +37,7 @@
#include <linux/kcsan.h>
#include <linux/rv.h>
#include <asm/kmap_size.h>
#include <linux/dept.h>

/* task_struct member predeclarations (sorted alphabetically): */
struct audit_context;
Expand Down Expand Up @@ -1168,6 +1169,8 @@ struct task_struct {
struct held_lock held_locks[MAX_LOCK_DEPTH];
#endif

struct dept_task dept_task;

#if defined(CONFIG_UBSAN) && !defined(CONFIG_UBSAN_TRAP)
unsigned int in_ubsan;
#endif
Expand Down
2 changes: 2 additions & 0 deletions init/init_task.c
Expand Up @@ -12,6 +12,7 @@
#include <linux/audit.h>
#include <linux/numa.h>
#include <linux/scs.h>
#include <linux/dept.h>

#include <linux/uaccess.h>

Expand Down Expand Up @@ -194,6 +195,7 @@ struct task_struct init_task
.curr_chain_key = INITIAL_CHAIN_KEY,
.lockdep_recursion = 0,
#endif
.dept_task = DEPT_TASK_INITIALIZER(init_task),
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.ret_stack = NULL,
.tracing_graph_pause = ATOMIC_INIT(0),
Expand Down
2 changes: 2 additions & 0 deletions init/main.c
Expand Up @@ -66,6 +66,7 @@
#include <linux/debug_locks.h>
#include <linux/debugobjects.h>
#include <linux/lockdep.h>
#include <linux/dept.h>
#include <linux/kmemleak.h>
#include <linux/padata.h>
#include <linux/pid_namespace.h>
Expand Down Expand Up @@ -1080,6 +1081,7 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void)
panic_param);

lockdep_init();
dept_init();

/*
* Need to run this when irqs are enabled, because it wants
Expand Down
1 change: 1 addition & 0 deletions kernel/Makefile
Expand Up @@ -51,6 +51,7 @@ obj-y += livepatch/
obj-y += dma/
obj-y += entry/
obj-$(CONFIG_MODULES) += module/
obj-y += dependency/

obj-$(CONFIG_KCMP) += kcmp.o
obj-$(CONFIG_FREEZER) += freezer.o
Expand Down
3 changes: 3 additions & 0 deletions kernel/dependency/Makefile
@@ -0,0 +1,3 @@
# SPDX-License-Identifier: GPL-2.0

obj-$(CONFIG_DEPT) += dept.o

0 comments on commit fe11f4e

Please sign in to comment.