[PATCH v15 34/43] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation

Byungchul Park posted 43 patches 9 months ago
There is a newer version of this series
[PATCH v15 34/43] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
Posted by Byungchul Park 9 months ago
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 b1e5a1ec6763..19d72b0b0c4b 100644
--- a/include/linux/dept.h
+++ b/include/linux/dept.h
@@ -135,6 +135,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
 	 */
@@ -161,6 +166,7 @@ struct dept_map {
 {									\
 	.name = #n,							\
 	.keys = (struct dept_key *)(k),					\
+	.lockdep_map = NULL,						\
 	.sub_u = 0,							\
 	.map_key = { .classes = { NULL, } },				\
 	.wgen = 0U,							\
@@ -432,6 +438,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 { };
@@ -474,5 +482,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 8bea64bab379..5397167c7031 100644
--- a/kernel/dependency/dept.c
+++ b/kernel/dependency/dept.c
@@ -1624,9 +1624,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;
@@ -1667,8 +1697,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();
@@ -2154,6 +2189,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);
 }
@@ -2377,7 +2413,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 d2805ce250cb..acab023eb015 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5036,6 +5036,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);
 }
-- 
2.17.1
Re: [PATCH v15 34/43] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
Posted by kernel test robot 9 months ago
Hi Byungchul,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3]

url:    https://github.com/intel-lab-lkp/linux/commits/Byungchul-Park/llist-move-llist_-head-node-definition-to-types-h/20250513-181346
base:   82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3
patch link:    https://lore.kernel.org/r/20250513100730.12664-35-byungchul%40sk.com
patch subject: [PATCH v15 34/43] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20250515/202505150014.6SbTtVaA-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250515/202505150014.6SbTtVaA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505150014.6SbTtVaA-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/net/ethernet/microchip/vcap/vcap_api.c:3610:
>> drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:1921:13: warning: stack frame size (2104) exceeds limit (2048) in 'vcap_api_next_lookup_advanced_test' [-Wframe-larger-than]
    1921 | static void vcap_api_next_lookup_advanced_test(struct kunit *test)
         |             ^
   1 warning generated.


vim +/vcap_api_next_lookup_advanced_test +1921 drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c

c956b9b318d903 Steen Hegelund 2022-11-09  1920  
c956b9b318d903 Steen Hegelund 2022-11-09 @1921  static void vcap_api_next_lookup_advanced_test(struct kunit *test)
c956b9b318d903 Steen Hegelund 2022-11-09  1922  {
129ff4de58ff0c Arnd Bergmann  2023-02-17  1923  	struct vcap_admin admin[] = {
129ff4de58ff0c Arnd Bergmann  2023-02-17  1924  	{
c956b9b318d903 Steen Hegelund 2022-11-09  1925  		.vtype = VCAP_TYPE_IS0,
c956b9b318d903 Steen Hegelund 2022-11-09  1926  		.vinst = 0,
c956b9b318d903 Steen Hegelund 2022-11-09  1927  		.first_cid = 1000000,
c956b9b318d903 Steen Hegelund 2022-11-09  1928  		.last_cid =  1199999,
c956b9b318d903 Steen Hegelund 2022-11-09  1929  		.lookups = 6,
c956b9b318d903 Steen Hegelund 2022-11-09  1930  		.lookups_per_instance = 2,
129ff4de58ff0c Arnd Bergmann  2023-02-17  1931  	}, {
c956b9b318d903 Steen Hegelund 2022-11-09  1932  		.vtype = VCAP_TYPE_IS0,
c956b9b318d903 Steen Hegelund 2022-11-09  1933  		.vinst = 1,
c956b9b318d903 Steen Hegelund 2022-11-09  1934  		.first_cid = 1200000,
c956b9b318d903 Steen Hegelund 2022-11-09  1935  		.last_cid =  1399999,
c956b9b318d903 Steen Hegelund 2022-11-09  1936  		.lookups = 6,
c956b9b318d903 Steen Hegelund 2022-11-09  1937  		.lookups_per_instance = 2,
129ff4de58ff0c Arnd Bergmann  2023-02-17  1938  	}, {
c956b9b318d903 Steen Hegelund 2022-11-09  1939  		.vtype = VCAP_TYPE_IS0,
c956b9b318d903 Steen Hegelund 2022-11-09  1940  		.vinst = 2,
c956b9b318d903 Steen Hegelund 2022-11-09  1941  		.first_cid = 1400000,
c956b9b318d903 Steen Hegelund 2022-11-09  1942  		.last_cid =  1599999,
c956b9b318d903 Steen Hegelund 2022-11-09  1943  		.lookups = 6,
c956b9b318d903 Steen Hegelund 2022-11-09  1944  		.lookups_per_instance = 2,
129ff4de58ff0c Arnd Bergmann  2023-02-17  1945  	}, {
c956b9b318d903 Steen Hegelund 2022-11-09  1946  		.vtype = VCAP_TYPE_IS2,
c956b9b318d903 Steen Hegelund 2022-11-09  1947  		.vinst = 0,
c956b9b318d903 Steen Hegelund 2022-11-09  1948  		.first_cid = 8000000,
c956b9b318d903 Steen Hegelund 2022-11-09  1949  		.last_cid = 8199999,
c956b9b318d903 Steen Hegelund 2022-11-09  1950  		.lookups = 4,
c956b9b318d903 Steen Hegelund 2022-11-09  1951  		.lookups_per_instance = 2,
129ff4de58ff0c Arnd Bergmann  2023-02-17  1952  	}, {
c956b9b318d903 Steen Hegelund 2022-11-09  1953  		.vtype = VCAP_TYPE_IS2,
c956b9b318d903 Steen Hegelund 2022-11-09  1954  		.vinst = 1,
c956b9b318d903 Steen Hegelund 2022-11-09  1955  		.first_cid = 8200000,
c956b9b318d903 Steen Hegelund 2022-11-09  1956  		.last_cid = 8399999,
c956b9b318d903 Steen Hegelund 2022-11-09  1957  		.lookups = 4,
c956b9b318d903 Steen Hegelund 2022-11-09  1958  		.lookups_per_instance = 2,
129ff4de58ff0c Arnd Bergmann  2023-02-17  1959  	}
c956b9b318d903 Steen Hegelund 2022-11-09  1960  	};
c956b9b318d903 Steen Hegelund 2022-11-09  1961  	bool ret;
c956b9b318d903 Steen Hegelund 2022-11-09  1962  
129ff4de58ff0c Arnd Bergmann  2023-02-17  1963  	vcap_test_api_init(&admin[0]);
129ff4de58ff0c Arnd Bergmann  2023-02-17  1964  	list_add_tail(&admin[1].list, &test_vctrl.list);
129ff4de58ff0c Arnd Bergmann  2023-02-17  1965  	list_add_tail(&admin[2].list, &test_vctrl.list);
129ff4de58ff0c Arnd Bergmann  2023-02-17  1966  	list_add_tail(&admin[3].list, &test_vctrl.list);
129ff4de58ff0c Arnd Bergmann  2023-02-17  1967  	list_add_tail(&admin[4].list, &test_vctrl.list);
c956b9b318d903 Steen Hegelund 2022-11-09  1968  
c956b9b318d903 Steen Hegelund 2022-11-09  1969  	ret = vcap_is_next_lookup(&test_vctrl, 1000000, 1001000);
c956b9b318d903 Steen Hegelund 2022-11-09  1970  	KUNIT_EXPECT_EQ(test, false, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1971  	ret = vcap_is_next_lookup(&test_vctrl, 1000000, 1101000);
c956b9b318d903 Steen Hegelund 2022-11-09  1972  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1973  
c956b9b318d903 Steen Hegelund 2022-11-09  1974  	ret = vcap_is_next_lookup(&test_vctrl, 1100000, 1201000);
c956b9b318d903 Steen Hegelund 2022-11-09  1975  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1976  	ret = vcap_is_next_lookup(&test_vctrl, 1100000, 1301000);
784c3067d094dd Steen Hegelund 2023-01-14  1977  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1978  	ret = vcap_is_next_lookup(&test_vctrl, 1100000, 8101000);
784c3067d094dd Steen Hegelund 2023-01-14  1979  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1980  	ret = vcap_is_next_lookup(&test_vctrl, 1300000, 1401000);
c956b9b318d903 Steen Hegelund 2022-11-09  1981  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1982  	ret = vcap_is_next_lookup(&test_vctrl, 1400000, 1501000);
c956b9b318d903 Steen Hegelund 2022-11-09  1983  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1984  	ret = vcap_is_next_lookup(&test_vctrl, 1500000, 8001000);
c956b9b318d903 Steen Hegelund 2022-11-09  1985  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1986  
c956b9b318d903 Steen Hegelund 2022-11-09  1987  	ret = vcap_is_next_lookup(&test_vctrl, 8000000, 8001000);
c956b9b318d903 Steen Hegelund 2022-11-09  1988  	KUNIT_EXPECT_EQ(test, false, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1989  	ret = vcap_is_next_lookup(&test_vctrl, 8000000, 8101000);
c956b9b318d903 Steen Hegelund 2022-11-09  1990  	KUNIT_EXPECT_EQ(test, true, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1991  
c956b9b318d903 Steen Hegelund 2022-11-09  1992  	ret = vcap_is_next_lookup(&test_vctrl, 8300000, 8301000);
c956b9b318d903 Steen Hegelund 2022-11-09  1993  	KUNIT_EXPECT_EQ(test, false, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1994  	ret = vcap_is_next_lookup(&test_vctrl, 8300000, 8401000);
784c3067d094dd Steen Hegelund 2023-01-14  1995  	KUNIT_EXPECT_EQ(test, false, ret);
c956b9b318d903 Steen Hegelund 2022-11-09  1996  }
c956b9b318d903 Steen Hegelund 2022-11-09  1997  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki