linux-kernel - Re: [PATCH 2/3] lockdep: Introduce lock_acquire

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] [day] [month] [year] [list]

Message-ID: <20170926005820.GO5994@X58A-UD3R>
Date:   Tue, 26 Sep 2017 09:58:21 +0900
From:   Byungchul Park <byungchul.park@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     tj@...nel.org, johannes.berg@...el.com, mingo@...nel.org,
        tglx@...utronix.de, oleg@...hat.com, david@...morbit.com,
        linux-kernel@...r.kernel.org, kernel-team@....com
Subject: Re: [PATCH 2/3] lockdep: Introduce lock_acquire_might()

On Tue, Sep 12, 2017 at 09:35:02AM +0900, Byungchul Park wrote:
> On Tue, Sep 05, 2017 at 09:22:39AM +0200, Peter Zijlstra wrote:
> > On Tue, Sep 05, 2017 at 11:29:13AM +0900, Byungchul Park wrote:
> > > From the point of view of crossrelease, we can never be aware of the
> > > release context in advance, until we get to the lock_release().
> > > However, this way we cannot report deadlocks occured at the time.
> > > 
> > > Sometimes, we want to report that kind of problems, taking a risk
> > > generating false dependencies e.g. lock_acquire()s in workqueue code,
> > > which inevitably generate false ones with all acquisitions in works.
> > > 
> > > It would be better to provide another primitive, lock_acquire_might()
> > > for that purpose so that lockdep internal can be aware of what users
> > > expect and get chances to enhance to avoid false ones.
> > > 
> > > The primitive should:
> > > 
> > >    1. work as if it's trylock, since links between lock_acquire_might()
> > >       and later ones are only meaningful. Remind this should be used to
> > >       do what crossrelease commit does, in advance.
> > > 
> > >    2. make acquisitions by lock_acquire_might() ignored on the commit.
> > > 
> > 
> > Shees, talk about ugly... Also might-lock has a different meaning.
> 
> OK. The description should be modified. I think I failed to explain what
> I intended. What do you think about the following, which I saied in
> another thread?
> 
>    If we use real acquisitions instead of 'might' for that speculative
>    purpose as the workqueue code currently does:
> 
>    (1) All locks used in every work->func() generate false dependencies
>    with 'work' lockdep_map and 'wq' lockdep_map, while any flush works
>    are not involved. But, it's inevitable.
> 
>    (2) Moreover, it also generates more false ones between the real
>    acquisitions. Of course, it can be avoidable if we force to use only
>    recursive-read for that purpose, which is not true for now.
> 
>    (3) Moreover, it also generates more false ones between holding locks
>    and the real ones. Of course, the workqueue code is not the case for
>    now.
> 
>    (4) Moreover, it also generates more false ones between the real ones
>    and a crosslock on commit, once crossrelease is able to work for
>    recursive-read things.

Here, I want to discuss 'might' thing. IMHO, new primitive should be
used instead of read-recursive on speculative acquisitons used in e.g.
workqueue or smp/hotplug.