linux-kernel - Re: [PATCH v4 05/15] lockdep: Make check_prev_add can use a separate stack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170118151053.GF6500@twins.programming.kicks-ass.net>
Date:   Wed, 18 Jan 2017 16:10:53 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Byungchul Park <byungchul.park@....com>
Cc:     mingo@...nel.org, tglx@...utronix.de, walken@...gle.com,
        boqun.feng@...il.com, kirill@...temov.name,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        npiggin@...il.com
Subject: Re: [PATCH v4 05/15] lockdep: Make check_prev_add can use a separate
 stack_trace

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