lkml.org 
[lkml]   [2011]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][PATCH 4/7] lockdep: Seperate lock ids for read/write acquires
    From
    Date
    On Sun, 2011-04-17 at 11:45 +0200, Peter Zijlstra wrote:
    >
    > +/*
    > + * A lock's class id is used to calculate the chain-key. Since we need to
    > + * differentiate between the chains which contain the read acquire of
    > + * a lock from the chains having write acquire of the same lock,
    > + * we offset the class_idx by MAX_LOCKDEP_KEYS if it is a read acquire.

    Don't we only care to do this if we have a recursive read? I thought
    simple reads still work fine with the current algorithm?

    > + *
    > + * Thus the the lock's key during a chain-key calculation can be in the range
    > + * 1 to 2 * MAX_LOCKDEP_KEYS - 1.
    > + *
    > + * LOCKDEP_CHAIN_KEY_BITS holds the number of bits required to
    > + * represent this range.
    > + */
    > +#define LOCKDEP_CHAIN_KEY_BITS (MAX_LOCKDEP_KEYS_BITS + 1)
    > struct held_lock {
    > /*
    > * One-way hash of the dependency chain up to this point. We
    > Index: linux-2.6/kernel/lockdep.c
    > ===================================================================
    > --- linux-2.6.orig/kernel/lockdep.c
    > +++ linux-2.6/kernel/lockdep.c
    > @@ -303,8 +303,8 @@ static struct list_head chainhash_table[
    > * unique.
    > */
    > #define iterate_chain_key(key1, key2) \
    > - (((key1) << MAX_LOCKDEP_KEYS_BITS) ^ \
    > - ((key1) >> (64-MAX_LOCKDEP_KEYS_BITS)) ^ \
    > + (((key1) << LOCKDEP_CHAIN_KEY_BITS) ^ \
    > + ((key1) >> (64 - LOCKDEP_CHAIN_KEY_BITS)) ^ \
    > (key2))
    >
    > void lockdep_off(void)
    > @@ -1988,6 +1988,9 @@ static void check_chain_key(struct task_
    > if (DEBUG_LOCKS_WARN_ON(id >= MAX_LOCKDEP_KEYS))
    > return;
    >
    > + if (is_read(hlock->rw_state))
    > + id += MAX_LOCKDEP_KEYS;

    Again, isn't this about recursive reads? Or am I just confused ;)

    -- Steve

    > +
    > if (prev_hlock && (prev_hlock->irq_context !=
    > hlock->irq_context))
    > chain_key = 0;
    > @@ -2815,6 +2818,18 @@ static int __lock_acquire(struct lockdep
    > if (DEBUG_LOCKS_WARN_ON(id >= MAX_LOCKDEP_KEYS))
    > return 0;
    >
    > + /*
    > + * Factor in the read/write state in the chain key calculation.
    > + *
    > + * Two chains containing lock dependencies in the same order can
    > + * still differ due to their read/write state
    > + * eg: lock(A)->Rlock(B) is different from lock(A)->Wlock(B)
    > + *
    > + * Hence distinguish between such chains.
    > + */
    > + if (is_read(rw_state))
    > + id += MAX_LOCKDEP_KEYS;
    > +
    > chain_key = curr->curr_chain_key;
    > if (!depth) {
    > if (DEBUG_LOCKS_WARN_ON(chain_key != 0))
    >




    \
     
     \ /
      Last update: 2011-04-18 18:49    [W:0.046 / U:66.224 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site