[PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2014-01-12 Thread Davidlohr Bueso
From: Davidlohr Bueso In futex_wake() there is clearly no point in taking the hb->lock if we know beforehand that there are no tasks to be woken. While the hash bucket's plist head is a cheap way of knowing this, we cannot rely 100% on it as there is a racy window between the futex_wait call and

[PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2014-01-12 Thread Davidlohr Bueso
From: Davidlohr Bueso davidl...@hp.com In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. While the hash bucket's plist head is a cheap way of knowing this, we cannot rely 100% on it as there is a racy window between the

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-26 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Darren Hart wrote: > On Mon, 2013-11-25 at 20:47 +0100, Thomas Gleixner wrote: > > So now your code melts down to: > > > > write(hb->waiters)|write(uaddr) > > mb|read(hb->waiters) > > read(uaddr) > > > > I fear you simply managed to make

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-26 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Darren Hart wrote: On Mon, 2013-11-25 at 20:47 +0100, Thomas Gleixner wrote: So now your code melts down to: write(hb-waiters)|write(uaddr) mb|read(hb-waiters) read(uaddr) I fear you simply managed to make the window small

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Darren Hart wrote: > On Mon, 2013-11-25 at 20:47 +0100, Thomas Gleixner wrote: > > > which restores the ordering guarantee, which the hash bucket lock > > > provided so far. > > > > Actually that's not true by design, it just happens to work. > > > > atomic_inc() on x86 is a

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Darren Hart
On Mon, 2013-11-25 at 20:47 +0100, Thomas Gleixner wrote: > On Sat, 23 Nov 2013, Thomas Gleixner wrote: > > On Fri, 22 Nov 2013, Davidlohr Bueso wrote: > > So with the atomic ops you are changing that to: > > > > CPU 0 CPU 1 > > > > val = *futex; > >

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Davidlohr Bueso wrote: > On Mon, 2013-11-25 at 18:32 +0100, Thomas Gleixner wrote: > > If the smp_mb() is heavy weight, then it will hurt massivly in the > > case where the hash bucket is not empty, because we add the price for > > the smp_mb() just for no gain. > > > > In

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Sat, 23 Nov 2013, Thomas Gleixner wrote: > On Fri, 22 Nov 2013, Davidlohr Bueso wrote: > So with the atomic ops you are changing that to: > > CPU 0 CPU 1 > > val = *futex; > futex_wait(futex, val); > > spin_lock(>lock); > >

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Davidlohr Bueso
On Mon, 2013-11-25 at 18:32 +0100, Thomas Gleixner wrote: > On Mon, 25 Nov 2013, Peter Zijlstra wrote: > > On Mon, Nov 25, 2013 at 05:23:51PM +0100, Thomas Gleixner wrote: > > > On Sat, 23 Nov 2013, Linus Torvalds wrote: > > > > > > > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner > > > >

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Peter Zijlstra
On Mon, Nov 25, 2013 at 06:32:55PM +0100, Thomas Gleixner wrote: > In that context it would also be helpful to measure the overhead on > x86 for the !empty case. Yes, because mfence is by no means a cheap instruction on x86. -- To unsubscribe from this list: send the line "unsubscribe

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Peter Zijlstra wrote: > On Mon, Nov 25, 2013 at 05:23:51PM +0100, Thomas Gleixner wrote: > > On Sat, 23 Nov 2013, Linus Torvalds wrote: > > > > > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner > > > wrote: > > > > > > > > Now the question is why we queue the waiter

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Peter Zijlstra
On Mon, Nov 25, 2013 at 05:23:51PM +0100, Thomas Gleixner wrote: > On Sat, 23 Nov 2013, Linus Torvalds wrote: > > > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner wrote: > > > > > > Now the question is why we queue the waiter _AFTER_ reading the user > > > space value. The comment in the code

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Sat, 23 Nov 2013, Linus Torvalds wrote: > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner wrote: > > > > Now the question is why we queue the waiter _AFTER_ reading the user > > space value. The comment in the code is pretty non sensical: > > > >* On the other hand, we insert q and

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Sat, 23 Nov 2013, Davidlohr Bueso wrote: > On Sat, 2013-11-23 at 19:46 -0800, Linus Torvalds wrote: > > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner wrote: > > > > > > Now the question is why we queue the waiter _AFTER_ reading the user > > > space value. The comment in the code is pretty

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Sat, 23 Nov 2013, Davidlohr Bueso wrote: On Sat, 2013-11-23 at 19:46 -0800, Linus Torvalds wrote: On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de wrote: Now the question is why we queue the waiter _AFTER_ reading the user space value. The comment in the code is

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Sat, 23 Nov 2013, Linus Torvalds wrote: On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de wrote: Now the question is why we queue the waiter _AFTER_ reading the user space value. The comment in the code is pretty non sensical: * On the other hand, we insert q

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Peter Zijlstra
On Mon, Nov 25, 2013 at 05:23:51PM +0100, Thomas Gleixner wrote: On Sat, 23 Nov 2013, Linus Torvalds wrote: On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de wrote: Now the question is why we queue the waiter _AFTER_ reading the user space value. The comment in the

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Peter Zijlstra wrote: On Mon, Nov 25, 2013 at 05:23:51PM +0100, Thomas Gleixner wrote: On Sat, 23 Nov 2013, Linus Torvalds wrote: On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de wrote: Now the question is why we queue the waiter _AFTER_

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Peter Zijlstra
On Mon, Nov 25, 2013 at 06:32:55PM +0100, Thomas Gleixner wrote: In that context it would also be helpful to measure the overhead on x86 for the !empty case. Yes, because mfence is by no means a cheap instruction on x86. -- To unsubscribe from this list: send the line unsubscribe linux-kernel

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Davidlohr Bueso
On Mon, 2013-11-25 at 18:32 +0100, Thomas Gleixner wrote: On Mon, 25 Nov 2013, Peter Zijlstra wrote: On Mon, Nov 25, 2013 at 05:23:51PM +0100, Thomas Gleixner wrote: On Sat, 23 Nov 2013, Linus Torvalds wrote: On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Sat, 23 Nov 2013, Thomas Gleixner wrote: On Fri, 22 Nov 2013, Davidlohr Bueso wrote: So with the atomic ops you are changing that to: CPU 0 CPU 1 val = *futex; futex_wait(futex, val); spin_lock(hb-lock); atomic_inc(hb-waiters);

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Davidlohr Bueso wrote: On Mon, 2013-11-25 at 18:32 +0100, Thomas Gleixner wrote: If the smp_mb() is heavy weight, then it will hurt massivly in the case where the hash bucket is not empty, because we add the price for the smp_mb() just for no gain. In that context

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Darren Hart
On Mon, 2013-11-25 at 20:47 +0100, Thomas Gleixner wrote: On Sat, 23 Nov 2013, Thomas Gleixner wrote: On Fri, 22 Nov 2013, Davidlohr Bueso wrote: So with the atomic ops you are changing that to: CPU 0 CPU 1 val = *futex; futex_wait(futex,

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-25 Thread Thomas Gleixner
On Mon, 25 Nov 2013, Darren Hart wrote: On Mon, 2013-11-25 at 20:47 +0100, Thomas Gleixner wrote: which restores the ordering guarantee, which the hash bucket lock provided so far. Actually that's not true by design, it just happens to work. atomic_inc() on x86 is a lock incl.

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-23 Thread Davidlohr Bueso
On Sat, 2013-11-23 at 19:46 -0800, Linus Torvalds wrote: > On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner wrote: > > > > Now the question is why we queue the waiter _AFTER_ reading the user > > space value. The comment in the code is pretty non sensical: > > > >* On the other hand, we

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-23 Thread Linus Torvalds
On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner wrote: > > Now the question is why we queue the waiter _AFTER_ reading the user > space value. The comment in the code is pretty non sensical: > >* On the other hand, we insert q and release the hash-bucket only >* after testing *uaddr.

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-23 Thread Thomas Gleixner
On Fri, 22 Nov 2013, Davidlohr Bueso wrote: > On Fri, 2013-11-22 at 17:25 -0800, Linus Torvalds wrote: > > On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso wrote: > > > In futex_wake() there is clearly no point in taking the hb->lock if > > > we know beforehand that there are no tasks to be

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-23 Thread Thomas Gleixner
On Fri, 22 Nov 2013, Davidlohr Bueso wrote: On Fri, 2013-11-22 at 17:25 -0800, Linus Torvalds wrote: On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso davidl...@hp.com wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-23 Thread Linus Torvalds
On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de wrote: Now the question is why we queue the waiter _AFTER_ reading the user space value. The comment in the code is pretty non sensical: * On the other hand, we insert q and release the hash-bucket only * after testing

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-23 Thread Davidlohr Bueso
On Sat, 2013-11-23 at 19:46 -0800, Linus Torvalds wrote: On Sat, Nov 23, 2013 at 5:16 AM, Thomas Gleixner t...@linutronix.de wrote: Now the question is why we queue the waiter _AFTER_ reading the user space value. The comment in the code is pretty non sensical: * On the other hand,

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Darren Hart
On Fri, 2013-11-22 at 19:19 -0800, Davidlohr Bueso wrote: > On Fri, 2013-11-22 at 17:25 -0800, Linus Torvalds wrote: > > On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso wrote: > > > In futex_wake() there is clearly no point in taking the hb->lock if > > > we know beforehand that there are no

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Darren Hart
On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote: > In futex_wake() there is clearly no point in taking the hb->lock if > we know beforehand that there are no tasks to be woken. This comes > at the smaller cost of doing some atomic operations to keep track of > the list's size.

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Hart, Darren
On Fri, 2013-11-22 at 21:40 -0800, Darren Hart wrote: > On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote: > > In futex_wake() there is clearly no point in taking the hb->lock if > > we know beforehand that there are no tasks to be woken. This comes > > at the smaller cost of doing some

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Darren Hart
On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote: > In futex_wake() there is clearly no point in taking the hb->lock if > we know beforehand that there are no tasks to be woken. This comes > at the smaller cost of doing some atomic operations to keep track of > the list's size.

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Waiman Long
On 11/22/2013 08:25 PM, Linus Torvalds wrote: On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso wrote: In futex_wake() there is clearly no point in taking the hb->lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic operations to

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Davidlohr Bueso
On Fri, 2013-11-22 at 17:25 -0800, Linus Torvalds wrote: > On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso wrote: > > In futex_wake() there is clearly no point in taking the hb->lock if > > we know beforehand that there are no tasks to be woken. This comes > > at the smaller cost of doing some

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Jason Low
On Fri, Nov 22, 2013 at 5:25 PM, Linus Torvalds wrote: > On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso wrote: >> In futex_wake() there is clearly no point in taking the hb->lock if >> we know beforehand that there are no tasks to be woken. This comes >> at the smaller cost of doing some

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Linus Torvalds
On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso wrote: > In futex_wake() there is clearly no point in taking the hb->lock if > we know beforehand that there are no tasks to be woken. This comes > at the smaller cost of doing some atomic operations to keep track of > the list's size. Hmm. Why?

[PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Davidlohr Bueso
In futex_wake() there is clearly no point in taking the hb->lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic operations to keep track of the list's size. Specifically, increment the counter when an element is added to the list, and

[PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Davidlohr Bueso
In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic operations to keep track of the list's size. Specifically, increment the counter when an element is added to the list, and

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Linus Torvalds
On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso davidl...@hp.com wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic operations to keep track of the list's size.

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Jason Low
On Fri, Nov 22, 2013 at 5:25 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso davidl...@hp.com wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Davidlohr Bueso
On Fri, 2013-11-22 at 17:25 -0800, Linus Torvalds wrote: On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso davidl...@hp.com wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Waiman Long
On 11/22/2013 08:25 PM, Linus Torvalds wrote: On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Buesodavidl...@hp.com wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Darren Hart
On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic operations to keep track of the list's size. Specifically,

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Hart, Darren
On Fri, 2013-11-22 at 21:40 -0800, Darren Hart wrote: On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Darren Hart
On Fri, 2013-11-22 at 16:56 -0800, Davidlohr Bueso wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are no tasks to be woken. This comes at the smaller cost of doing some atomic operations to keep track of the list's size. Specifically,

Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup

2013-11-22 Thread Darren Hart
On Fri, 2013-11-22 at 19:19 -0800, Davidlohr Bueso wrote: On Fri, 2013-11-22 at 17:25 -0800, Linus Torvalds wrote: On Fri, Nov 22, 2013 at 4:56 PM, Davidlohr Bueso davidl...@hp.com wrote: In futex_wake() there is clearly no point in taking the hb-lock if we know beforehand that there are