Messages in this thread | | | Subject | Re: [PATCH 4/5] futex: Avoid taking hb lock if nothing to wakeup | From | Darren Hart <> | Date | Fri, 22 Nov 2013 23:20:05 -0800 |
| |
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, increment the counter when an element is > added to the list, and decrement when it is removed. Of course, if the > counter is 0, then there are no tasks blocked on a futex. Some special > considerations: > > - increment the counter at queue_lock() as we always end up calling > queue_me() which adds the element to the list. Upon any error, > queue_unlock() is called for housekeeping, for which we decrement > to mach the increment done in queue_lock().
^match
> > - decrement the counter at __unqueue_me() to reflect when an element is > removed from the queue for wakeup related purposes.
__unqueue_futex (not __unqueue_me)
> @@ -999,6 +1001,10 @@ futex_wake(u32 __user *uaddr, unsigned int flags, int nr_wake, u32 bitset) > goto out; > > hb = hash_futex(&key); > + /* make sure we really have tasks to wakeup */
Nit, but please use proper sentence formatting for consistency with the rest of the comments in futex.c (most of them anyway).
/* Make sure we really have tasks to wake up. */
Now... I'm not thrilled with adding atomics if we don't need to, especially for an optimization since the atomics themselves cause enough problems, especially across a large number of CPUs... I'll respond to Linus's thread though.
-- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
| |