commit eb1cfd09f788e ("lockdep: Add lock_set_cmp_fn() annotation") has been added to address the issue that lockdep was not able to detect a true deadlock like the following:
https://lore.kernel.org/lkml/20220510232633.GA18445@X58A-UD3R/
The approach is only for lockdep but dept should work being aware of it because the new annotation is already used to avoid false positive of lockdep in the code.
Make dept aware of the new lockdep annotation.
Signed-off-by: Byungchul Park byungchul@sk.com --- include/linux/dept.h | 10 +++++++++ kernel/dependency/dept.c | 48 +++++++++++++++++++++++++++++++++++----- kernel/locking/lockdep.c | 1 + 3 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/include/linux/dept.h b/include/linux/dept.h index 7d5e01f56e8e..bbcb48f6140a 100644 --- a/include/linux/dept.h +++ b/include/linux/dept.h @@ -53,6 +53,11 @@ struct dept_map { const char *name; struct dept_key *keys;
+ /* + * keep lockdep map to handle lockdep_set_lock_cmp_fn(). + */ + void *lockdep_map; + /* * subclass that can be set from user */ @@ -79,6 +84,7 @@ struct dept_map { { \ .name = #n, \ .keys = (struct dept_key *)(k), \ + .lockdep_map = NULL, \ .sub_u = 0, \ .map_key = { .classes = { NULL, } }, \ .wgen = 0U, \ @@ -179,6 +185,8 @@ extern void dept_softirqs_on_ip(unsigned long ip); extern void dept_hardirqs_on(void); extern void dept_softirqs_off(void); extern void dept_hardirqs_off(void); + +#define dept_set_lockdep_map(m, lockdep_m) ({ (m)->lockdep_map = lockdep_m; }) #else /* !CONFIG_DEPT */ struct dept_key { }; struct dept_map { }; @@ -221,5 +229,7 @@ struct dept_ext_wgen { }; #define dept_hardirqs_on() do { } while (0) #define dept_softirqs_off() do { } while (0) #define dept_hardirqs_off() do { } while (0) + +#define dept_set_lockdep_map(m, lockdep_m) do { } while (0) #endif #endif /* __LINUX_DEPT_H */ diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c index 1c6eb0a6d0a6..a0eb4b108de0 100644 --- a/kernel/dependency/dept.c +++ b/kernel/dependency/dept.c @@ -1615,9 +1615,39 @@ static int next_wgen(void) return atomic_inc_return(&wgen) ?: atomic_inc_return(&wgen); }
-static void add_wait(struct dept_class *c, unsigned long ip, - const char *w_fn, int sub_l, bool sched_sleep, - bool timeout) +/* + * XXX: This is a temporary patch needed until lockdep stops tracking + * dependency in wrong way. lockdep has added an annotation to specify + * a callback to determin whether the given lock aquisition order is + * okay or not in its own way. Even though dept is already working + * correctly with sub class on that issue, it needs to be aware of the + * annotation anyway. + */ +static bool lockdep_cmp_fn(struct dept_map *prev, struct dept_map *next) +{ + /* + * Assumes the cmp_fn thing comes from struct lockdep_map. + */ + struct lockdep_map *p_lock = (struct lockdep_map *)prev->lockdep_map; + struct lockdep_map *n_lock = (struct lockdep_map *)next->lockdep_map; + struct lock_class *p_class = p_lock ? p_lock->class_cache[0] : NULL; + struct lock_class *n_class = n_lock ? n_lock->class_cache[0] : NULL; + + if (!p_class || !n_class) + return false; + + if (p_class != n_class) + return false; + + if (!p_class->cmp_fn) + return false; + + return p_class->cmp_fn(p_lock, n_lock) < 0; +} + +static void add_wait(struct dept_map *m, struct dept_class *c, + unsigned long ip, const char *w_fn, int sub_l, + bool sched_sleep, bool timeout) { struct dept_task *dt = dept_task(); struct dept_wait *w; @@ -1658,8 +1688,13 @@ static void add_wait(struct dept_class *c, unsigned long ip, if (!eh->ecxt) continue;
- if (eh->ecxt->class != c || eh->sub_l == sub_l) - add_dep(eh->ecxt, w); + if (eh->ecxt->class == c && eh->sub_l != sub_l) + continue; + + if (i == dt->ecxt_held_pos - 1 && lockdep_cmp_fn(eh->map, m)) + continue; + + add_dep(eh->ecxt, w); }
wg = next_wgen(); @@ -2145,6 +2180,7 @@ void dept_map_init(struct dept_map *m, struct dept_key *k, int sub_u, m->name = n; m->wgen = 0U; m->nocheck = !valid_key(k); + m->lockdep_map = NULL;
dept_exit_recursive(flags); } @@ -2366,7 +2402,7 @@ static void __dept_wait(struct dept_map *m, unsigned long w_f, if (!c) continue;
- add_wait(c, ip, w_fn, sub_l, sched_sleep, timeout); + add_wait(m, c, ip, w_fn, sub_l, sched_sleep, timeout); } }
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 39b9e3e27c0b..c99f91f7a54d 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -5035,6 +5035,7 @@ void lockdep_set_lock_cmp_fn(struct lockdep_map *lock, lock_cmp_fn cmp_fn, class->print_fn = print_fn; }
+ dept_set_lockdep_map(&lock->dmap, lock); lockdep_recursion_finish(); raw_local_irq_restore(flags); }