Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1730593 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/3] lockdep: Introduce lock_acquire_might() Byungchul Park <byungchul.park@lge.com> - 2017-09-12 02:40 +0200
    Re: [PATCH 2/3] lockdep: Introduce lock_acquire_might() Byungchul Park <byungchul.park@lge.com> - 2017-09-26 03:00 +0200

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

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.

[toc] | [next] | [standalone]

#1739407

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web