Oops when accessing /proc/lockdep_chains - Kernel

This is a discussion on Oops when accessing /proc/lockdep_chains - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 8 of 8

Thread: Oops when accessing /proc/lockdep_chains

  1. Oops when accessing /proc/lockdep_chains

    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: [] 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:[] 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] [] ? string+0x26/0xa0
    [ 584.464510] [] ? restore_nocheck_notrace+0x0/0xe
    [ 584.464859] [] ? vsnprintf+0x3a7/0x6a0
    [ 584.465141] [] ? trace_hardirqs_on_thunk+0xc/0x10
    [ 584.465219] [] ? do_softirq+0x3e/0xb0
    [ 584.465219] [] ? restore_nocheck_notrace+0x0/0xe
    [ 584.465219] [] ? vsnprintf+0x414/0x6a0
    [ 584.465219] [] ? __lock_acquire+0x257/0xe50
    [ 584.465219] [] ? seq_printf+0x32/0x60
    [ 584.465219] [] ? print_name+0x31/0xb0
    [ 584.465219] [] ? mark_held_locks+0x40/0x80
    [ 584.465219] [] ? trace_hardirqs_on+0xb/0x10
    [ 584.465219] [] ? trace_hardirqs_on_caller+0xc9/0x160
    [ 584.465219] [] ? __mutex_lock_common+0x1e4/0x2e0
    [ 584.465219] [] ? seq_read+0x2a/0x2a0
    [ 584.465219] [] ? seq_printf+0x32/0x60
    [ 584.465219] [] ? lc_show+0x6f/0xc0
    [ 584.465219] [] ? seq_read+0x16f/0x2a0
    [ 584.465219] [] ? seq_read+0x0/0x2a0
    [ 584.465219] [] ? proc_reg_read+0x62/0x90
    [ 584.465219] [] ? vfs_read+0x99/0x160
    [ 584.465219] [] ? proc_reg_read+0x0/0x90
    [ 584.465219] [] ? sys_read+0x42/0x70
    [ 584.465219] [] ? 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: [] 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: [] 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:[] 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] [] ? string+0x26/0xa0
    [ 657.509382] [] ? vsnprintf+0x3a7/0x6a0
    [ 657.509382] [] ? try_to_wake_up+0x6b/0xf0
    [ 657.509382] [] ? vsnprintf+0x414/0x6a0
    [ 657.509382] [] ? n_tty_receive_buf+0x63d/0x1230
    [ 657.509382] [] ? __change_page_attr_set_clr+0xcf/0x4e0
    [ 657.509382] [] ? handle_level_irq+0x0/0xd0
    [ 657.509382] [] ? trace_hardirqs_off_caller+0x14/0xa0
    [ 657.509382] [] ? seq_printf+0x32/0x60
    [ 657.509382] [] ? print_name+0x31/0xb0
    [ 657.509382] [] ? trace_hardirqs_off_caller+0x14/0xa0
    [ 657.509382] [] ? trace_hardirqs_on_caller+0x15/0x160
    [ 657.509382] [] ? __mutex_lock_common+0x1e4/0x2e0
    [ 657.509382] [] ? seq_read+0x2a/0x2a0
    [ 657.509382] [] ? seq_printf+0x32/0x60
    [ 657.509382] [] ? lc_show+0x6f/0xc0
    [ 657.509382] [] ? seq_read+0x16f/0x2a0
    [ 657.509382] [] ? seq_read+0x0/0x2a0
    [ 657.509382] [] ? proc_reg_read+0x62/0x90
    [ 657.509382] [] ? vfs_read+0x99/0x160
    [ 657.509382] [] ? proc_reg_read+0x0/0x90
    [ 657.509382] [] ? sys_read+0x42/0x70
    [ 657.509382] [] ? sysenter_do_call+0x12/0x31
    [ 657.509382] [] ? __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: [] strnlen+0xe/0x20 SS:ESP 0068:c867fce8
    [ 657.510433] ---[ end trace 560310b80b6f2ef3 ]---



    Greetings, Eric
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: Oops when accessing /proc/lockdep_chains

    * Eric Sesterhenn (snakebyte@gmx.de) 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [PATCH] 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.

    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
    Signed-off-by: Rabin Vincent
    ---
    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: Oops when accessing /proc/lockdep_chains

    On Wed, Aug 06, 2008 at 02:41:34PM +0200, Eric Sesterhenn wrote:
    > * Eric Sesterhenn (snakebyte@gmx.de) 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] lockdep: handle chains involving classes defined in modules

    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
    > Signed-off-by: Rabin Vincent
    > ---
    > 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;
    > }


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] lockdep: handle chains involving classes defined in modules

    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
    > Signed-off-by: Rabin Vincent


    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

    > ---
    > 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;
    > }


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH] lockdep: handle chains involving classes defined in modules

    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
    Signed-off-by: Rabin Vincent

    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");
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH] lockdep: handle chains involving classes defined in modules

    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
    > Signed-off-by: Rabin Vincent
    >
    > 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

    Best Regards,
    Huang Ying


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread