[PATCH v10 05/25] dept: Tie to Lockdep and IRQ tracing

From: Byungchul Park
Date: Mon Jun 26 2023 - 08:15:19 EST

Next message: Matthew Wilcox: "Re: f2fs async buffered write patch"
Previous message: Byungchul Park: "[PATCH v10 07/25] dept: Apply sdt_might_sleep_{start,end}() to wait_for_completion()/complete()"
In reply to: Byungchul Park: "[PATCH v10 07/25] dept: Apply sdt_might_sleep_{start,end}() to wait_for_completion()/complete()"
Next in thread: Byungchul Park: "[PATCH v10 16/25] dept: Apply sdt_might_sleep_{start,end}() to dma fence wait"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Yes. How to place Dept in here looks so ugly. But it's inevitable as
long as relying on Lockdep. The way should be enhanced gradually.

1. Basically relies on Lockdep to track typical locks and IRQ things.

2. Dept fails to recognize IRQ situation so it generates false alarms
when raw_local_irq_*() APIs are used. So made it track those too.

3. Lockdep doesn't track the outmost {hard,soft}irq entracnes but
Dept makes use of it. So made it track those too.

Signed-off-by: Byungchul Park <byungchul@xxxxxx>
---
include/linux/irqflags.h | 22 +++++-
include/linux/local_lock_internal.h | 1 +
include/linux/lockdep.h | 102 ++++++++++++++++++++++------
include/linux/lockdep_types.h | 3 +
include/linux/mutex.h | 1 +
include/linux/percpu-rwsem.h | 2 +-
include/linux/rtmutex.h | 1 +
include/linux/rwlock_types.h | 1 +
include/linux/rwsem.h | 1 +
include/linux/seqlock.h | 2 +-
include/linux/spinlock_types_raw.h | 3 +
include/linux/srcu.h | 2 +-
kernel/dependency/dept.c | 4 +-
kernel/locking/lockdep.c | 23 +++++++
14 files changed, 139 insertions(+), 29 deletions(-)

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 5ec0fa71399e..0ebc5ec2dbd4 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -13,6 +13,7 @@
#define _LINUX_TRACE_IRQFLAGS_H

#include <linux/typecheck.h>
+#include <linux/dept.h>
#include <asm/irqflags.h>
#include <asm/percpu.h>

@@ -60,8 +61,10 @@ extern void trace_hardirqs_off(void);
# define lockdep_softirqs_enabled(p) ((p)->softirqs_enabled)
# define lockdep_hardirq_enter() \
do { \
- if (__this_cpu_inc_return(hardirq_context) == 1)\
+ if (__this_cpu_inc_return(hardirq_context) == 1) { \
current->hardirq_threaded = 0; \
+ dept_hardirq_enter(); \
+ } \
} while (0)
# define lockdep_hardirq_threaded() \
do { \
@@ -136,6 +139,8 @@ do { \
# define lockdep_softirq_enter() \
do { \
current->softirq_context++; \
+ if (current->softirq_context == 1) \
+ dept_softirq_enter(); \
} while (0)
# define lockdep_softirq_exit() \
do { \
@@ -170,17 +175,28 @@ extern void warn_bogus_irq_restore(void);
/*
* Wrap the arch provided IRQ routines to provide appropriate checks.
*/
-#define raw_local_irq_disable() arch_local_irq_disable()
-#define raw_local_irq_enable() arch_local_irq_enable()
+#define raw_local_irq_disable() \
+ do { \
+ arch_local_irq_disable(); \
+ dept_hardirqs_off(); \
+ } while (0)
+#define raw_local_irq_enable() \
+ do { \
+ dept_hardirqs_on(); \
+ arch_local_irq_enable(); \
+ } while (0)
#define raw_local_irq_save(flags) \
do { \
typecheck(unsigned long, flags); \
flags = arch_local_irq_save(); \
+ dept_hardirqs_off(); \
} while (0)
#define raw_local_irq_restore(flags) \
do { \
typecheck(unsigned long, flags); \
raw_check_bogus_irq_restore(); \
+ if (!arch_irqs_disabled_flags(flags)) \
+ dept_hardirqs_on(); \
arch_local_irq_restore(flags); \
} while (0)
#define raw_local_save_flags(flags) \
diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
index 975e33b793a7..39f67788fd95 100644
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -21,6 +21,7 @@ typedef struct {
.name = #lockname, \
.wait_type_inner = LD_WAIT_CONFIG, \
.lock_type = LD_LOCK_PERCPU, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL),\
}, \
.owner = NULL,

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 1f1099dac3f0..99961026ba43 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -12,6 +12,7 @@

#include <linux/lockdep_types.h>
#include <linux/smp.h>
+#include <linux/dept_ldt.h>
#include <asm/percpu.h>

struct task_struct;
@@ -39,6 +40,8 @@ static inline void lockdep_copy_map(struct lockdep_map *to,
*/
for (i = 0; i < NR_LOCKDEP_CACHING_CLASSES; i++)
to->class_cache[i] = NULL;
+
+ dept_map_copy(&to->dmap, &from->dmap);
}

/*
@@ -441,7 +444,8 @@ enum xhlock_context_t {
* Note that _name must not be NULL.
*/
#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
- { .name = (_name), .key = (void *)(_key), }
+ { .name = (_name), .key = (void *)(_key), \
+ .dmap = DEPT_MAP_INITIALIZER(_name, _key) }

static inline void lockdep_invariant_state(bool force) {}
static inline void lockdep_free_task(struct task_struct *task) {}
@@ -523,33 +527,89 @@ extern bool read_lock_is_recursive(void);
#define lock_acquire_shared(l, s, t, n, i) lock_acquire(l, s, t, 1, 1, n, i)
#define lock_acquire_shared_recursive(l, s, t, n, i) lock_acquire(l, s, t, 2, 1, n, i)

-#define spin_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
-#define spin_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i)
-#define spin_release(l, i) lock_release(l, i)
-
-#define rwlock_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
+#define spin_acquire(l, s, t, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, NULL, i); \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+} while (0)
+#define spin_acquire_nest(l, s, t, n, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, n, i); \
+ lock_acquire_exclusive(l, s, t, n, i); \
+} while (0)
+#define spin_release(l, i) \
+do { \
+ ldt_unlock(&(l)->dmap, i); \
+ lock_release(l, i); \
+} while (0)
+#define rwlock_acquire(l, s, t, i) \
+do { \
+ ldt_wlock(&(l)->dmap, s, t, NULL, i); \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+} while (0)
#define rwlock_acquire_read(l, s, t, i) \
do { \
+ ldt_rlock(&(l)->dmap, s, t, NULL, i, !read_lock_is_recursive());\
if (read_lock_is_recursive()) \
lock_acquire_shared_recursive(l, s, t, NULL, i); \
else \
lock_acquire_shared(l, s, t, NULL, i); \
} while (0)
-
-#define rwlock_release(l, i) lock_release(l, i)
-
-#define seqcount_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
-#define seqcount_acquire_read(l, s, t, i) lock_acquire_shared_recursive(l, s, t, NULL, i)
-#define seqcount_release(l, i) lock_release(l, i)
-
-#define mutex_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
-#define mutex_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i)
-#define mutex_release(l, i) lock_release(l, i)
-
-#define rwsem_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
-#define rwsem_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i)
-#define rwsem_acquire_read(l, s, t, i) lock_acquire_shared(l, s, t, NULL, i)
-#define rwsem_release(l, i) lock_release(l, i)
+#define rwlock_release(l, i) \
+do { \
+ ldt_unlock(&(l)->dmap, i); \
+ lock_release(l, i); \
+} while (0)
+#define seqcount_acquire(l, s, t, i) \
+do { \
+ ldt_wlock(&(l)->dmap, s, t, NULL, i); \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+} while (0)
+#define seqcount_acquire_read(l, s, t, i) \
+do { \
+ ldt_rlock(&(l)->dmap, s, t, NULL, i, false); \
+ lock_acquire_shared_recursive(l, s, t, NULL, i); \
+} while (0)
+#define seqcount_release(l, i) \
+do { \
+ ldt_unlock(&(l)->dmap, i); \
+ lock_release(l, i); \
+} while (0)
+#define mutex_acquire(l, s, t, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, NULL, i); \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+} while (0)
+#define mutex_acquire_nest(l, s, t, n, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, n, i); \
+ lock_acquire_exclusive(l, s, t, n, i); \
+} while (0)
+#define mutex_release(l, i) \
+do { \
+ ldt_unlock(&(l)->dmap, i); \
+ lock_release(l, i); \
+} while (0)
+#define rwsem_acquire(l, s, t, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, NULL, i); \
+ lock_acquire_exclusive(l, s, t, NULL, i); \
+} while (0)
+#define rwsem_acquire_nest(l, s, t, n, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, n, i); \
+ lock_acquire_exclusive(l, s, t, n, i); \
+} while (0)
+#define rwsem_acquire_read(l, s, t, i) \
+do { \
+ ldt_lock(&(l)->dmap, s, t, NULL, i); \
+ lock_acquire_shared(l, s, t, NULL, i); \
+} while (0)
+#define rwsem_release(l, i) \
+do { \
+ ldt_unlock(&(l)->dmap, i); \
+ lock_release(l, i); \
+} while (0)

#define lock_map_acquire(l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h
index d22430840b53..50c887967dd8 100644
--- a/include/linux/lockdep_types.h
+++ b/include/linux/lockdep_types.h
@@ -11,6 +11,7 @@
#define __LINUX_LOCKDEP_TYPES_H

#include <linux/types.h>
+#include <linux/dept.h>

#define MAX_LOCKDEP_SUBCLASSES 8UL

@@ -76,6 +77,7 @@ struct lock_class_key {
struct hlist_node hash_entry;
struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES];
};
+ struct dept_key dkey;
};

extern struct lock_class_key __lockdep_no_validate__;
@@ -185,6 +187,7 @@ struct lockdep_map {
int cpu;
unsigned long ip;
#endif
+ struct dept_map dmap;
};

struct pin_cookie { unsigned int val; };
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index 8f226d460f51..58bf314eddeb 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -25,6 +25,7 @@
, .dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_SLEEP, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL),\
}
#else
# define __DEP_MAP_MUTEX_INITIALIZER(lockname)
diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
index 36b942b67b7d..e871aca04645 100644
--- a/include/linux/percpu-rwsem.h
+++ b/include/linux/percpu-rwsem.h
@@ -21,7 +21,7 @@ struct percpu_rw_semaphore {
};

#ifdef CONFIG_DEBUG_LOCK_ALLOC
-#define __PERCPU_RWSEM_DEP_MAP_INIT(lockname) .dep_map = { .name = #lockname },
+#define __PERCPU_RWSEM_DEP_MAP_INIT(lockname) .dep_map = { .name = #lockname, .dmap = DEPT_MAP_INITIALIZER(lockname, NULL) },
#else
#define __PERCPU_RWSEM_DEP_MAP_INIT(lockname)
#endif
diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 7d049883a08a..35889ac5eeae 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -81,6 +81,7 @@ do { \
.dep_map = { \
.name = #mutexname, \
.wait_type_inner = LD_WAIT_SLEEP, \
+ .dmap = DEPT_MAP_INITIALIZER(mutexname, NULL),\
}
#else
#define __DEP_MAP_RT_MUTEX_INITIALIZER(mutexname)
diff --git a/include/linux/rwlock_types.h b/include/linux/rwlock_types.h
index 1948442e7750..6e58dfc84997 100644
--- a/include/linux/rwlock_types.h
+++ b/include/linux/rwlock_types.h
@@ -10,6 +10,7 @@
.dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_CONFIG, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL), \
}
#else
# define RW_DEP_MAP_INIT(lockname)
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index efa5c324369a..4f856e745dce 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -21,6 +21,7 @@
.dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_SLEEP, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL),\
},
#else
# define __RWSEM_DEP_MAP_INIT(lockname)
diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index 3926e9027947..6ba00bcbc11a 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -81,7 +81,7 @@ static inline void __seqcount_init(seqcount_t *s, const char *name,
#ifdef CONFIG_DEBUG_LOCK_ALLOC

# define SEQCOUNT_DEP_MAP_INIT(lockname) \
- .dep_map = { .name = #lockname }
+ .dep_map = { .name = #lockname, .dmap = DEPT_MAP_INITIALIZER(lockname, NULL) }

/**
* seqcount_init() - runtime initializer for seqcount_t
diff --git a/include/linux/spinlock_types_raw.h b/include/linux/spinlock_types_raw.h
index 91cb36b65a17..3dcc551ded25 100644
--- a/include/linux/spinlock_types_raw.h
+++ b/include/linux/spinlock_types_raw.h
@@ -31,11 +31,13 @@ typedef struct raw_spinlock {
.dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_SPIN, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL),\
}
# define SPIN_DEP_MAP_INIT(lockname) \
.dep_map = { \
.name = #lockname, \
.wait_type_inner = LD_WAIT_CONFIG, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL),\
}

# define LOCAL_SPIN_DEP_MAP_INIT(lockname) \
@@ -43,6 +45,7 @@ typedef struct raw_spinlock {
.name = #lockname, \
.wait_type_inner = LD_WAIT_CONFIG, \
.lock_type = LD_LOCK_PERCPU, \
+ .dmap = DEPT_MAP_INITIALIZER(lockname, NULL),\
}
#else
# define RAW_SPIN_DEP_MAP_INIT(lockname)
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 9b9d0bbf1d3c..c934158ed4e8 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -35,7 +35,7 @@ int __init_srcu_struct(struct srcu_struct *ssp, const char *name,
__init_srcu_struct((ssp), #ssp, &__srcu_key); \
})

-#define __SRCU_DEP_MAP_INIT(srcu_name) .dep_map = { .name = #srcu_name },
+#define __SRCU_DEP_MAP_INIT(srcu_name) .dep_map = { .name = #srcu_name, .dmap = DEPT_MAP_INITIALIZER(srcu_name, NULL) },
#else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */

int init_srcu_struct(struct srcu_struct *ssp);
diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
index 8ec638254e5f..d3b6d2f4cd7b 100644
--- a/kernel/dependency/dept.c
+++ b/kernel/dependency/dept.c
@@ -245,10 +245,10 @@ static inline bool dept_working(void)
* Even k == NULL is considered as a valid key because it would use
* &->map_key as the key in that case.
*/
-struct dept_key __dept_no_validate__;
+extern struct lock_class_key __lockdep_no_validate__;
static inline bool valid_key(struct dept_key *k)
{
- return &__dept_no_validate__ != k;
+ return &__lockdep_no_validate__.dkey != k;
}

/*
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index e3375bc40dad..7ff3fb4d2735 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1220,6 +1220,8 @@ void lockdep_register_key(struct lock_class_key *key)
struct lock_class_key *k;
unsigned long flags;

+ dept_key_init(&key->dkey);
+
if (WARN_ON_ONCE(static_obj(key)))
return;
hash_head = keyhashentry(key);
@@ -4327,6 +4329,8 @@ void noinstr lockdep_hardirqs_on(unsigned long ip)
{
struct irqtrace_events *trace = &current->irqtrace;

+ dept_hardirqs_on_ip(ip);
+
if (unlikely(!debug_locks))
return;

@@ -4392,6 +4396,8 @@ EXPORT_SYMBOL_GPL(lockdep_hardirqs_on);
*/
void noinstr lockdep_hardirqs_off(unsigned long ip)
{
+ dept_hardirqs_off_ip(ip);
+
if (unlikely(!debug_locks))
return;

@@ -4436,6 +4442,8 @@ void lockdep_softirqs_on(unsigned long ip)
{
struct irqtrace_events *trace = &current->irqtrace;

+ dept_softirqs_on_ip(ip);
+
if (unlikely(!lockdep_enabled()))
return;

@@ -4474,6 +4482,9 @@ void lockdep_softirqs_on(unsigned long ip)
*/
void lockdep_softirqs_off(unsigned long ip)
{
+
+ dept_softirqs_off_ip(ip);
+
if (unlikely(!lockdep_enabled()))
return;

@@ -4806,6 +4817,8 @@ void lockdep_init_map_type(struct lockdep_map *lock, const char *name,
{
int i;

+ ldt_init(&lock->dmap, &key->dkey, subclass, name);
+
for (i = 0; i < NR_LOCKDEP_CACHING_CLASSES; i++)
lock->class_cache[i] = NULL;

@@ -5544,6 +5557,12 @@ void lock_set_class(struct lockdep_map *lock, const char *name,
{
unsigned long flags;

+ /*
+ * dept_map_(re)init() might be called twice redundantly. But
+ * there's no choice as long as Dept relies on Lockdep.
+ */
+ ldt_set_class(&lock->dmap, name, &key->dkey, subclass, ip);
+
if (unlikely(!lockdep_enabled()))
return;

@@ -5561,6 +5580,8 @@ void lock_downgrade(struct lockdep_map *lock, unsigned long ip)
{
unsigned long flags;

+ ldt_downgrade(&lock->dmap, ip);
+
if (unlikely(!lockdep_enabled()))
return;

@@ -6333,6 +6354,8 @@ void lockdep_unregister_key(struct lock_class_key *key)
unsigned long flags;
bool found = false;

+ dept_key_destroy(&key->dkey);
+
might_sleep();

if (WARN_ON_ONCE(static_obj(key)))
--
2.17.1

Next message: Matthew Wilcox: "Re: f2fs async buffered write patch"
Previous message: Byungchul Park: "[PATCH v10 07/25] dept: Apply sdt_might_sleep_{start,end}() to wait_for_completion()/complete()"
In reply to: Byungchul Park: "[PATCH v10 07/25] dept: Apply sdt_might_sleep_{start,end}() to wait_for_completion()/complete()"
Next in thread: Byungchul Park: "[PATCH v10 16/25] dept: Apply sdt_might_sleep_{start,end}() to dma fence wait"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]