Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Waiman Long
On 10/02/2013 03:30 PM, Jason Low wrote: On Wed, Oct 2, 2013 at 12:19 PM, Waiman Long wrote: On 09/26/2013 06:42 PM, Jason Low wrote: On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: Okay, that would makes sense for consistency because we always first set node->lock = 0 at the top of the

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Waiman Long
On 10/02/2013 02:43 PM, Tim Chen wrote: On Tue, 2013-10-01 at 21:25 -0400, Waiman Long wrote: If the lock and unlock functions are done right, there should be no overlap of critical section. So it is job of the lock/unlock functions to make sure that critical section code won't leak out. There

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Jason Low
On Wed, Oct 2, 2013 at 12:19 PM, Waiman Long wrote: > On 09/26/2013 06:42 PM, Jason Low wrote: >> >> On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: >>> >>> Okay, that would makes sense for consistency because we always >>> first set node->lock = 0 at the top of the function. >>> >>> If we

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Waiman Long
On 09/26/2013 06:42 PM, Jason Low wrote: On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: Okay, that would makes sense for consistency because we always first set node->lock = 0 at the top of the function. If we prefer to optimize this a bit though, perhaps we can first move the node->lock =

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Tim Chen
On Tue, 2013-10-01 at 21:25 -0400, Waiman Long wrote: > On 10/01/2013 05:16 PM, Tim Chen wrote: > > On Tue, 2013-10-01 at 16:01 -0400, Waiman Long wrote: > >>> > >>> The cpu could still be executing out of order load instruction from the > >>> critical section before checking node->locked?

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Tim Chen
On Tue, 2013-10-01 at 21:25 -0400, Waiman Long wrote: On 10/01/2013 05:16 PM, Tim Chen wrote: On Tue, 2013-10-01 at 16:01 -0400, Waiman Long wrote: The cpu could still be executing out of order load instruction from the critical section before checking node-locked? Probably smp_mb() is

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Waiman Long
On 09/26/2013 06:42 PM, Jason Low wrote: On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: Okay, that would makes sense for consistency because we always first set node-lock = 0 at the top of the function. If we prefer to optimize this a bit though, perhaps we can first move the node-lock = 0

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Jason Low
On Wed, Oct 2, 2013 at 12:19 PM, Waiman Long waiman.l...@hp.com wrote: On 09/26/2013 06:42 PM, Jason Low wrote: On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: Okay, that would makes sense for consistency because we always first set node-lock = 0 at the top of the function. If we prefer

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Waiman Long
On 10/02/2013 02:43 PM, Tim Chen wrote: On Tue, 2013-10-01 at 21:25 -0400, Waiman Long wrote: If the lock and unlock functions are done right, there should be no overlap of critical section. So it is job of the lock/unlock functions to make sure that critical section code won't leak out. There

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-02 Thread Waiman Long
On 10/02/2013 03:30 PM, Jason Low wrote: On Wed, Oct 2, 2013 at 12:19 PM, Waiman Longwaiman.l...@hp.com wrote: On 09/26/2013 06:42 PM, Jason Low wrote: On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: Okay, that would makes sense for consistency because we always first set node-lock = 0 at

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Waiman Long
On 10/01/2013 05:16 PM, Tim Chen wrote: On Tue, 2013-10-01 at 16:01 -0400, Waiman Long wrote: The cpu could still be executing out of order load instruction from the critical section before checking node->locked? Probably smp_mb() is still needed. Tim But this is the lock function, a

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Tim Chen
On Tue, 2013-10-01 at 16:01 -0400, Waiman Long wrote: > On 10/01/2013 12:48 PM, Tim Chen wrote: > > On Mon, 2013-09-30 at 12:36 -0400, Waiman Long wrote: > >> On 09/30/2013 12:10 PM, Jason Low wrote: > >>> On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: > On 09/28/2013 12:34 AM, Jason

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Waiman Long
On 10/01/2013 12:48 PM, Tim Chen wrote: On Mon, 2013-09-30 at 12:36 -0400, Waiman Long wrote: On 09/30/2013 12:10 PM, Jason Low wrote: On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock()

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Tim Chen
On Mon, 2013-09-30 at 12:36 -0400, Waiman Long wrote: > On 09/30/2013 12:10 PM, Jason Low wrote: > > On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: > >> On 09/28/2013 12:34 AM, Jason Low wrote: > Also, below is what the mcs_spin_lock() and mcs_spin_unlock() > functions would look

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Tim Chen
On Mon, 2013-09-30 at 12:36 -0400, Waiman Long wrote: On 09/30/2013 12:10 PM, Jason Low wrote: On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock() functions would look like after

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Waiman Long
On 10/01/2013 12:48 PM, Tim Chen wrote: On Mon, 2013-09-30 at 12:36 -0400, Waiman Long wrote: On 09/30/2013 12:10 PM, Jason Low wrote: On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock()

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Tim Chen
On Tue, 2013-10-01 at 16:01 -0400, Waiman Long wrote: On 10/01/2013 12:48 PM, Tim Chen wrote: On Mon, 2013-09-30 at 12:36 -0400, Waiman Long wrote: On 09/30/2013 12:10 PM, Jason Low wrote: On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote:

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-10-01 Thread Waiman Long
On 10/01/2013 05:16 PM, Tim Chen wrote: On Tue, 2013-10-01 at 16:01 -0400, Waiman Long wrote: The cpu could still be executing out of order load instruction from the critical section before checking node-locked? Probably smp_mb() is still needed. Tim But this is the lock function, a

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Waiman Long
On 09/30/2013 12:10 PM, Jason Low wrote: On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock() functions would look like after applying the proposed changes. static noinline void

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Tim Chen
On Fri, 2013-09-27 at 19:19 -0700, Paul E. McKenney wrote: > On Fri, Sep 27, 2013 at 04:54:06PM -0700, Jason Low wrote: > > On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney > > wrote: > > > Yep. The previous lock holder's smp_wmb() won't keep either the compiler > > > or the CPU from reordering

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Jason Low
On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: > On 09/28/2013 12:34 AM, Jason Low wrote: > >> Also, below is what the mcs_spin_lock() and mcs_spin_unlock() > >> functions would look like after applying the proposed changes. > >> > >> static noinline > >> void mcs_spin_lock(struct

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Waiman Long
On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock() functions would look like after applying the proposed changes. static noinline void mcs_spin_lock(struct mcs_spin_node **lock, struct mcs_spin_node *node) { struct mcs_spin_node

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Waiman Long
On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock() functions would look like after applying the proposed changes. static noinline void mcs_spin_lock(struct mcs_spin_node **lock, struct mcs_spin_node *node) { struct mcs_spin_node

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Jason Low
On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock() functions would look like after applying the proposed changes. static noinline void mcs_spin_lock(struct mcs_spin_node **lock,

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Tim Chen
On Fri, 2013-09-27 at 19:19 -0700, Paul E. McKenney wrote: On Fri, Sep 27, 2013 at 04:54:06PM -0700, Jason Low wrote: On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Yep. The previous lock holder's smp_wmb() won't keep either the compiler or the CPU

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-30 Thread Waiman Long
On 09/30/2013 12:10 PM, Jason Low wrote: On Mon, 2013-09-30 at 11:51 -0400, Waiman Long wrote: On 09/28/2013 12:34 AM, Jason Low wrote: Also, below is what the mcs_spin_lock() and mcs_spin_unlock() functions would look like after applying the proposed changes. static noinline void

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, Sep 27, 2013 at 7:19 PM, Paul E. McKenney wrote: > On Fri, Sep 27, 2013 at 04:54:06PM -0700, Jason Low wrote: >> On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney >> wrote: >> > Yep. The previous lock holder's smp_wmb() won't keep either the compiler >> > or the CPU from reordering

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Waiman Long
On 09/27/2013 02:09 PM, Tim Chen wrote: On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Fri, Sep 27, 2013 at 04:54:06PM -0700, Jason Low wrote: > On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney > wrote: > > Yep. The previous lock holder's smp_wmb() won't keep either the compiler > > or the CPU from reordering things for the new lock holder. They could for > > example reorder

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Davidlohr Bueso
On Fri, 2013-09-27 at 16:54 -0700, Jason Low wrote: > On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney > wrote: > > Yep. The previous lock holder's smp_wmb() won't keep either the compiler > > or the CPU from reordering things for the new lock holder. They could for > > example reorder the

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney wrote: > Yep. The previous lock holder's smp_wmb() won't keep either the compiler > or the CPU from reordering things for the new lock holder. They could for > example reorder the critical section to precede the node->locked check, > which would

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Fri, Sep 27, 2013 at 03:46:45PM -0700, Tim Chen wrote: > On Fri, 2013-09-27 at 13:38 -0700, Paul E. McKenney wrote: > > On Fri, Sep 27, 2013 at 12:38:53PM -0700, Tim Chen wrote: > > > On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: > > > > On Wed, Sep 25, 2013 at 03:10:49PM -0700,

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 13:38 -0700, Paul E. McKenney wrote: > On Fri, Sep 27, 2013 at 12:38:53PM -0700, Tim Chen wrote: > > On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: > > > On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: > > > > We will need the MCS lock code for doing

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Fri, Sep 27, 2013 at 12:38:53PM -0700, Tim Chen wrote: > On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: > > On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: > > > We will need the MCS lock code for doing optimistic spinning for rwsem. > > > Extracting the MCS code from

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, Sep 27, 2013 at 12:38 PM, Tim Chen wrote: > BTW, is the above memory barrier necessary? It seems like the xchg > instruction already provided a memory barrier. > > Now if we made the changes that Jason suggested: > > > /* Init node */ > - node->locked = 0; >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: > On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: > > We will need the MCS lock code for doing optimistic spinning for rwsem. > > Extracting the MCS code from mutex.c and put into its own file allow us > > to reuse this code

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: > On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: > > We will need the MCS lock code for doing optimistic spinning for rwsem. > > Extracting the MCS code from mutex.c and put into its own file allow us > > to reuse this code

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 09:12 -0700, Jason Low wrote: > On Fri, 2013-09-27 at 08:02 +0200, Ingo Molnar wrote: > > Would be nice to have this as a separate, add-on patch. Every single > > instruction removal that has no downside is an upside! > > Okay, so here is a patch. Tim, would you like to add

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, 2013-09-27 at 08:02 +0200, Ingo Molnar wrote: > Would be nice to have this as a separate, add-on patch. Every single > instruction removal that has no downside is an upside! Okay, so here is a patch. Tim, would you like to add this to v7? ... Subject: MCS lock: Remove and reorder

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: > We will need the MCS lock code for doing optimistic spinning for rwsem. > Extracting the MCS code from mutex.c and put into its own file allow us > to reuse this code easily for rwsem. > > Signed-off-by: Tim Chen > Signed-off-by:

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Peter Zijlstra
On Fri, Sep 27, 2013 at 07:05:00AM -0700, Joe Perches wrote: > On Fri, 2013-09-27 at 15:48 +0200, Peter Zijlstra wrote: > > On Fri, Sep 27, 2013 at 06:44:55AM -0700, Joe Perches wrote: > > > It's a CHK test, so it's only tested with --strict > > > > > > $ scripts/checkpatch.pl -f --strict

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Joe Perches
On Fri, 2013-09-27 at 15:48 +0200, Peter Zijlstra wrote: > On Fri, Sep 27, 2013 at 06:44:55AM -0700, Joe Perches wrote: > > It's a CHK test, so it's only tested with --strict > > > > $ scripts/checkpatch.pl -f --strict kernel/mutex.c 2>&1 | grep memory > > CHECK: memory barrier without comment >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Peter Zijlstra
On Fri, Sep 27, 2013 at 06:44:55AM -0700, Joe Perches wrote: > It's a CHK test, so it's only tested with --strict > > $ scripts/checkpatch.pl -f --strict kernel/mutex.c 2>&1 | grep memory > CHECK: memory barrier without comment > CHECK: memory barrier without comment > > It could be changed to

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Joe Perches
On Fri, 2013-09-27 at 13:23 +0200, Peter Zijlstra wrote: > checkpatch.pl should really warn about that; and it appears there > code in there for that; however: > > # grep -C3 smp_mb scripts/checkpatch.pl [] > # check for memory barriers without a comment. > if ($line =~ >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Peter Zijlstra
On Fri, Sep 27, 2013 at 08:02:13AM +0200, Ingo Molnar wrote: > Would be nice to have this as a separate, add-on patch. Every single > instruction removal that has no downside is an upside! > > You can add a comment that explains it. If someone is going to do add-on patches to the mcslock.h

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, 2013-09-27 at 08:02 +0200, Ingo Molnar wrote: > * Tim Chen wrote: > > > > If we prefer to optimize this a bit though, perhaps we can first move > > > the node->lock = 0 so that it gets executed after the "if (likely(prev > > > == NULL)) {}" code block and then delete "node->lock = 1"

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Ingo Molnar
* Tim Chen wrote: > > If we prefer to optimize this a bit though, perhaps we can first move > > the node->lock = 0 so that it gets executed after the "if (likely(prev > > == NULL)) {}" code block and then delete "node->lock = 1" inside the > > code block. > > I suppose we can save one

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Ingo Molnar
* Tim Chen tim.c.c...@linux.intel.com wrote: If we prefer to optimize this a bit though, perhaps we can first move the node-lock = 0 so that it gets executed after the if (likely(prev == NULL)) {} code block and then delete node-lock = 1 inside the code block. I suppose we can save

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, 2013-09-27 at 08:02 +0200, Ingo Molnar wrote: * Tim Chen tim.c.c...@linux.intel.com wrote: If we prefer to optimize this a bit though, perhaps we can first move the node-lock = 0 so that it gets executed after the if (likely(prev == NULL)) {} code block and then delete

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Peter Zijlstra
On Fri, Sep 27, 2013 at 08:02:13AM +0200, Ingo Molnar wrote: Would be nice to have this as a separate, add-on patch. Every single instruction removal that has no downside is an upside! You can add a comment that explains it. If someone is going to do add-on patches to the mcslock.h file,

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Joe Perches
On Fri, 2013-09-27 at 13:23 +0200, Peter Zijlstra wrote: checkpatch.pl should really warn about that; and it appears there code in there for that; however: # grep -C3 smp_mb scripts/checkpatch.pl [] # check for memory barriers without a comment. if ($line =~

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Peter Zijlstra
On Fri, Sep 27, 2013 at 06:44:55AM -0700, Joe Perches wrote: It's a CHK test, so it's only tested with --strict $ scripts/checkpatch.pl -f --strict kernel/mutex.c 21 | grep memory CHECK: memory barrier without comment CHECK: memory barrier without comment It could be changed to WARN so

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Joe Perches
On Fri, 2013-09-27 at 15:48 +0200, Peter Zijlstra wrote: On Fri, Sep 27, 2013 at 06:44:55AM -0700, Joe Perches wrote: It's a CHK test, so it's only tested with --strict $ scripts/checkpatch.pl -f --strict kernel/mutex.c 21 | grep memory CHECK: memory barrier without comment CHECK:

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Peter Zijlstra
On Fri, Sep 27, 2013 at 07:05:00AM -0700, Joe Perches wrote: On Fri, 2013-09-27 at 15:48 +0200, Peter Zijlstra wrote: On Fri, Sep 27, 2013 at 06:44:55AM -0700, Joe Perches wrote: It's a CHK test, so it's only tested with --strict $ scripts/checkpatch.pl -f --strict kernel/mutex.c 21 |

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for rwsem. Signed-off-by: Tim Chen tim.c.c...@linux.intel.com

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, 2013-09-27 at 08:02 +0200, Ingo Molnar wrote: Would be nice to have this as a separate, add-on patch. Every single instruction removal that has no downside is an upside! Okay, so here is a patch. Tim, would you like to add this to v7? ... Subject: MCS lock: Remove and reorder

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 09:12 -0700, Jason Low wrote: On Fri, 2013-09-27 at 08:02 +0200, Ingo Molnar wrote: Would be nice to have this as a separate, add-on patch. Every single instruction removal that has no downside is an upside! Okay, so here is a patch. Tim, would you like to add this

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, Sep 27, 2013 at 12:38 PM, Tim Chen tim.c.c...@linux.intel.com wrote: BTW, is the above memory barrier necessary? It seems like the xchg instruction already provided a memory barrier. Now if we made the changes that Jason suggested: /* Init node */ - node-locked = 0;

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Fri, Sep 27, 2013 at 12:38:53PM -0700, Tim Chen wrote: On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Tim Chen
On Fri, 2013-09-27 at 13:38 -0700, Paul E. McKenney wrote: On Fri, Sep 27, 2013 at 12:38:53PM -0700, Tim Chen wrote: On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Fri, Sep 27, 2013 at 03:46:45PM -0700, Tim Chen wrote: On Fri, 2013-09-27 at 13:38 -0700, Paul E. McKenney wrote: On Fri, Sep 27, 2013 at 12:38:53PM -0700, Tim Chen wrote: On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Yep. The previous lock holder's smp_wmb() won't keep either the compiler or the CPU from reordering things for the new lock holder. They could for example reorder the critical section to precede the

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Davidlohr Bueso
On Fri, 2013-09-27 at 16:54 -0700, Jason Low wrote: On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Yep. The previous lock holder's smp_wmb() won't keep either the compiler or the CPU from reordering things for the new lock holder. They could for

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Paul E. McKenney
On Fri, Sep 27, 2013 at 04:54:06PM -0700, Jason Low wrote: On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Yep. The previous lock holder's smp_wmb() won't keep either the compiler or the CPU from reordering things for the new lock holder. They could for

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Waiman Long
On 09/27/2013 02:09 PM, Tim Chen wrote: On Fri, 2013-09-27 at 08:29 -0700, Paul E. McKenney wrote: On Wed, Sep 25, 2013 at 03:10:49PM -0700, Tim Chen wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-27 Thread Jason Low
On Fri, Sep 27, 2013 at 7:19 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: On Fri, Sep 27, 2013 at 04:54:06PM -0700, Jason Low wrote: On Fri, Sep 27, 2013 at 4:01 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Yep. The previous lock holder's smp_wmb() won't keep either the

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Tim Chen
On Thu, 2013-09-26 at 15:42 -0700, Jason Low wrote: > On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: > > On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: > > > On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: > > > > On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: > > > > > On

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: > On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: > > On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: > > > On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: > > > > On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: > > > >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Davidlohr Bueso
On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: > On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: > > On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: > > > On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: > > > > On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: > > >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Tim Chen
On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: > On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: > > On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: > > > On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: > > > > On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: > > >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: > On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: > > On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: > > > On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: > > > > On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen > > > > wrote:

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Davidlohr Bueso
On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: > On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: > > On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: > > > On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen > > > wrote: > > > > We will need the MCS lock code for doing optimistic

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: > On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: > > On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen > > wrote: > > > We will need the MCS lock code for doing optimistic spinning for rwsem. > > > Extracting the MCS code from mutex.c and

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Davidlohr Bueso
On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: > On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen wrote: > > We will need the MCS lock code for doing optimistic spinning for rwsem. > > Extracting the MCS code from mutex.c and put into its own file allow us > > to reuse this code easily for rwsem. >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen wrote: > We will need the MCS lock code for doing optimistic spinning for rwsem. > Extracting the MCS code from mutex.c and put into its own file allow us > to reuse this code easily for rwsem. > > Signed-off-by: Tim Chen > Signed-off-by: Davidlohr Bueso

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Tim Chen
On Thu, 2013-09-26 at 10:40 +0200, Peter Zijlstra wrote: > On Thu, Sep 26, 2013 at 08:46:29AM +0200, Ingo Molnar wrote: > > > +/* > > > + * MCS lock defines > > > + * > > > + * This file contains the main data structure and API definitions of MCS > > > lock. > > > > A (very) short blurb about

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Ingo Molnar
* Peter Zijlstra wrote: > On Thu, Sep 26, 2013 at 08:46:29AM +0200, Ingo Molnar wrote: > > > +/* > > > + * MCS lock defines > > > + * > > > + * This file contains the main data structure and API definitions of MCS > > > lock. > > > > A (very) short blurb about what an MCS lock is would be

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Peter Zijlstra
On Thu, Sep 26, 2013 at 08:46:29AM +0200, Ingo Molnar wrote: > > +/* > > + * MCS lock defines > > + * > > + * This file contains the main data structure and API definitions of MCS > > lock. > > A (very) short blurb about what an MCS lock is would be nice here. A while back I suggested including

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Ingo Molnar
* Tim Chen wrote: > We will need the MCS lock code for doing optimistic spinning for rwsem. > Extracting the MCS code from mutex.c and put into its own file allow us > to reuse this code easily for rwsem. > > Signed-off-by: Tim Chen > Signed-off-by: Davidlohr Bueso > --- >

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Ingo Molnar
* Tim Chen tim.c.c...@linux.intel.com wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for rwsem. Signed-off-by: Tim Chen tim.c.c...@linux.intel.com

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Peter Zijlstra
On Thu, Sep 26, 2013 at 08:46:29AM +0200, Ingo Molnar wrote: +/* + * MCS lock defines + * + * This file contains the main data structure and API definitions of MCS lock. A (very) short blurb about what an MCS lock is would be nice here. A while back I suggested including a link to

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Ingo Molnar
* Peter Zijlstra pet...@infradead.org wrote: On Thu, Sep 26, 2013 at 08:46:29AM +0200, Ingo Molnar wrote: +/* + * MCS lock defines + * + * This file contains the main data structure and API definitions of MCS lock. A (very) short blurb about what an MCS lock is would be

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Tim Chen
On Thu, 2013-09-26 at 10:40 +0200, Peter Zijlstra wrote: On Thu, Sep 26, 2013 at 08:46:29AM +0200, Ingo Molnar wrote: +/* + * MCS lock defines + * + * This file contains the main data structure and API definitions of MCS lock. A (very) short blurb about what an MCS lock is

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen tim.c.c...@linux.intel.com wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for rwsem. Signed-off-by: Tim Chen

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Davidlohr Bueso
On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen tim.c.c...@linux.intel.com wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen tim.c.c...@linux.intel.com wrote: We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Davidlohr Bueso
On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen tim.c.c...@linux.intel.com wrote: We will need the MCS lock code for doing

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: On Wed, Sep 25, 2013 at 3:10 PM, Tim Chen tim.c.c...@linux.intel.com

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Tim Chen
On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: On Wed, Sep

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Davidlohr Bueso
On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 12:27 -0700, Jason Low wrote: On Wed, Sep

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Jason Low
On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:06 -0700, Davidlohr Bueso wrote: On Thu,

Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-26 Thread Tim Chen
On Thu, 2013-09-26 at 15:42 -0700, Jason Low wrote: On Thu, 2013-09-26 at 14:41 -0700, Tim Chen wrote: On Thu, 2013-09-26 at 14:09 -0700, Jason Low wrote: On Thu, 2013-09-26 at 13:40 -0700, Davidlohr Bueso wrote: On Thu, 2013-09-26 at 13:23 -0700, Jason Low wrote: On Thu, 2013-09-26

[PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-25 Thread Tim Chen
We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for rwsem. Signed-off-by: Tim Chen Signed-off-by: Davidlohr Bueso --- include/linux/mcslock.h | 58

[PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file

2013-09-25 Thread Tim Chen
We will need the MCS lock code for doing optimistic spinning for rwsem. Extracting the MCS code from mutex.c and put into its own file allow us to reuse this code easily for rwsem. Signed-off-by: Tim Chen tim.c.c...@linux.intel.com Signed-off-by: Davidlohr Bueso davidl...@hp.com ---