Re: [PATCH 2/3] lockdep: Introduce lock_acquire_might()

[Byungchul Park]

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.