* Pekka Enberg <[email protected]> wrote:
> On 7/13/06, Ingo Molnar <[email protected]> wrote:
> >mm/slab.c uses nested locking when dealing with 'off-slab'
> >caches, in that case it allocates the slab header from the
> >(on-slab) kmalloc caches. Teach the lock validator about
> >this by putting all on-slab caches into a separate class.
>
> What's "nested lock" btw? If I understood from the other patch, you're
> talking about ac->lock. Surely you can't take the same lock twice but
> it's perfectly legal to take lock as long as the ac instance is
> different...
yeah - there's some ambiguity of the term "nested lock". For the lock
validator it means "holding two instances of the same lock class". A
"lock class" is something like inode->i_mutex. (standalone static locks
like cache_chain_mutex form their own singleton lock class. See
Documentation/lockdep-design.txt for more details.)
"trying to lock the same lock instance twice" we call "recursive
locking", and that's a bug for everything except read_lock() on rwlocks.
There is no "recursive locking" in slab.c, but there is "nested
locking". For the case of "nested locking" the lock validator needs to
be taught of the relation between instances. Most of the cases the
relation is "static" and can thus be assigned build-time via the use of
separate lock-keys that "split up" a class into subclasses. In rare
cases the relation is dynamic (for example the VFS has such nesting
rules).
initially i annotated slab.c via dynamic nesting - but as Arjan has
correctly observed, most of the nested locking in slab.c is of static
type: we first take the off-slab lock, then the on-slab lock. [or we
only take an on-slab lock, if the cache is on-slab]
Note: nested locking annotations arent just done to "shut up" lockdep,
but rather to enable lockdep from now on to enforce this dependency
rule: if anywhere we take an off-slab lock after an on-slab lock it will
print a warning. That's why we go the trouble of identifying all the
nested locking cases instead of going the easy path of "shutting lockdep
off" (we could trivially ignore nesting within lockdep.c) - there were a
couple of locking bugs found already that were related to nested
locking.
btw., there's still one nested locking construct in slab.c that could
cause problems:
Call Trace:
[<ffffffff8020b0b9>] show_trace+0xaa/0x23d
[<ffffffff8020b261>] dump_stack+0x15/0x17
[<ffffffff802456c4>] __lock_acquire+0x127/0x9c4
[<ffffffff8024648b>] lock_acquire+0x4b/0x6a
[<ffffffff804aead3>] _spin_lock+0x25/0x31
[<ffffffff8027301a>] __drain_alien_cache+0x34/0x78
[<ffffffff80272b1f>] __cache_free+0xe8/0x215
[<ffffffff80272dc4>] slab_destroy+0x9e/0xc2
[<ffffffff80272ed3>] free_block+0xeb/0x135
[<ffffffff80273043>] __drain_alien_cache+0x5d/0x78
[<ffffffff80272b1f>] __cache_free+0xe8/0x215
[<ffffffff80273203>] kfree+0x8c/0xb0
what guarantees that while we are 'draining' another node's cache, that
other node does not drain our cache (and thus we'd deadlock)?
Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
