Messages in this thread | | | From | Alexander Zarochentsev <> | Subject | Re: 2.6.17-rc5-mm3: bad unlock ordering (reiser4?) | Date | Mon, 5 Jun 2006 15:22:13 +0400 |
| |
Hello,
On Monday 05 June 2006 11:37, Ingo Molnar wrote: > * Ingo Molnar <mingo@elte.hu> wrote: > > +++ linux/fs/reiser4/txnmgr.h > > @@ -613,7 +613,7 @@ static inline void spin_unlock_txnmgr(tx > > LOCK_CNT_DEC(spin_locked_txnmgr); > > LOCK_CNT_DEC(spin_locked); > > > > - spin_unlock(&(mgr->tmgr_lock)); > > + spin_unlock_non_nested(&(mgr->tmgr_lock)); > > } > > > > typedef enum { > > Btw., this particular annotation also documents a locking/scalability > inefficiency. mgr->tmgr_lock is a "global" lock (per superblock it > seems), while atom->alock is a more "finegrained" lock. > > Typical usage: tmgr_lock is used a 'master lock', it's taken, then > atom->alock is taken, and then ->tmgr_lock is released. Then code > runs under atom->alock, and atom->alock is released finally.
> The scalability problem with such 'master locks' is that they pretty > much control scalability, so the scalability advantage of the finer > grained lock is reduced (often eliminated). Since access to the finer > grained lock goes via the master lock, the master lock cacheline will > bounce from CPU to CPU.
please note that the master lock is taken by try_caputure only if new atom is created. It is likely than current thread has an atom already or the block already captured.
> A much more scalable design is to get to the finer grained lock in > some read-mostly, lockless way, and then take it. This often > necessiates the utilization of RCU, but it's well worth it.
There was a code to measure lock contention for reiser4 locks which showed that the tmgr lock was contented less than atom and jnode spin locks were.
> There's other kernel code that has been annotated for similar reasons > - e.g. the netfilter code makes frequent use of master-locks.
> All in one, it's a good idea to document such locking constructs via > the _non_nested() annotation. Often they can be eliminated altogether > and the code improves. It's not a maintainance problem either, > because right now there are only 42 such annotations, out of 46,000+ > locking API uses covered by the lock validator.
I think the txnh lock and the tmgr lock are _non_nested. And, there is a place where two atom locks are taken in deadlock-free order w/o always keeping correct order of unlocking. The latest thing can be made lock-validator-friendly.
> Ingo
Best, Alex.
- 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/
| |