Messages in this thread | | | Date | Mon, 5 Jun 2006 09:37:01 +0200 | From | Ingo Molnar <> | Subject | Re: 2.6.17-rc5-mm3: bad unlock ordering (reiser4?) |
| |
* 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.
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'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.
Ingo - 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/
| |