Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Rik van Riel
On 03/29/2013 09:08 AM, Michel Lespinasse wrote: That is where I had it initially. I may have gotten too clever and worked on keeping more accesses read-only. If you want, I can move it back here and re-submit the patch :) I think I would prefer that - I feel having it upper serves little

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Michel Lespinasse
On Fri, Mar 29, 2013 at 2:57 AM, Peter Zijlstra wrote: > On Thu, 2013-03-28 at 19:50 -0700, Michel Lespinasse wrote: >> So, there are a few things I don't like about spin_unlock_wait(): >> >> 1- From a lock ordering point of view, it is strictly equivalent to >> taking the lock and then releasing

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Michel Lespinasse
On Fri, Mar 29, 2013 at 5:07 AM, Rik van Riel wrote: > On 03/28/2013 10:50 PM, Michel Lespinasse wrote: >> On Thu, Mar 28, 2013 at 1:23 PM, Rik van Riel wrote: >>> if (unlikely(sma->complex_count)) { >>> spin_unlock(>lock); >>> -

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Rik van Riel
On 03/28/2013 10:50 PM, Michel Lespinasse wrote: On Thu, Mar 28, 2013 at 1:23 PM, Rik van Riel wrote: Subject: [PATCH -mm -next] ipc,sem: change locking scheme to make lockdep happy Unfortunately the locking scheme originally proposed has false positives with lockdep. This can be fixed by

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Peter Zijlstra
On Thu, 2013-03-28 at 19:50 -0700, Michel Lespinasse wrote: > So, there are a few things I don't like about spin_unlock_wait(): > > 1- From a lock ordering point of view, it is strictly equivalent to > taking the lock and then releasing it - and yet, lockdep won't catch > any deadlocks that

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Peter Zijlstra
On Thu, 2013-03-28 at 19:50 -0700, Michel Lespinasse wrote: So, there are a few things I don't like about spin_unlock_wait(): 1- From a lock ordering point of view, it is strictly equivalent to taking the lock and then releasing it - and yet, lockdep won't catch any deadlocks that involve

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Rik van Riel
On 03/28/2013 10:50 PM, Michel Lespinasse wrote: On Thu, Mar 28, 2013 at 1:23 PM, Rik van Riel r...@surriel.com wrote: Subject: [PATCH -mm -next] ipc,sem: change locking scheme to make lockdep happy Unfortunately the locking scheme originally proposed has false positives with lockdep. This

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Michel Lespinasse
On Fri, Mar 29, 2013 at 5:07 AM, Rik van Riel r...@surriel.com wrote: On 03/28/2013 10:50 PM, Michel Lespinasse wrote: On Thu, Mar 28, 2013 at 1:23 PM, Rik van Riel r...@surriel.com wrote: if (unlikely(sma-complex_count)) { spin_unlock(sem-lock); -

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Michel Lespinasse
On Fri, Mar 29, 2013 at 2:57 AM, Peter Zijlstra pet...@infradead.org wrote: On Thu, 2013-03-28 at 19:50 -0700, Michel Lespinasse wrote: So, there are a few things I don't like about spin_unlock_wait(): 1- From a lock ordering point of view, it is strictly equivalent to taking the lock and

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-29 Thread Rik van Riel
On 03/29/2013 09:08 AM, Michel Lespinasse wrote: That is where I had it initially. I may have gotten too clever and worked on keeping more accesses read-only. If you want, I can move it back here and re-submit the patch :) I think I would prefer that - I feel having it upper serves little

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-28 Thread Michel Lespinasse
On Thu, Mar 28, 2013 at 1:23 PM, Rik van Riel wrote: > Subject: [PATCH -mm -next] ipc,sem: change locking scheme to make lockdep > happy > > Unfortunately the locking scheme originally proposed has false positives > with lockdep. This can be fixed by changing the code to only ever take > one

[PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-28 Thread Rik van Riel
On Wed, 27 Mar 2013 09:42:30 +0100 Peter Zijlstra wrote: > I since realized there's an ordering problem with ->global_locked, we > need to use spin_is_locked() or somesuch. > > Two competing sma_lock() operations will screw over the separate > variable. I created a worse version of the stress

[PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-28 Thread Rik van Riel
On Wed, 27 Mar 2013 09:42:30 +0100 Peter Zijlstra pet...@infradead.org wrote: I since realized there's an ordering problem with -global_locked, we need to use spin_is_locked() or somesuch. Two competing sma_lock() operations will screw over the separate variable. I created a worse version

Re: [PATCH v2 -mm -next] ipc,sem: fix lockdep false positive

2013-03-28 Thread Michel Lespinasse
On Thu, Mar 28, 2013 at 1:23 PM, Rik van Riel r...@surriel.com wrote: Subject: [PATCH -mm -next] ipc,sem: change locking scheme to make lockdep happy Unfortunately the locking scheme originally proposed has false positives with lockdep. This can be fixed by changing the code to only ever