lkml.org 
[lkml]   [2006]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: lockdep oddity

    * Ingo Molnar <mingo@elte.hu> wrote:

    > > The reason is that the BUILD_LOCK_OPS macros in kernel/lockdep.c
    > > don't contain any of the *_acquire calls, while all of the _unlock
    > > functions contain a *_release call. Hence I get immediately
    > > unbalanced locks.
    >
    > hmmm ... that sounds like a bug. Weird - i recently ran
    > PREEMPT+SMP+LOCKDEP kernels and didnt notice this.

    ok, the reason i didnt find this problem is because this is fixed in my
    tree, but i didnt realize that it's a fix also for upstream ...

    The patch below is what is in my tree, but that needs separation from
    other changes. I'll distill a patch for upstream.

    Ingo

    NOT-Signed-off-by: Ingo Molnar <mingo@elte.hu>

    Index: linux/kernel/spinlock.c
    ===================================================================
    --- linux.orig/kernel/spinlock.c
    +++ linux/kernel/spinlock.c
    @@ -20,14 +20,14 @@
    * Generic declaration of the raw read_trylock() function,
    * architectures are supposed to optimize this:
    */
    -int __lockfunc generic__raw_read_trylock(raw_rwlock_t *lock)
    +int __lockfunc generic_raw_read_trylock(raw_rwlock_t *lock)
    {
    - __raw_read_lock(lock);
    + _raw_read_lock(lock);
    return 1;
    }
    -EXPORT_SYMBOL(generic__raw_read_trylock);
    +EXPORT_SYMBOL(generic_raw_read_trylock);

    -int __lockfunc _spin_trylock(spinlock_t *lock)
    +int __lockfunc __spin_trylock(raw_spinlock_t *lock)
    {
    preempt_disable();
    if (_raw_spin_trylock(lock)) {
    @@ -38,9 +38,46 @@ int __lockfunc _spin_trylock(spinlock_t
    preempt_enable();
    return 0;
    }
    -EXPORT_SYMBOL(_spin_trylock);
    +EXPORT_SYMBOL(__spin_trylock);
    +
    +int __lockfunc __spin_trylock_irq(raw_spinlock_t *lock)
    +{
    + local_irq_disable();
    + preempt_disable();
    +
    + if (_raw_spin_trylock(lock)) {
    + spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
    + return 1;
    + }
    +
    + __preempt_enable_no_resched();
    + local_irq_enable();
    + preempt_check_resched();
    +
    + return 0;
    +}
    +EXPORT_SYMBOL(__spin_trylock_irq);

    -int __lockfunc _read_trylock(rwlock_t *lock)
    +int __lockfunc __spin_trylock_irqsave(raw_spinlock_t *lock,
    + unsigned long *flags)
    +{
    + local_irq_save(*flags);
    + preempt_disable();
    +
    + if (_raw_spin_trylock(lock)) {
    + spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
    + return 1;
    + }
    +
    + __preempt_enable_no_resched();
    + local_irq_restore(*flags);
    + preempt_check_resched();
    +
    + return 0;
    +}
    +EXPORT_SYMBOL(__spin_trylock_irqsave);
    +
    +int __lockfunc __read_trylock(raw_rwlock_t *lock)
    {
    preempt_disable();
    if (_raw_read_trylock(lock)) {
    @@ -51,9 +88,9 @@ int __lockfunc _read_trylock(rwlock_t *l
    preempt_enable();
    return 0;
    }
    -EXPORT_SYMBOL(_read_trylock);
    +EXPORT_SYMBOL(__read_trylock);

    -int __lockfunc _write_trylock(rwlock_t *lock)
    +int __lockfunc __write_trylock(raw_rwlock_t *lock)
    {
    preempt_disable();
    if (_raw_write_trylock(lock)) {
    @@ -64,7 +101,7 @@ int __lockfunc _write_trylock(rwlock_t *
    preempt_enable();
    return 0;
    }
    -EXPORT_SYMBOL(_write_trylock);
    +EXPORT_SYMBOL(__write_trylock);

    /*
    * If lockdep is enabled then we use the non-preemption spin-ops
    @@ -72,17 +109,17 @@ EXPORT_SYMBOL(_write_trylock);
    * not re-enabled during lock-acquire (which the preempt-spin-ops do):
    */
    #if !defined(CONFIG_PREEMPT) || !defined(CONFIG_SMP) || \
    - defined(CONFIG_PROVE_LOCKING)
    + defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_PREEMPT_RT)

    -void __lockfunc _read_lock(rwlock_t *lock)
    +void __lockfunc __read_lock(raw_rwlock_t *lock)
    {
    preempt_disable();
    rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_read_lock(lock);
    }
    -EXPORT_SYMBOL(_read_lock);
    +EXPORT_SYMBOL(__read_lock);

    -unsigned long __lockfunc _spin_lock_irqsave(spinlock_t *lock)
    +unsigned long __lockfunc __spin_lock_irqsave(raw_spinlock_t *lock)
    {
    unsigned long flags;

    @@ -101,27 +138,27 @@ unsigned long __lockfunc _spin_lock_irqs
    #endif
    return flags;
    }
    -EXPORT_SYMBOL(_spin_lock_irqsave);
    +EXPORT_SYMBOL(__spin_lock_irqsave);

    -void __lockfunc _spin_lock_irq(spinlock_t *lock)
    +void __lockfunc __spin_lock_irq(raw_spinlock_t *lock)
    {
    local_irq_disable();
    preempt_disable();
    spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_spin_lock(lock);
    }
    -EXPORT_SYMBOL(_spin_lock_irq);
    +EXPORT_SYMBOL(__spin_lock_irq);

    -void __lockfunc _spin_lock_bh(spinlock_t *lock)
    +void __lockfunc __spin_lock_bh(raw_spinlock_t *lock)
    {
    local_bh_disable();
    preempt_disable();
    spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_spin_lock(lock);
    }
    -EXPORT_SYMBOL(_spin_lock_bh);
    +EXPORT_SYMBOL(__spin_lock_bh);

    -unsigned long __lockfunc _read_lock_irqsave(rwlock_t *lock)
    +unsigned long __lockfunc __read_lock_irqsave(raw_rwlock_t *lock)
    {
    unsigned long flags;

    @@ -131,27 +168,27 @@ unsigned long __lockfunc _read_lock_irqs
    _raw_read_lock(lock);
    return flags;
    }
    -EXPORT_SYMBOL(_read_lock_irqsave);
    +EXPORT_SYMBOL(__read_lock_irqsave);

    -void __lockfunc _read_lock_irq(rwlock_t *lock)
    +void __lockfunc __read_lock_irq(raw_rwlock_t *lock)
    {
    local_irq_disable();
    preempt_disable();
    rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_read_lock(lock);
    }
    -EXPORT_SYMBOL(_read_lock_irq);
    +EXPORT_SYMBOL(__read_lock_irq);

    -void __lockfunc _read_lock_bh(rwlock_t *lock)
    +void __lockfunc __read_lock_bh(raw_rwlock_t *lock)
    {
    local_bh_disable();
    preempt_disable();
    rwlock_acquire_read(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_read_lock(lock);
    }
    -EXPORT_SYMBOL(_read_lock_bh);
    +EXPORT_SYMBOL(__read_lock_bh);

    -unsigned long __lockfunc _write_lock_irqsave(rwlock_t *lock)
    +unsigned long __lockfunc __write_lock_irqsave(raw_rwlock_t *lock)
    {
    unsigned long flags;

    @@ -161,43 +198,43 @@ unsigned long __lockfunc _write_lock_irq
    _raw_write_lock(lock);
    return flags;
    }
    -EXPORT_SYMBOL(_write_lock_irqsave);
    +EXPORT_SYMBOL(__write_lock_irqsave);

    -void __lockfunc _write_lock_irq(rwlock_t *lock)
    +void __lockfunc __write_lock_irq(raw_rwlock_t *lock)
    {
    local_irq_disable();
    preempt_disable();
    rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_write_lock(lock);
    }
    -EXPORT_SYMBOL(_write_lock_irq);
    +EXPORT_SYMBOL(__write_lock_irq);

    -void __lockfunc _write_lock_bh(rwlock_t *lock)
    +void __lockfunc __write_lock_bh(raw_rwlock_t *lock)
    {
    local_bh_disable();
    preempt_disable();
    rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_write_lock(lock);
    }
    -EXPORT_SYMBOL(_write_lock_bh);
    +EXPORT_SYMBOL(__write_lock_bh);

    -void __lockfunc _spin_lock(spinlock_t *lock)
    +void __lockfunc __spin_lock(raw_spinlock_t *lock)
    {
    preempt_disable();
    spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_spin_lock(lock);
    }

    -EXPORT_SYMBOL(_spin_lock);
    +EXPORT_SYMBOL(__spin_lock);

    -void __lockfunc _write_lock(rwlock_t *lock)
    +void __lockfunc __write_lock(raw_rwlock_t *lock)
    {
    preempt_disable();
    rwlock_acquire(&lock->dep_map, 0, 0, _RET_IP_);
    _raw_write_lock(lock);
    }

    -EXPORT_SYMBOL(_write_lock);
    +EXPORT_SYMBOL(__write_lock);

    #else /* CONFIG_PREEMPT: */

    @@ -210,7 +247,7 @@ EXPORT_SYMBOL(_write_lock);
    */

    #define BUILD_LOCK_OPS(op, locktype) \
    -void __lockfunc _##op##_lock(locktype##_t *lock) \
    +void __lockfunc __##op##_lock(locktype##_t *lock) \
    { \
    for (;;) { \
    preempt_disable(); \
    @@ -220,15 +257,16 @@ void __lockfunc _##op##_lock(locktype##_
    \
    if (!(lock)->break_lock) \
    (lock)->break_lock = 1; \
    - while (!op##_can_lock(lock) && (lock)->break_lock) \
    + while (!__raw_##op##_can_lock(&(lock)->raw_lock) && \
    + (lock)->break_lock) \
    cpu_relax(); \
    } \
    (lock)->break_lock = 0; \
    } \
    \
    -EXPORT_SYMBOL(_##op##_lock); \
    +EXPORT_SYMBOL(__##op##_lock); \
    \
    -unsigned long __lockfunc _##op##_lock_irqsave(locktype##_t *lock) \
    +unsigned long __lockfunc __##op##_lock_irqsave(locktype##_t *lock) \
    { \
    unsigned long flags; \
    \
    @@ -242,23 +280,24 @@ unsigned long __lockfunc _##op##_lock_ir
    \
    if (!(lock)->break_lock) \
    (lock)->break_lock = 1; \
    - while (!op##_can_lock(lock) && (lock)->break_lock) \
    + while (!__raw_##op##_can_lock(&(lock)->raw_lock) && \
    + (lock)->break_lock) \
    cpu_relax(); \
    } \
    (lock)->break_lock = 0; \
    return flags; \
    } \
    \
    -EXPORT_SYMBOL(_##op##_lock_irqsave); \
    +EXPORT_SYMBOL(__##op##_lock_irqsave); \
    \
    -void __lockfunc _##op##_lock_irq(locktype##_t *lock) \
    +void __lockfunc __##op##_lock_irq(locktype##_t *lock) \
    { \
    - _##op##_lock_irqsave(lock); \
    + __##op##_lock_irqsave(lock); \
    } \
    \
    -EXPORT_SYMBOL(_##op##_lock_irq); \
    +EXPORT_SYMBOL(__##op##_lock_irq); \
    \
    -void __lockfunc _##op##_lock_bh(locktype##_t *lock) \
    +void __lockfunc __##op##_lock_bh(locktype##_t *lock) \
    { \
    unsigned long flags; \
    \
    @@ -267,147 +306,161 @@ void __lockfunc _##op##_lock_bh(locktype
    /* irq-disabling. We use the generic preemption-aware */ \
    /* function: */ \
    /**/ \
    - flags = _##op##_lock_irqsave(lock); \
    + flags = __##op##_lock_irqsave(lock); \
    local_bh_disable(); \
    local_irq_restore(flags); \
    } \
    \
    -EXPORT_SYMBOL(_##op##_lock_bh)
    +EXPORT_SYMBOL(__##op##_lock_bh)

    /*
    * Build preemption-friendly versions of the following
    * lock-spinning functions:
    *
    - * _[spin|read|write]_lock()
    - * _[spin|read|write]_lock_irq()
    - * _[spin|read|write]_lock_irqsave()
    - * _[spin|read|write]_lock_bh()
    + * __[spin|read|write]_lock()
    + * __[spin|read|write]_lock_irq()
    + * __[spin|read|write]_lock_irqsave()
    + * __[spin|read|write]_lock_bh()
    */
    -BUILD_LOCK_OPS(spin, spinlock);
    -BUILD_LOCK_OPS(read, rwlock);
    -BUILD_LOCK_OPS(write, rwlock);
    +BUILD_LOCK_OPS(spin, raw_spinlock);
    +BUILD_LOCK_OPS(read, raw_rwlock);
    +BUILD_LOCK_OPS(write, raw_rwlock);

    #endif /* CONFIG_PREEMPT */

    #ifdef CONFIG_DEBUG_LOCK_ALLOC

    -void __lockfunc _spin_lock_nested(spinlock_t *lock, int subclass)
    +void __lockfunc __spin_lock_nested(raw_spinlock_t *lock, int subclass)
    {
    preempt_disable();
    spin_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
    _raw_spin_lock(lock);
    }

    -EXPORT_SYMBOL(_spin_lock_nested);
    +EXPORT_SYMBOL(__spin_lock_nested);

    #endif

    -void __lockfunc _spin_unlock(spinlock_t *lock)
    +void __lockfunc __spin_unlock(raw_spinlock_t *lock)
    {
    spin_release(&lock->dep_map, 1, _RET_IP_);
    _raw_spin_unlock(lock);
    preempt_enable();
    }
    -EXPORT_SYMBOL(_spin_unlock);
    +EXPORT_SYMBOL(__spin_unlock);

    -void __lockfunc _write_unlock(rwlock_t *lock)
    +void __lockfunc __spin_unlock_no_resched(raw_spinlock_t *lock)
    +{
    + spin_release(&lock->dep_map, 1, _RET_IP_);
    + _raw_spin_unlock(lock);
    + __preempt_enable_no_resched();
    +}
    +/* not exported */
    +
    +void __lockfunc __write_unlock(raw_rwlock_t *lock)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_write_unlock(lock);
    preempt_enable();
    }
    -EXPORT_SYMBOL(_write_unlock);
    +EXPORT_SYMBOL(__write_unlock);

    -void __lockfunc _read_unlock(rwlock_t *lock)
    +void __lockfunc __read_unlock(raw_rwlock_t *lock)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_read_unlock(lock);
    preempt_enable();
    }
    -EXPORT_SYMBOL(_read_unlock);
    +EXPORT_SYMBOL(__read_unlock);

    -void __lockfunc _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
    +void __lockfunc __spin_unlock_irqrestore(raw_spinlock_t *lock, unsigned long flags)
    {
    spin_release(&lock->dep_map, 1, _RET_IP_);
    _raw_spin_unlock(lock);
    + __preempt_enable_no_resched();
    local_irq_restore(flags);
    - preempt_enable();
    + preempt_check_resched();
    }
    -EXPORT_SYMBOL(_spin_unlock_irqrestore);
    +EXPORT_SYMBOL(__spin_unlock_irqrestore);

    -void __lockfunc _spin_unlock_irq(spinlock_t *lock)
    +void __lockfunc __spin_unlock_irq(raw_spinlock_t *lock)
    {
    spin_release(&lock->dep_map, 1, _RET_IP_);
    _raw_spin_unlock(lock);
    + __preempt_enable_no_resched();
    local_irq_enable();
    - preempt_enable();
    + preempt_check_resched();
    }
    -EXPORT_SYMBOL(_spin_unlock_irq);
    +EXPORT_SYMBOL(__spin_unlock_irq);

    -void __lockfunc _spin_unlock_bh(spinlock_t *lock)
    +void __lockfunc __spin_unlock_bh(raw_spinlock_t *lock)
    {
    spin_release(&lock->dep_map, 1, _RET_IP_);
    _raw_spin_unlock(lock);
    - preempt_enable_no_resched();
    + __preempt_enable_no_resched();
    local_bh_enable_ip((unsigned long)__builtin_return_address(0));
    }
    -EXPORT_SYMBOL(_spin_unlock_bh);
    +EXPORT_SYMBOL(__spin_unlock_bh);

    -void __lockfunc _read_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
    +void __lockfunc __read_unlock_irqrestore(raw_rwlock_t *lock, unsigned long flags)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_read_unlock(lock);
    + __preempt_enable_no_resched();
    local_irq_restore(flags);
    - preempt_enable();
    + preempt_check_resched();
    }
    -EXPORT_SYMBOL(_read_unlock_irqrestore);
    +EXPORT_SYMBOL(__read_unlock_irqrestore);

    -void __lockfunc _read_unlock_irq(rwlock_t *lock)
    +void __lockfunc __read_unlock_irq(raw_rwlock_t *lock)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_read_unlock(lock);
    + __preempt_enable_no_resched();
    local_irq_enable();
    - preempt_enable();
    + preempt_check_resched();
    }
    -EXPORT_SYMBOL(_read_unlock_irq);
    +EXPORT_SYMBOL(__read_unlock_irq);

    -void __lockfunc _read_unlock_bh(rwlock_t *lock)
    +void __lockfunc __read_unlock_bh(raw_rwlock_t *lock)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_read_unlock(lock);
    - preempt_enable_no_resched();
    + __preempt_enable_no_resched();
    local_bh_enable_ip((unsigned long)__builtin_return_address(0));
    }
    -EXPORT_SYMBOL(_read_unlock_bh);
    +EXPORT_SYMBOL(__read_unlock_bh);

    -void __lockfunc _write_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
    +void __lockfunc __write_unlock_irqrestore(raw_rwlock_t *lock, unsigned long flags)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_write_unlock(lock);
    + __preempt_enable_no_resched();
    local_irq_restore(flags);
    - preempt_enable();
    + preempt_check_resched();
    }
    -EXPORT_SYMBOL(_write_unlock_irqrestore);
    +EXPORT_SYMBOL(__write_unlock_irqrestore);

    -void __lockfunc _write_unlock_irq(rwlock_t *lock)
    +void __lockfunc __write_unlock_irq(raw_rwlock_t *lock)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_write_unlock(lock);
    + __preempt_enable_no_resched();
    local_irq_enable();
    - preempt_enable();
    + preempt_check_resched();
    }
    -EXPORT_SYMBOL(_write_unlock_irq);
    +EXPORT_SYMBOL(__write_unlock_irq);

    -void __lockfunc _write_unlock_bh(rwlock_t *lock)
    +void __lockfunc __write_unlock_bh(raw_rwlock_t *lock)
    {
    rwlock_release(&lock->dep_map, 1, _RET_IP_);
    _raw_write_unlock(lock);
    - preempt_enable_no_resched();
    + __preempt_enable_no_resched();
    local_bh_enable_ip((unsigned long)__builtin_return_address(0));
    }
    -EXPORT_SYMBOL(_write_unlock_bh);
    +EXPORT_SYMBOL(__write_unlock_bh);

    -int __lockfunc _spin_trylock_bh(spinlock_t *lock)
    +int __lockfunc __spin_trylock_bh(raw_spinlock_t *lock)
    {
    local_bh_disable();
    preempt_disable();
    @@ -416,13 +469,13 @@ int __lockfunc _spin_trylock_bh(spinlock
    return 1;
    }

    - preempt_enable_no_resched();
    + __preempt_enable_no_resched();
    local_bh_enable_ip((unsigned long)__builtin_return_address(0));
    return 0;
    }
    -EXPORT_SYMBOL(_spin_trylock_bh);
    +EXPORT_SYMBOL(__spin_trylock_bh);

    -int in_lock_functions(unsigned long addr)
    +int in_lock_functions(unsigned long addr)
    {
    /* Linker adds these: start and end of __lockfunc functions */
    extern char __lock_text_start[], __lock_text_end[];
    -
    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/
    \
     
     \ /
      Last update: 2006-09-05 21:19    [W:0.054 / U:0.236 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site