Re: spinlock assertion macros

From: Oliver Xymoron (oxymoron@waste.org)
Date: Thu Jul 11 2002 - 22:22:38 EST


On Thu, 11 Jul 2002, Sandy Harris wrote:

> Oliver Xymoron wrote:
> >
> > On Thu, 11 Jul 2002, Daniel Phillips wrote:
> >
> > > I was thinking of something as simple as:
> > >
> > > #define spin_assert_locked(LOCK) BUG_ON(!spin_is_locked(LOCK))
> > >
> > > but in truth I'd be happy regardless of the internal implementation. A note
> > > on names: Linus likes to shout the names of his BUG macros. I've never been
> > > one for shouting, but it's not my kernel, and anyway, I'm happy he now likes
> > > asserts. I bet he'd like it more spelled like this though:
> > >
> > > MUST_HOLD(&lock);
> >
> > I prefer that form too.
>
> Is it worth adding MUST_NOT_HOLD(&lock) in an attempt to catch potential
> deadlocks?

No, you'd rather do that inside the debugging version of spinlock itself.
I see MUST_HOLD happening inside helper functions that presume a lock is
in effect but don't do any on their own to ensure integrity, while
MUST_NOT_HOLD is a matter of forcing ordering and avoiding deadlocks.
Locking order is larger than functions and should be documented at the
point of declaration of the locks.

You can do lock order checking in the spinlock debugging code something
like this:

struct spinlock
{
        atomic_t val;
#ifdef SPINLOCK_DEBUG
        void *eip;
        struct spinlock *previous;
        struct spinlock *ordering;
};

DECLARE_SPINLOCK(a, 0); /* a is outermost for this group */
DECLARE_SPINLOCK(b, &a); /* b must nest inside a */
DECLARE_SPINLOCK(c, &b);

Each time you take a spinlock, record the eip, stick the address of the
current lock in the task struct, and stick the outer lock in previous.
Then see if the current lock appears anywhere in the previous lock's
ordering chain (it shouldn't). This may be a bit overkill though.

It might make sense to have a MAY_SLEEP assert in a few key places (high
up in the chains of things that rarely sleep or for which the call path to
sleeping isn't obvious).

-- 
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."

- 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/



This archive was generated by hypermail 2b29 : Mon Jul 15 2002 - 22:00:21 EST