Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 29, 2017 at 05:52:05PM +0200, Oleg Nesterov wrote: > The problem is that start_flush_work() does not do acquire/release > unconditionally, it does this only if it is going to wait, and I am not > sure this is right... Right, I think you're right, we can move it earlier, once we have

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 29, 2017 at 05:52:05PM +0200, Oleg Nesterov wrote: > The problem is that start_flush_work() does not do acquire/release > unconditionally, it does this only if it is going to wait, and I am not > sure this is right... Right, I think you're right, we can move it earlier, once we have

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Peter, sorry for delay, didn't have a chance to return to this discussion... On 08/23, Peter Zijlstra wrote: > > > > It was added by Oleg in commit: > > > > > > a67da70dc095 ("workqueues: lockdep annotations for flush_work()") > > > > No, these annotations were moved later into start_flush,

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Peter, sorry for delay, didn't have a chance to return to this discussion... On 08/23, Peter Zijlstra wrote: > > > > It was added by Oleg in commit: > > > > > > a67da70dc095 ("workqueues: lockdep annotations for flush_work()") > > > > No, these annotations were moved later into start_flush,

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 05:11:01PM +0900, Byungchul Park wrote: > On Thu, Aug 24, 2017 at 04:37:13PM +0900, Byungchul Park wrote: > > On Thu, Aug 24, 2017 at 03:11:53PM +0900, Byungchul Park wrote: > > > On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > > > > Those are fine and are

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 05:11:01PM +0900, Byungchul Park wrote: > On Thu, Aug 24, 2017 at 04:37:13PM +0900, Byungchul Park wrote: > > On Thu, Aug 24, 2017 at 03:11:53PM +0900, Byungchul Park wrote: > > > On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > > > > Those are fine and are

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 04:37:13PM +0900, Byungchul Park wrote: > On Thu, Aug 24, 2017 at 03:11:53PM +0900, Byungchul Park wrote: > > On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > > > Those are fine and are indeed the flush_work() vs work inversion. > > > > > > The two

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 04:37:13PM +0900, Byungchul Park wrote: > On Thu, Aug 24, 2017 at 03:11:53PM +0900, Byungchul Park wrote: > > On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > > > Those are fine and are indeed the flush_work() vs work inversion. > > > > > > The two

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 03:11:53PM +0900, Byungchul Park wrote: > On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > > Those are fine and are indeed the flush_work() vs work inversion. > > > > The two straight forward annotations are: > > > > flush_work(work)

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 03:11:53PM +0900, Byungchul Park wrote: > On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > > Those are fine and are indeed the flush_work() vs work inversion. > > > > The two straight forward annotations are: > > > > flush_work(work)

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 11:02:36AM +0900, Byungchul Park wrote: > On Wed, Aug 23, 2017 at 12:20:48PM +0200, Peter Zijlstra wrote: > > On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > > > > > We have to detect dependecies if it exists, even in the following > > > > > case: >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Thu, Aug 24, 2017 at 11:02:36AM +0900, Byungchul Park wrote: > On Wed, Aug 23, 2017 at 12:20:48PM +0200, Peter Zijlstra wrote: > > On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > > > > > We have to detect dependecies if it exists, even in the following > > > > > case: >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > Those are fine and are indeed the flush_work() vs work inversion. > > The two straight forward annotations are: > > flush_work(work) process_one_work(wq, work) > A(work) A(work) > R(work)

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 07:47:14PM +0200, Peter Zijlstra wrote: > Those are fine and are indeed the flush_work() vs work inversion. > > The two straight forward annotations are: > > flush_work(work) process_one_work(wq, work) > A(work) A(work) > R(work)

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 12:26:52PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:43:23AM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > > > > So I think we'll end up hitting a lockdep deficiency and not trigger the > > > splat on

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 12:26:52PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:43:23AM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > > > > So I think we'll end up hitting a lockdep deficiency and not trigger the > > > splat on

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 12:46:48PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > > Currently, we do the following in process_one_work(), > > > > lockdep_map_acquire for a

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 12:46:48PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > > Currently, we do the following in process_one_work(), > > > > lockdep_map_acquire for a

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 12:20:48PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > > > We have to detect dependecies if it exists, even in the following case: > > > > > > > > oooiii. > > > > |<- range for commit

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 12:20:48PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > > > We have to detect dependecies if it exists, even in the following case: > > > > > > > > oooiii. > > > > |<- range for commit

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 06:39:03PM +0200, Oleg Nesterov wrote: > Peter, I am all confused and I am still trying to understand your email. > In particular, because I no longer understand the lockdep annotations in > workqueue.c, it turns out I forgot everything... Yeah, that happens :/ > On

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 06:39:03PM +0200, Oleg Nesterov wrote: > Peter, I am all confused and I am still trying to understand your email. > In particular, because I no longer understand the lockdep annotations in > workqueue.c, it turns out I forgot everything... Yeah, that happens :/ > On

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Peter, I am all confused and I am still trying to understand your email. In particular, because I no longer understand the lockdep annotations in workqueue.c, it turns out I forgot everything... On 08/22, Peter Zijlstra wrote: > > I am however slightly puzzled by the need of flush_work() to take

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Peter, I am all confused and I am still trying to understand your email. In particular, because I no longer understand the lockdep annotations in workqueue.c, it turns out I forgot everything... On 08/22, Peter Zijlstra wrote: > > I am however slightly puzzled by the need of flush_work() to take

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > Currently, we do the following in process_one_work(), > > lockdep_map_acquire for a workqueue > lockdep_map_acquire for a work > > But IMHO it should be, > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > Currently, we do the following in process_one_work(), > > lockdep_map_acquire for a workqueue > lockdep_map_acquire for a work > > But IMHO it should be, > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:43:23AM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > > So I think we'll end up hitting a lockdep deficiency and not trigger the > > splat on flush_work(), see also: > > > > https://lwn.net/Articles/332801/ > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:43:23AM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > > So I think we'll end up hitting a lockdep deficiency and not trigger the > > splat on flush_work(), see also: > > > > https://lwn.net/Articles/332801/ > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > We have to detect dependecies if it exists, even in the following case: > > > > > > oooiii. > > > |<- range for commit ->| > > > > > > where > > > o: acquisition outside of each work, > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > We have to detect dependecies if it exists, even in the following case: > > > > > > oooiii. > > > |<- range for commit ->| > > > > > > where > > > o: acquisition outside of each work, > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:43:23AM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > > On Tue, Aug 22, 2017 at 12:08:40PM +0200, Peter Zijlstra wrote: > > > > > > > > I meant: > > > > > > > > > > > > mutex_lock() > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:43:23AM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > > On Tue, Aug 22, 2017 at 12:08:40PM +0200, Peter Zijlstra wrote: > > > > > > > > I meant: > > > > > > > > > > > > mutex_lock() > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > > > IIUC, that's a problem because XFS does this all over the place. > > > Let me pull the trace apart in reverse order to explain why XFS is > > > going to throw endless false positives on this: > > > > Yeah, and I agree that's

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > > > IIUC, that's a problem because XFS does this all over the place. > > > Let me pull the trace apart in reverse order to explain why XFS is > > > going to throw endless false positives on this: > > > > Yeah, and I agree that's

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > We have to detect dependecies if it exists, even in the following case: > > > > > > oooiii. > > > |<- range for commit ->| > > > > > > where > > > o: acquisition outside of each work, > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Wed, Aug 23, 2017 at 11:12:17AM +0900, Byungchul Park wrote: > > > We have to detect dependecies if it exists, even in the following case: > > > > > > oooiii. > > > |<- range for commit ->| > > > > > > where > > > o: acquisition outside of each work, > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 12:08:40PM +0200, Peter Zijlstra wrote: > > > > > > I meant: > > > > > > > > > > mutex_lock() > > > > > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 12:08:40PM +0200, Peter Zijlstra wrote: > > > > > > I meant: > > > > > > > > > > mutex_lock() > > > > > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > > Let me get this straight, first. If we: > > > > 1. take a lock in a work queue context;

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > > Let me get this straight, first. If we: > > > > 1. take a lock in a work queue context;

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:37:03AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 06:22:36PM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > > So I did the below little hack, which basically wipes the entire lock > > > history when we

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:37:03AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 06:22:36PM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > > So I did the below little hack, which basically wipes the entire lock > > > history when we

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > > > > > Booting the very latest -tip on my test machine gets me the below splat. > > > > Dave, TJ, FYI, lockdep grew annotations for completions; it remembers > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > > > > > Booting the very latest -tip on my test machine gets me the below splat. > > > > Dave, TJ, FYI, lockdep grew annotations for completions; it remembers > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > > Even if I ignore the fact that buffer completions are run on > > different workqueues, there seems to be a bigger problem with this > > sort of completion checking. >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > > Even if I ignore the fact that buffer completions are run on > > different workqueues, there seems to be a bigger problem with this > > sort of completion checking. >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 05:59:06PM +0200, Oleg Nesterov wrote: > Peter, I'll read your email tomorrow, just one note... > > On 08/22, Peter Zijlstra wrote: > > > > Also^2, TJ, what's the purpose of using atomic_long_t for work->data? > > All it ever seems to do is atomic_long_read() and

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 05:59:06PM +0200, Oleg Nesterov wrote: > Peter, I'll read your email tomorrow, just one note... > > On 08/22, Peter Zijlstra wrote: > > > > Also^2, TJ, what's the purpose of using atomic_long_t for work->data? > > All it ever seems to do is atomic_long_read() and

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Peter, I'll read your email tomorrow, just one note... On 08/22, Peter Zijlstra wrote: > > Also^2, TJ, what's the purpose of using atomic_long_t for work->data? > All it ever seems to do is atomic_long_read() and atomic_long_set(), plust set/clear bit, for example

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Peter, I'll read your email tomorrow, just one note... On 08/22, Peter Zijlstra wrote: > > Also^2, TJ, what's the purpose of using atomic_long_t for work->data? > All it ever seems to do is atomic_long_read() and atomic_long_set(), plust set/clear bit, for example

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 04:46:02PM +0200, Peter Zijlstra wrote: > I am however slightly puzzled by the need of flush_work() to take Q, > what deadlock potential is there? > > Task: Work-W1:Work-W2: > > M(A) AR(Q) AR(Q) > flush_work(W1)

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 04:46:02PM +0200, Peter Zijlstra wrote: > I am however slightly puzzled by the need of flush_work() to take Q, > what deadlock potential is there? > > Task: Work-W1:Work-W2: > > M(A) AR(Q) AR(Q) > flush_work(W1)

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > Now, this means I also have to consider the existing > lock_map_acquire_read() users and if they really wanted to be recursive > or not. When I change lock_map_acquire_read() to use > lock_acquire_shared() this annotation no longer

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:49:22PM +0200, Peter Zijlstra wrote: > Now, this means I also have to consider the existing > lock_map_acquire_read() users and if they really wanted to be recursive > or not. When I change lock_map_acquire_read() to use > lock_acquire_shared() this annotation no longer

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 12:08:40PM +0200, Peter Zijlstra wrote: > > > > I meant: > > > > > > > > mutex_lock() > > > > > > > > lockdep_map_acquire_read() > > > > mutex_lock() > > > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 12:08:40PM +0200, Peter Zijlstra wrote: > > > > I meant: > > > > > > > > mutex_lock() > > > > > > > > lockdep_map_acquire_read() > > > > mutex_lock() > > > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 06:33:37PM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 11:21:41AM +0200, Peter Zijlstra wrote: > > On Tue, Aug 22, 2017 at 05:51:00PM +0900, Byungchul Park wrote: > > > On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > > > > That wouldn't work.

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 06:33:37PM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 11:21:41AM +0200, Peter Zijlstra wrote: > > On Tue, Aug 22, 2017 at 05:51:00PM +0900, Byungchul Park wrote: > > > On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > > > > That wouldn't work.

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

TJ, afaict the workqueue stuff doesn't in fact use signals, right? Should we do the below? That is, the only reason it appears to use INTERRUPTIBLE is to avoid increasing the loadavg, and we've introduced IDLE for that a while back: 80ed87c8a9ca ("sched/wait: Introduce TASK_NOLOAD and

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

TJ, afaict the workqueue stuff doesn't in fact use signals, right? Should we do the below? That is, the only reason it appears to use INTERRUPTIBLE is to avoid increasing the loadavg, and we've introduced IDLE for that a while back: 80ed87c8a9ca ("sched/wait: Introduce TASK_NOLOAD and

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 06:22:36PM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > So I did the below little hack, which basically wipes the entire lock > > history when we start a work and thereby disregards/looses the > > dependency on the work

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 06:22:36PM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > So I did the below little hack, which basically wipes the entire lock > > history when we start a work and thereby disregards/looses the > > dependency on the work

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:21:41AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 05:51:00PM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > > > That wouldn't work. That annotation is to help find deadlocks like: > > > > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:21:41AM +0200, Peter Zijlstra wrote: > On Tue, Aug 22, 2017 at 05:51:00PM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > > > That wouldn't work. That annotation is to help find deadlocks like: > > > > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > So I did the below little hack, which basically wipes the entire lock > history when we start a work and thereby disregards/looses the > dependency on the work 'lock'. > > It makes my test box able to boot and build a kernel on

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > So I did the below little hack, which basically wipes the entire lock > history when we start a work and thereby disregards/looses the > dependency on the work 'lock'. > > It makes my test box able to boot and build a kernel on

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 05:51:00PM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > > That wouldn't work. That annotation is to help find deadlocks like: > > > > > > mutex_lock() > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 05:51:00PM +0900, Byungchul Park wrote: > On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > > That wouldn't work. That annotation is to help find deadlocks like: > > > > > > mutex_lock() > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > Let me get this straight, first. If we: > > 1. take a lock in a work queue context; and > 2. in a separate context, hold that lock while we wait for a >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 03:46:03PM +1000, Dave Chinner wrote: > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > Let me get this straight, first. If we: > > 1. take a lock in a work queue context; and > 2. in a separate context, hold that lock while we wait for a >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > That wouldn't work. That annotation is to help find deadlocks like: > > > mutex_lock() > > mutex_lock() > > flush_work() > I meant: mutex_lock()

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 09:52:38AM +0200, Peter Zijlstra wrote: > That wouldn't work. That annotation is to help find deadlocks like: > > > mutex_lock() > > mutex_lock() > > flush_work() > I meant: mutex_lock()

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 02:14:38PM +0900, Byungchul Park wrote: > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > Now given the above observance rule and the fact that the below report > > is from the complete, the thing that happened appears to be: > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Tue, Aug 22, 2017 at 02:14:38PM +0900, Byungchul Park wrote: > On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > Now given the above observance rule and the fact that the below report > > is from the complete, the thing that happened appears to be: > > > > > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > > Booting the very latest -tip on my test machine gets me the below splat. > > Dave, TJ, FYI, lockdep grew annotations for completions; it remembers > which locks were taken before we complete() and checks none of those are >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > > > Booting the very latest -tip on my test machine gets me the below splat. > > Dave, TJ, FYI, lockdep grew annotations for completions; it remembers > which locks were taken before we complete() and checks none of those are >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > Now given the above observance rule and the fact that the below report > is from the complete, the thing that happened appears to be: > > > lockdep_map_acquire(>lockdep_map) > down_write() > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

On Mon, Aug 21, 2017 at 05:46:00PM +0200, Peter Zijlstra wrote: > Now given the above observance rule and the fact that the below report > is from the complete, the thing that happened appears to be: > > > lockdep_map_acquire(>lockdep_map) > down_write() > >

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Booting the very latest -tip on my test machine gets me the below splat. Dave, TJ, FYI, lockdep grew annotations for completions; it remembers which locks were taken before we complete() and checks none of those are held while we wait_for_completion(). That is, something like:

Re: [PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Booting the very latest -tip on my test machine gets me the below splat. Dave, TJ, FYI, lockdep grew annotations for completions; it remembers which locks were taken before we complete() and checks none of those are held while we wait_for_completion(). That is, something like:

[PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Crossrelease added LOCKDEP_CROSSRELEASE and LOCKDEP_COMPLETE configs. It makes little sense to enable them when PROVE_LOCKING is disabled. Make them non-interative option and all part of PROVE_LOCKING. Signed-off-by: Byungchul Park --- lib/Kconfig.debug | 7 ++- 1

[PATCH v3 1/3] lockdep: Make LOCKDEP_CROSSRELEASE configs all part of PROVE_LOCKING

Crossrelease added LOCKDEP_CROSSRELEASE and LOCKDEP_COMPLETE configs. It makes little sense to enable them when PROVE_LOCKING is disabled. Make them non-interative option and all part of PROVE_LOCKING. Signed-off-by: Byungchul Park --- lib/Kconfig.debug | 7 ++- 1 file changed, 2