Re: [PATCH v4 05/15] lockdep: Make check_prev_add can use a separate stack_trace

[Peter Zijlstra]

On Wed, Jan 18, 2017 at 11:04:32AM +0900, Byungchul Park wrote:
> On Tue, Jan 17, 2017 at 04:54:31PM +0100, Peter Zijlstra wrote:
> > On Fri, Jan 13, 2017 at 07:11:43PM +0900, Byungchul Park wrote:
> > > What do you think about the following patches doing it?
> > 
> > I was more thinking about something like so...
> > 
> > Also, I think I want to muck with struct stack_trace; the members:
> > max_nr_entries and skip are input arguments to save_stack_trace() and
> > bloat the structure for no reason.
> 
> With your approach, save_trace() must be called whenever check_prevs_add()
> is called, which might be unnecessary.

True.. but since we hold the graph_lock this is a slow path anyway, so I
didn't care much.

Then again, I forgot to clean up in a bunch of paths.

> Frankly speaking, I think what I proposed resolved it neatly. Don't you
> think so?

My initial reaction was to your patches being radically different to
what I had proposed. But after fixing mine I don't particularly like
either one of them.

Also, I think yours has a hole in, you check nr_stack_trace_entries
against an older copy to check we did save_stack(), this is not accurate
as check_prev_add() can drop graph_lock in the verbose case and then
someone else could have done save_stack().


Let me see if I can find something simpler..