Re: [BUG] perf: hard lockup when using perf-sched

[Peter Zijlstra]

On Thu, 2010-03-25 at 10:27 +0100, Mike Galbraith wrote:
> On Thu, 2010-03-25 at 16:04 +0800, Li Zefan wrote:
> > Mike Galbraith wrote:
> > > On Wed, 2010-03-24 at 08:32 +0100, Mike Galbraith wrote:
> > > 
> > >> I just saw this, hunted down your testcase and tried it here.  Looks
> > >> like perf_output_lock() wedged box.
> > > 
> > > (turns on frame pointers, and adds noinline)
> > > 
> > 
> > Thanks! Then who's going to fix this...
> 
> Well, that kinda depends on whether I figure out how the heck it's all
> supposed to work before somebody else whacks it or not.
> 
> ATM, I've instrumented, know _what's_ happening, but find myself saying
> "wtf?" a lot, especially wrt handle->locked.  The act of attempting to
> lock a handle declares it unlocked, turning perf_output_unlock() into a
> noop, which looks a bit strange.  We're spinning on those "unlocked"
> locks, all left genuinely locked by one CPU.  I just whacked the thing,
> and am very likely about to see in yet another trace.
> 
> Locking is hard, "curious construct" locking is even harder :)

Yeah, that perf_output_{un,}lock() stuff is a pain.. 

handle->locked should indicate if we're the outer most context

the problem this stuff is solving is that of publishing the data head
pointer, suppose two concurrent writers:

 head = 0;

 A: reserve N bytes
    write content
    commit

 B: reserve M bytes
    write content
    commit

Now, if B happens during A, and B completes before A is done, B's commit
would expose a head pointer of N+M bytes, even though the first N aren't
completely written yet.

So the trick is to only update the head pointer once everybody is done
writing. You can model this as a recursion problem and only update once
the recursion count hits 0 again.

However, with multiple cpus and NMIs and a busy enough environment this
never needs to happen, which would be bad, since we then cannot make
progress.

Hence the curious construct which basically serializes per cpu. Since
each cpu can only have a limited nesting context, and any one cpu is
bound to make progress (unless of course there is another bug that
prevents that) we'll post regular updates to the head pointer.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/