hi,
with current -git (aka -rc2) i sometime get the following oops
when doing a cat /proc/lockdep_chains
The other /proc/lockdep* files dont cause any errors
I dont get it after a fresh reboot :| But was able to reproduce it when
running my testscripts, I'll try to narrow it down.
[ 584.458673] BUG: unable to handle kernel paging request at d1a27580
[ 584.459010] IP: [<c049403e>] strnlen+0xe/0x20
[ 584.459340] Oops: 0000 [#1] DEBUG_PAGEALLOC
[ 584.459596] Modules linked in: [last unloaded: rcutorture]
[ 584.459923]
[ 584.460038] Pid: 9047, comm: cat Not tainted (2.6.27-rc2 #26)
[ 584.460245] EIP: 0060:[<c049403e>] EFLAGS: 00010297 CPU: 0
[ 584.460394] EIP is at strnlen+0xe/0x20
[ 584.460532] EAX: d1a27580 EBX: c86efca8 ECX: d1a27580 EDX: fffffffe
[ 584.460682] ESI: 00000000 EDI: c86f0000 EBP: c87fdce8 ESP: c87fdce8
[ 584.460890] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
[ 584.461035] Process cat (pid: 9047, ti=c87fd000 task=c87d2420 task.ti=c87fd000)
[ 584.461193] Stack: c87fdd04 c0493036 c010317c d1a27580 c86efca8 00000000 c87fde5c c87fde34
[ 584.462138] c0493457 ffffffff ffffffff 00000000 ffffffff ffffffff 00000000 00000358
[ 584.463021] c86efca8 ffffffff c86f0000 ffffffff ffffffff ffffffff 0000000f c0a3b725
[ 584.463964] Call Trace:
[ 584.464173] [<c0493036>] ? string+0x26/0xa0
[ 584.464510] [<c010317c>] ? restore_nocheck_notrace+0x0/0xe
[ 584.464859] [<c0493457>] ? vsnprintf+0x3a7/0x6a0
[ 584.465141] [<c04940ac>] ? trace_hardirqs_on_thunk+0xc/0x10
[ 584.465219] [<c01059de>] ? do_softirq+0x3e/0xb0
[ 584.465219] [<c010317c>] ? restore_nocheck_notrace+0x0/0xe
[ 584.465219] [<c04934c4>] ? vsnprintf+0x414/0x6a0
[ 584.465219] [<c0149837>] ? __lock_acquire+0x257/0xe50
[ 584.465219] [<c01b46c2>] ? seq_printf+0x32/0x60
[ 584.465219] [<c014aa81>] ? print_name+0x31/0xb0
[ 584.465219] [<c0149260>] ? mark_held_locks+0x40/0x80
[ 584.465219] [<c01494ab>] ? trace_hardirqs_on+0xb/0x10
[ 584.465219] [<c0149409>] ? trace_hardirqs_on_caller+0xc9/0x160
[ 584.465219] [<c0830f64>] ? __mutex_lock_common+0x1e4/0x2e0
[ 584.465219] [<c01b4c1a>] ? seq_read+0x2a/0x2a0
[ 584.465219] [<c01b46c2>] ? seq_printf+0x32/0x60
[ 584.465219] [<c014ab6f>] ? lc_show+0x6f/0xc0
[ 584.465219] [<c01b4d5f>] ? seq_read+0x16f/0x2a0
[ 584.465219] [<c01b4bf0>] ? seq_read+0x0/0x2a0
[ 584.465219] [<c01d58f2>] ? proc_reg_read+0x62/0x90
[ 584.465219] [<c0199269>] ? vfs_read+0x99/0x160
[ 584.465219] [<c01d5890>] ? proc_reg_read+0x0/0x90
[ 584.465219] [<c0199842>] ? sys_read+0x42/0x70
[ 584.465219] [<c0103059>] ? sysenter_do_call+0x12/0x31
[ 584.465219] =======================
[ 584.465219] Code: 57 0f 1f 44 00 00 85 c9 89 c7 89 d0 74 05 f2 ae 75 01 4f 89 f8 5f 5d c3 90 8d 74 26 00 55 89 e5 0f 1f 44 00 00 89 c1 89 c8 eb 06 <80> 38 00 74 07 40 4a 83 fa ff 75 f4 29 c8 5d c3 90 90 55 89 e5
[ 584.465219] EIP: [<c049403e>] strnlen+0xe/0x20 SS:ESP 0068:c87fdce8
[ 584.465219] ---[ end trace 560310b80b6f2ef3 ]---
[ 657.508210] BUG: unable to handle kernel paging request at d1a27580
[ 657.508546] IP: [<c049403e>] strnlen+0xe/0x20
[ 657.508827] *pde = 0ef53067 *pte = 00000000
[ 657.509067] Oops: 0000 [#2] DEBUG_PAGEALLOC
[ 657.509364] Modules linked in: [last unloaded: rcutorture]
[ 657.509382]
[ 657.509382] Pid: 14555, comm: cat Tainted: G D (2.6.27-rc2 #26)
[ 657.509382] EIP: 0060:[<c049403e>] EFLAGS: 00010297 CPU: 0
[ 657.509382] EIP is at strnlen+0xe/0x20
[ 657.509382] EAX: d1a27580 EBX: c86ef36c ECX: d1a27580 EDX: fffffffe
[ 657.509382] ESI: 00000000 EDI: c86f0000 EBP: c867fce8 ESP: c867fce8
[ 657.509382] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
[ 657.509382] Process cat (pid: 14555, ti=c867f000 task=c86a4420 task.ti=c867f000)
[ 657.509382] Stack: c867fd04 c0493036 00000000 d1a27580 c86ef36c 00000000 c867fe5c c867fe34
[ 657.509382] c0493457 ffffffff ffffffff 00000000 00000000 c867fd44 00000046 00000c94
[ 657.509382] c86ef36c ffffffff c86f0000 ffffffff ffffffff ffffffff 0000000f c0a3b725
[ 657.509382] Call Trace:
[ 657.509382] [<c0493036>] ? string+0x26/0xa0
[ 657.509382] [<c0493457>] ? vsnprintf+0x3a7/0x6a0
[ 657.509382] [<c012417b>] ? try_to_wake_up+0x6b/0xf0
[ 657.509382] [<c04934c4>] ? vsnprintf+0x414/0x6a0
[ 657.509382] [<c0514aed>] ? n_tty_receive_buf+0x63d/0x1230
[ 657.509382] [<c01189ff>] ? __change_page_attr_set_clr+0xcf/0x4e0
[ 657.509382] [<c0164ef0>] ? handle_level_irq+0x0/0xd0
[ 657.509382] [<c0146d84>] ? trace_hardirqs_off_caller+0x14/0xa0
[ 657.509382] [<c01b46c2>] ? seq_printf+0x32/0x60
[ 657.509382] [<c014aa81>] ? print_name+0x31/0xb0
[ 657.509382] [<c0146d84>] ? trace_hardirqs_off_caller+0x14/0xa0
[ 657.509382] [<c0149355>] ? trace_hardirqs_on_caller+0x15/0x160
[ 657.509382] [<c0830f64>] ? __mutex_lock_common+0x1e4/0x2e0
[ 657.509382] [<c01b4c1a>] ? seq_read+0x2a/0x2a0
[ 657.509382] [<c01b46c2>] ? seq_printf+0x32/0x60
[ 657.509382] [<c014ab6f>] ? lc_show+0x6f/0xc0
[ 657.509382] [<c01b4d5f>] ? seq_read+0x16f/0x2a0
[ 657.509382] [<c01b4bf0>] ? seq_read+0x0/0x2a0
[ 657.509382] [<c01d58f2>] ? proc_reg_read+0x62/0x90
[ 657.509382] [<c0199269>] ? vfs_read+0x99/0x160
[ 657.509382] [<c01d5890>] ? proc_reg_read+0x0/0x90
[ 657.509382] [<c0199842>] ? sys_read+0x42/0x70
[ 657.509382] [<c0103059>] ? sysenter_do_call+0x12/0x31
[ 657.509382] [<c0124abf>] ? __cleanup_sighand+0x1f/0x30
[ 657.509382] =======================
[ 657.509382] Code: 57 0f 1f 44 00 00 85 c9 89 c7 89 d0 74 05 f2 ae 75 01 4f 89 f8 5f 5d c3 90 8d 74 26 00 55 89 e5 0f 1f 44 00 00 89 c1 89 c8 eb 06 <80> 38 00 74 07 40 4a 83 fa ff 75 f4 29 c8 5d c3 90 90 55 89 e5
[ 657.509382] EIP: [<c049403e>] strnlen+0xe/0x20 SS:ESP 0068:c867fce8
[ 657.510433] ---[ end trace 560310b80b6f2ef3 ]---
Greetings, Eric
* Eric Sesterhenn ([email protected]) wrote:
> hi,
>
> with current -git (aka -rc2) i sometime get the following oops
> when doing a cat /proc/lockdep_chains
> The other /proc/lockdep* files dont cause any errors
>
> I dont get it after a fresh reboot :| But was able to reproduce it when
> running my testscripts, I'll try to narrow it down.
I can easily reproduce this with
modprobe rcutorture; sleep 10s; rmmod rcutorture; cat
/proc/lockdep_chains > /dev/null
Greetings, Eric
On Wed, Aug 06, 2008 at 02:41:34PM +0200, Eric Sesterhenn wrote:
> * Eric Sesterhenn ([email protected]) wrote:
> > hi,
> >
> > with current -git (aka -rc2) i sometime get the following oops
> > when doing a cat /proc/lockdep_chains
> > The other /proc/lockdep* files dont cause any errors
> >
> > I dont get it after a fresh reboot :| But was able to reproduce it when
> > running my testscripts, I'll try to narrow it down.
>
> I can easily reproduce this with
>
> modprobe rcutorture; sleep 10s; rmmod rcutorture; cat
> /proc/lockdep_chains > /dev/null
Indeed, it oopses after any module which creates a lock is unloaded.
I'll send a patch for this shortly.
Rabin
/proc/lockdep_chains currently oopses after any module which creates and
uses a lock is unloaded. This is because one of the chains involves a
class which was defined in the module just unloaded.
The classes are already correctly taken care of using the
all_lock_classes which keeps track of all active lock classses. Add a
similar all_lock_chains list and use it for keeping track of chains.
Reported-by: Eric Sesterhenn <[email protected]>
Signed-off-by: Rabin Vincent <[email protected]>
---
include/linux/lockdep.h | 3 +-
kernel/lockdep.c | 53 ++++++++++++++++++++++++++++++++++++++-----
kernel/lockdep_internals.h | 2 +-
kernel/lockdep_proc.c | 17 ++++++++++----
4 files changed, 61 insertions(+), 14 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 2486eb4..735d06b 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -185,7 +185,8 @@ struct lock_chain {
u8 irq_context;
u8 depth;
u16 base;
- struct list_head entry;
+ struct list_head hash_entry;
+ struct list_head lock_entry;
u64 chain_key;
};
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index d38a643..1b3537b 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -253,6 +253,11 @@ LIST_HEAD(all_lock_classes);
static struct list_head classhash_table[CLASSHASH_SIZE];
/*
+ * We also keep a global list of all lock chains.
+ */
+LIST_HEAD(all_lock_chains);
+
+/*
* We put the lock dependency chains into a hash-table as well, to cache
* their existence:
*/
@@ -1462,7 +1467,7 @@ out_bug:
}
unsigned long nr_lock_chains;
-struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
+static struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
int nr_chain_hlocks;
static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
@@ -1493,7 +1498,7 @@ static inline int lookup_chain_cache(struct task_struct *curr,
* We can walk it lock-free, because entries only get added
* to the hash:
*/
- list_for_each_entry(chain, hash_head, entry) {
+ list_for_each_entry(chain, hash_head, hash_entry) {
if (chain->chain_key == chain_key) {
cache_hit:
debug_atomic_inc(&chain_lookup_hits);
@@ -1517,7 +1522,7 @@ cache_hit:
/*
* We have to walk the chain again locked - to avoid duplicates:
*/
- list_for_each_entry(chain, hash_head, entry) {
+ list_for_each_entry(chain, hash_head, hash_entry) {
if (chain->chain_key == chain_key) {
graph_unlock();
goto cache_hit;
@@ -1559,7 +1564,8 @@ cache_hit:
}
chain_hlocks[chain->base + j] = class - lock_classes;
}
- list_add_tail_rcu(&chain->entry, hash_head);
+ list_add_tail_rcu(&chain->hash_entry, hash_head);
+ list_add_tail_rcu(&chain->lock_entry, &all_lock_chains);
debug_atomic_inc(&chain_lookup_misses);
inc_chains();
@@ -2967,6 +2973,8 @@ void lockdep_reset(void)
debug_locks = 1;
for (i = 0; i < CHAINHASH_SIZE; i++)
INIT_LIST_HEAD(chainhash_table + i);
+ for (i = 0; i < CLASSHASH_SIZE; i++)
+ INIT_LIST_HEAD(classhash_table + i);
raw_local_irq_restore(flags);
}
@@ -2990,6 +2998,15 @@ static void zap_class(struct lock_class *class)
}
+static void zap_chain(struct lock_chain *chain)
+{
+ /*
+ * Unhash the chain and remove it from the all_lock_chains list:
+ */
+ list_del_rcu(&chain->hash_entry);
+ list_del_rcu(&chain->lock_entry);
+}
+
static inline int within(const void *addr, void *start, unsigned long size)
{
return addr >= start && addr < start + size;
@@ -2997,23 +3014,45 @@ static inline int within(const void *addr, void *start, unsigned long size)
void lockdep_free_key_range(void *start, unsigned long size)
{
- struct lock_class *class, *next;
+ struct lock_class *class, *nextclass;
+ struct lock_chain *chain, *nextchain;
struct list_head *head;
unsigned long flags;
- int i;
+ int i, j;
int locked;
raw_local_irq_save(flags);
locked = graph_lock();
/*
+ * Unhash all chains that involve classes created by this module:
+ */
+ for (i = 0; i < CHAINHASH_SIZE; i++) {
+ head = chainhash_table + i;
+ if (list_empty(head))
+ continue;
+ list_for_each_entry_safe(chain, nextchain, head, hash_entry) {
+ for (j = 0; j < chain->depth; j++) {
+ class = lock_classes +
+ chain_hlocks[chain->base + j];
+
+ if (within(class->key, start, size) ||
+ within(class->name, start, size)) {
+ zap_chain(chain);
+ break;
+ }
+ }
+ }
+ }
+
+ /*
* Unhash all classes that were created by this module:
*/
for (i = 0; i < CLASSHASH_SIZE; i++) {
head = classhash_table + i;
if (list_empty(head))
continue;
- list_for_each_entry_safe(class, next, head, hash_entry) {
+ list_for_each_entry_safe(class, nextclass, head, hash_entry) {
if (within(class->key, start, size))
zap_class(class);
else if (within(class->name, start, size))
diff --git a/kernel/lockdep_internals.h b/kernel/lockdep_internals.h
index c3600a0..ebf2ecb 100644
--- a/kernel/lockdep_internals.h
+++ b/kernel/lockdep_internals.h
@@ -32,7 +32,7 @@
#define MAX_STACK_TRACE_ENTRIES 262144UL
extern struct list_head all_lock_classes;
-extern struct lock_chain lock_chains[];
+extern struct list_head all_lock_chains;
extern void
get_usage_chars(struct lock_class *class, char *c1, char *c2, char *c3, char *c4);
diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
index 9b0e940..4f447fd 100644
--- a/kernel/lockdep_proc.c
+++ b/kernel/lockdep_proc.c
@@ -190,8 +190,9 @@ static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
else {
chain = v;
- if (*pos < nr_lock_chains)
- chain = lock_chains + *pos;
+ if (chain->lock_entry.next != &all_lock_chains)
+ chain = list_entry(chain->lock_entry.next,
+ struct lock_chain, lock_entry);
else
chain = NULL;
}
@@ -201,11 +202,16 @@ static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
static void *lc_start(struct seq_file *m, loff_t *pos)
{
+ struct lock_chain *chain;
+ loff_t i = 0;
+
if (*pos == 0)
return SEQ_START_TOKEN;
- if (*pos < nr_lock_chains)
- return lock_chains + *pos;
+ list_for_each_entry(chain, &all_lock_chains, lock_entry) {
+ if (++i == *pos)
+ return chain;
+ }
return NULL;
}
@@ -252,7 +258,8 @@ static int lockdep_chains_open(struct inode *inode, struct file *file)
struct seq_file *m = file->private_data;
if (nr_lock_chains)
- m->private = lock_chains;
+ m->private = list_entry(all_lock_chains.next,
+ struct lock_chain, lock_entry);
else
m->private = NULL;
}
--
1.5.6.3
Hi, Rabin,
I think there is a simpler method to deal with this.
- Mark class as useless during zap_class()
- When output lock_chain, if some classes are useless, do not output the
class.
Best Regards,
Huang Ying
On Fri, 2008-08-08 at 02:27 +0530, Rabin Vincent wrote:
> /proc/lockdep_chains currently oopses after any module which creates and
> uses a lock is unloaded. This is because one of the chains involves a
> class which was defined in the module just unloaded.
>
> The classes are already correctly taken care of using the
> all_lock_classes which keeps track of all active lock classses. Add a
> similar all_lock_chains list and use it for keeping track of chains.
>
> Reported-by: Eric Sesterhenn <[email protected]>
> Signed-off-by: Rabin Vincent <[email protected]>
> ---
> include/linux/lockdep.h | 3 +-
> kernel/lockdep.c | 53 ++++++++++++++++++++++++++++++++++++++-----
> kernel/lockdep_internals.h | 2 +-
> kernel/lockdep_proc.c | 17 ++++++++++----
> 4 files changed, 61 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index 2486eb4..735d06b 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -185,7 +185,8 @@ struct lock_chain {
> u8 irq_context;
> u8 depth;
> u16 base;
> - struct list_head entry;
> + struct list_head hash_entry;
> + struct list_head lock_entry;
> u64 chain_key;
> };
>
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index d38a643..1b3537b 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -253,6 +253,11 @@ LIST_HEAD(all_lock_classes);
> static struct list_head classhash_table[CLASSHASH_SIZE];
>
> /*
> + * We also keep a global list of all lock chains.
> + */
> +LIST_HEAD(all_lock_chains);
> +
> +/*
> * We put the lock dependency chains into a hash-table as well, to cache
> * their existence:
> */
> @@ -1462,7 +1467,7 @@ out_bug:
> }
>
> unsigned long nr_lock_chains;
> -struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
> +static struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
> int nr_chain_hlocks;
> static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
>
> @@ -1493,7 +1498,7 @@ static inline int lookup_chain_cache(struct task_struct *curr,
> * We can walk it lock-free, because entries only get added
> * to the hash:
> */
> - list_for_each_entry(chain, hash_head, entry) {
> + list_for_each_entry(chain, hash_head, hash_entry) {
> if (chain->chain_key == chain_key) {
> cache_hit:
> debug_atomic_inc(&chain_lookup_hits);
> @@ -1517,7 +1522,7 @@ cache_hit:
> /*
> * We have to walk the chain again locked - to avoid duplicates:
> */
> - list_for_each_entry(chain, hash_head, entry) {
> + list_for_each_entry(chain, hash_head, hash_entry) {
> if (chain->chain_key == chain_key) {
> graph_unlock();
> goto cache_hit;
> @@ -1559,7 +1564,8 @@ cache_hit:
> }
> chain_hlocks[chain->base + j] = class - lock_classes;
> }
> - list_add_tail_rcu(&chain->entry, hash_head);
> + list_add_tail_rcu(&chain->hash_entry, hash_head);
> + list_add_tail_rcu(&chain->lock_entry, &all_lock_chains);
> debug_atomic_inc(&chain_lookup_misses);
> inc_chains();
>
> @@ -2967,6 +2973,8 @@ void lockdep_reset(void)
> debug_locks = 1;
> for (i = 0; i < CHAINHASH_SIZE; i++)
> INIT_LIST_HEAD(chainhash_table + i);
> + for (i = 0; i < CLASSHASH_SIZE; i++)
> + INIT_LIST_HEAD(classhash_table + i);
> raw_local_irq_restore(flags);
> }
>
> @@ -2990,6 +2998,15 @@ static void zap_class(struct lock_class *class)
>
> }
>
> +static void zap_chain(struct lock_chain *chain)
> +{
> + /*
> + * Unhash the chain and remove it from the all_lock_chains list:
> + */
> + list_del_rcu(&chain->hash_entry);
> + list_del_rcu(&chain->lock_entry);
> +}
> +
> static inline int within(const void *addr, void *start, unsigned long size)
> {
> return addr >= start && addr < start + size;
> @@ -2997,23 +3014,45 @@ static inline int within(const void *addr, void *start, unsigned long size)
>
> void lockdep_free_key_range(void *start, unsigned long size)
> {
> - struct lock_class *class, *next;
> + struct lock_class *class, *nextclass;
> + struct lock_chain *chain, *nextchain;
> struct list_head *head;
> unsigned long flags;
> - int i;
> + int i, j;
> int locked;
>
> raw_local_irq_save(flags);
> locked = graph_lock();
>
> /*
> + * Unhash all chains that involve classes created by this module:
> + */
> + for (i = 0; i < CHAINHASH_SIZE; i++) {
> + head = chainhash_table + i;
> + if (list_empty(head))
> + continue;
> + list_for_each_entry_safe(chain, nextchain, head, hash_entry) {
> + for (j = 0; j < chain->depth; j++) {
> + class = lock_classes +
> + chain_hlocks[chain->base + j];
> +
> + if (within(class->key, start, size) ||
> + within(class->name, start, size)) {
> + zap_chain(chain);
> + break;
> + }
> + }
> + }
> + }
> +
> + /*
> * Unhash all classes that were created by this module:
> */
> for (i = 0; i < CLASSHASH_SIZE; i++) {
> head = classhash_table + i;
> if (list_empty(head))
> continue;
> - list_for_each_entry_safe(class, next, head, hash_entry) {
> + list_for_each_entry_safe(class, nextclass, head, hash_entry) {
> if (within(class->key, start, size))
> zap_class(class);
> else if (within(class->name, start, size))
> diff --git a/kernel/lockdep_internals.h b/kernel/lockdep_internals.h
> index c3600a0..ebf2ecb 100644
> --- a/kernel/lockdep_internals.h
> +++ b/kernel/lockdep_internals.h
> @@ -32,7 +32,7 @@
> #define MAX_STACK_TRACE_ENTRIES 262144UL
>
> extern struct list_head all_lock_classes;
> -extern struct lock_chain lock_chains[];
> +extern struct list_head all_lock_chains;
>
> extern void
> get_usage_chars(struct lock_class *class, char *c1, char *c2, char *c3, char *c4);
> diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
> index 9b0e940..4f447fd 100644
> --- a/kernel/lockdep_proc.c
> +++ b/kernel/lockdep_proc.c
> @@ -190,8 +190,9 @@ static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
> else {
> chain = v;
>
> - if (*pos < nr_lock_chains)
> - chain = lock_chains + *pos;
> + if (chain->lock_entry.next != &all_lock_chains)
> + chain = list_entry(chain->lock_entry.next,
> + struct lock_chain, lock_entry);
> else
> chain = NULL;
> }
> @@ -201,11 +202,16 @@ static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
>
> static void *lc_start(struct seq_file *m, loff_t *pos)
> {
> + struct lock_chain *chain;
> + loff_t i = 0;
> +
> if (*pos == 0)
> return SEQ_START_TOKEN;
>
> - if (*pos < nr_lock_chains)
> - return lock_chains + *pos;
> + list_for_each_entry(chain, &all_lock_chains, lock_entry) {
> + if (++i == *pos)
> + return chain;
> + }
>
> return NULL;
> }
> @@ -252,7 +258,8 @@ static int lockdep_chains_open(struct inode *inode, struct file *file)
> struct seq_file *m = file->private_data;
>
> if (nr_lock_chains)
> - m->private = lock_chains;
> + m->private = list_entry(all_lock_chains.next,
> + struct lock_chain, lock_entry);
> else
> m->private = NULL;
> }
On Fri, 2008-08-08 at 02:27 +0530, Rabin Vincent wrote:
> /proc/lockdep_chains currently oopses after any module which creates and
> uses a lock is unloaded. This is because one of the chains involves a
> class which was defined in the module just unloaded.
>
> The classes are already correctly taken care of using the
> all_lock_classes which keeps track of all active lock classses. Add a
> similar all_lock_chains list and use it for keeping track of chains.
>
> Reported-by: Eric Sesterhenn <[email protected]>
> Signed-off-by: Rabin Vincent <[email protected]>
Looks good - I was about to start poking at this and then I found your
patch in my mailbox, most appreciated!
Acked-by: Peter Zijlstra <[email protected]>
> ---
> include/linux/lockdep.h | 3 +-
> kernel/lockdep.c | 53 ++++++++++++++++++++++++++++++++++++++-----
> kernel/lockdep_internals.h | 2 +-
> kernel/lockdep_proc.c | 17 ++++++++++----
> 4 files changed, 61 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index 2486eb4..735d06b 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -185,7 +185,8 @@ struct lock_chain {
> u8 irq_context;
> u8 depth;
> u16 base;
> - struct list_head entry;
> + struct list_head hash_entry;
> + struct list_head lock_entry;
> u64 chain_key;
> };
>
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index d38a643..1b3537b 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -253,6 +253,11 @@ LIST_HEAD(all_lock_classes);
> static struct list_head classhash_table[CLASSHASH_SIZE];
>
> /*
> + * We also keep a global list of all lock chains.
> + */
> +LIST_HEAD(all_lock_chains);
> +
> +/*
> * We put the lock dependency chains into a hash-table as well, to cache
> * their existence:
> */
> @@ -1462,7 +1467,7 @@ out_bug:
> }
>
> unsigned long nr_lock_chains;
> -struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
> +static struct lock_chain lock_chains[MAX_LOCKDEP_CHAINS];
> int nr_chain_hlocks;
> static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS];
>
> @@ -1493,7 +1498,7 @@ static inline int lookup_chain_cache(struct task_struct *curr,
> * We can walk it lock-free, because entries only get added
> * to the hash:
> */
> - list_for_each_entry(chain, hash_head, entry) {
> + list_for_each_entry(chain, hash_head, hash_entry) {
> if (chain->chain_key == chain_key) {
> cache_hit:
> debug_atomic_inc(&chain_lookup_hits);
> @@ -1517,7 +1522,7 @@ cache_hit:
> /*
> * We have to walk the chain again locked - to avoid duplicates:
> */
> - list_for_each_entry(chain, hash_head, entry) {
> + list_for_each_entry(chain, hash_head, hash_entry) {
> if (chain->chain_key == chain_key) {
> graph_unlock();
> goto cache_hit;
> @@ -1559,7 +1564,8 @@ cache_hit:
> }
> chain_hlocks[chain->base + j] = class - lock_classes;
> }
> - list_add_tail_rcu(&chain->entry, hash_head);
> + list_add_tail_rcu(&chain->hash_entry, hash_head);
> + list_add_tail_rcu(&chain->lock_entry, &all_lock_chains);
> debug_atomic_inc(&chain_lookup_misses);
> inc_chains();
>
> @@ -2967,6 +2973,8 @@ void lockdep_reset(void)
> debug_locks = 1;
> for (i = 0; i < CHAINHASH_SIZE; i++)
> INIT_LIST_HEAD(chainhash_table + i);
> + for (i = 0; i < CLASSHASH_SIZE; i++)
> + INIT_LIST_HEAD(classhash_table + i);
> raw_local_irq_restore(flags);
> }
>
> @@ -2990,6 +2998,15 @@ static void zap_class(struct lock_class *class)
>
> }
>
> +static void zap_chain(struct lock_chain *chain)
> +{
> + /*
> + * Unhash the chain and remove it from the all_lock_chains list:
> + */
> + list_del_rcu(&chain->hash_entry);
> + list_del_rcu(&chain->lock_entry);
> +}
> +
> static inline int within(const void *addr, void *start, unsigned long size)
> {
> return addr >= start && addr < start + size;
> @@ -2997,23 +3014,45 @@ static inline int within(const void *addr, void *start, unsigned long size)
>
> void lockdep_free_key_range(void *start, unsigned long size)
> {
> - struct lock_class *class, *next;
> + struct lock_class *class, *nextclass;
> + struct lock_chain *chain, *nextchain;
> struct list_head *head;
> unsigned long flags;
> - int i;
> + int i, j;
> int locked;
>
> raw_local_irq_save(flags);
> locked = graph_lock();
>
> /*
> + * Unhash all chains that involve classes created by this module:
> + */
> + for (i = 0; i < CHAINHASH_SIZE; i++) {
> + head = chainhash_table + i;
> + if (list_empty(head))
> + continue;
> + list_for_each_entry_safe(chain, nextchain, head, hash_entry) {
> + for (j = 0; j < chain->depth; j++) {
> + class = lock_classes +
> + chain_hlocks[chain->base + j];
> +
> + if (within(class->key, start, size) ||
> + within(class->name, start, size)) {
> + zap_chain(chain);
> + break;
> + }
> + }
> + }
> + }
> +
> + /*
> * Unhash all classes that were created by this module:
> */
> for (i = 0; i < CLASSHASH_SIZE; i++) {
> head = classhash_table + i;
> if (list_empty(head))
> continue;
> - list_for_each_entry_safe(class, next, head, hash_entry) {
> + list_for_each_entry_safe(class, nextclass, head, hash_entry) {
> if (within(class->key, start, size))
> zap_class(class);
> else if (within(class->name, start, size))
> diff --git a/kernel/lockdep_internals.h b/kernel/lockdep_internals.h
> index c3600a0..ebf2ecb 100644
> --- a/kernel/lockdep_internals.h
> +++ b/kernel/lockdep_internals.h
> @@ -32,7 +32,7 @@
> #define MAX_STACK_TRACE_ENTRIES 262144UL
>
> extern struct list_head all_lock_classes;
> -extern struct lock_chain lock_chains[];
> +extern struct list_head all_lock_chains;
>
> extern void
> get_usage_chars(struct lock_class *class, char *c1, char *c2, char *c3, char *c4);
> diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
> index 9b0e940..4f447fd 100644
> --- a/kernel/lockdep_proc.c
> +++ b/kernel/lockdep_proc.c
> @@ -190,8 +190,9 @@ static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
> else {
> chain = v;
>
> - if (*pos < nr_lock_chains)
> - chain = lock_chains + *pos;
> + if (chain->lock_entry.next != &all_lock_chains)
> + chain = list_entry(chain->lock_entry.next,
> + struct lock_chain, lock_entry);
> else
> chain = NULL;
> }
> @@ -201,11 +202,16 @@ static void *lc_next(struct seq_file *m, void *v, loff_t *pos)
>
> static void *lc_start(struct seq_file *m, loff_t *pos)
> {
> + struct lock_chain *chain;
> + loff_t i = 0;
> +
> if (*pos == 0)
> return SEQ_START_TOKEN;
>
> - if (*pos < nr_lock_chains)
> - return lock_chains + *pos;
> + list_for_each_entry(chain, &all_lock_chains, lock_entry) {
> + if (++i == *pos)
> + return chain;
> + }
>
> return NULL;
> }
> @@ -252,7 +258,8 @@ static int lockdep_chains_open(struct inode *inode, struct file *file)
> struct seq_file *m = file->private_data;
>
> if (nr_lock_chains)
> - m->private = lock_chains;
> + m->private = list_entry(all_lock_chains.next,
> + struct lock_chain, lock_entry);
> else
> m->private = NULL;
> }
On Fri, Aug 08, 2008 at 11:24:37AM +0800, Huang Ying wrote:
> On Fri, 2008-08-08 at 02:27 +0530, Rabin Vincent wrote:
> > /proc/lockdep_chains currently oopses after any module which creates and
> > uses a lock is unloaded. This is because one of the chains involves a
> > class which was defined in the module just unloaded.
> >
> > The classes are already correctly taken care of using the
> > all_lock_classes which keeps track of all active lock classses. Add a
> > similar all_lock_chains list and use it for keeping track of chains.
> >
[...]
>
> I think there is a simpler method to deal with this.
Yes. I went with the all_lock_chains list approach because there was
similar code already being used to keep track of lock_class structures.
> - Mark class as useless during zap_class()
> - When output lock_chain, if some classes are useless, do not output the
> class.
Like the patch below? I set ->key to NULL after zapping the class and
use that as a condition to not print the class' information. The only
issue is that with this patch there will be some chains output with no
locks listed under them.
---
lockdep: handle chains involving classes defined in modules
/proc/lockdep_chains currently oopses after any module which creates and
uses a lock is unloaded. This is because one of the chains involves a
class which was defined in the module just unloaded.
Solve this by marking the classes as unused and not printing information
about the unused classes.
Reported-by: Eric Sesterhenn <[email protected]>
Signed-off-by: Rabin Vincent <[email protected]>
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index d38a643..8ade874 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2988,6 +2988,7 @@ static void zap_class(struct lock_class *class)
list_del_rcu(&class->hash_entry);
list_del_rcu(&class->lock_entry);
+ class->key = NULL;
}
static inline int within(const void *addr, void *start, unsigned long size)
diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
index 9b0e940..f09b6c7 100644
--- a/kernel/lockdep_proc.c
+++ b/kernel/lockdep_proc.c
@@ -229,6 +229,9 @@ static int lc_show(struct seq_file *m, void *v)
for (i = 0; i < chain->depth; i++) {
class = lock_chain_get_class(chain, i);
+ if (!class->key)
+ continue;
+
seq_printf(m, "[%p] ", class->key);
print_name(m, class);
seq_puts(m, "\n");
On Sat, 2008-08-09 at 03:55 +0530, Rabin Vincent wrote:
> On Fri, Aug 08, 2008 at 11:24:37AM +0800, Huang Ying wrote:
> > On Fri, 2008-08-08 at 02:27 +0530, Rabin Vincent wrote:
> > > /proc/lockdep_chains currently oopses after any module which creates and
> > > uses a lock is unloaded. This is because one of the chains involves a
> > > class which was defined in the module just unloaded.
> > >
> > > The classes are already correctly taken care of using the
> > > all_lock_classes which keeps track of all active lock classses. Add a
> > > similar all_lock_chains list and use it for keeping track of chains.
> > >
> [...]
> >
> > I think there is a simpler method to deal with this.
>
> Yes. I went with the all_lock_chains list approach because there was
> similar code already being used to keep track of lock_class structures.
>
> > - Mark class as useless during zap_class()
> > - When output lock_chain, if some classes are useless, do not output the
> > class.
>
> Like the patch below? I set ->key to NULL after zapping the class and
> use that as a condition to not print the class' information. The only
> issue is that with this patch there will be some chains output with no
> locks listed under them.
>
> ---
> lockdep: handle chains involving classes defined in modules
>
> /proc/lockdep_chains currently oopses after any module which creates and
> uses a lock is unloaded. This is because one of the chains involves a
> class which was defined in the module just unloaded.
>
> Solve this by marking the classes as unused and not printing information
> about the unused classes.
>
> Reported-by: Eric Sesterhenn <[email protected]>
> Signed-off-by: Rabin Vincent <[email protected]>
>
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index d38a643..8ade874 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -2988,6 +2988,7 @@ static void zap_class(struct lock_class *class)
> list_del_rcu(&class->hash_entry);
> list_del_rcu(&class->lock_entry);
>
> + class->key = NULL;
> }
>
> static inline int within(const void *addr, void *start, unsigned long size)
> diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
> index 9b0e940..f09b6c7 100644
> --- a/kernel/lockdep_proc.c
> +++ b/kernel/lockdep_proc.c
> @@ -229,6 +229,9 @@ static int lc_show(struct seq_file *m, void *v)
>
> for (i = 0; i < chain->depth; i++) {
> class = lock_chain_get_class(chain, i);
> + if (!class->key)
> + continue;
> +
> seq_printf(m, "[%p] ", class->key);
> print_name(m, class);
> seq_puts(m, "\n");
I think this patch is OK.
Acked-by: Huang Ying <[email protected]>
Best Regards,
Huang Ying