linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Darren Hart <dvhltc@us.ibm.com>
Cc: linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@elte.hu>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	"Peter W. Morreale" <pmorreale@novell.com>,
	Rik van Riel <riel@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Gregory Haskins <ghaskins@novell.com>,
	Sven-Thorsten Dietrich <sdietrich@novell.com>,
	Chris Mason <chris.mason@oracle.com>,
	John Cooper <john.cooper@third-harmonic.com>,
	Chris Wright <chrisw@sous-sol.org>
Subject: Re: [PATCH V2 0/6][RFC] futex: FUTEX_LOCK with optional adaptive spinning
Date: Tue, 06 Apr 2010 01:21:45 +0300	[thread overview]
Message-ID: <4BBA6279.20802@redhat.com> (raw)
In-Reply-To: <4BBA5C00.4090703@us.ibm.com>

On 04/06/2010 12:54 AM, Darren Hart wrote:
> Avi Kivity wrote:
>> On 04/05/2010 11:23 PM, Darren Hart wrote:
>>> In-Reply-To:
>>>
>>> NOT FOR INCLUSION
>>>
>>> The following patch series implements a new experimental kernel side 
>>> futex mutex
>>> via new FUTEX_LOCK and FUTEX_LOCK_ADAPTIVE futex op codes. The 
>>> adaptive spin
>>> follows the kernel mutex model of allowing one spinner until the 
>>> lock is
>>> released or the owner is descheduled. The patch currently allows the 
>>> user to
>>> specify if they want no spinning, a single adaptive spinner, or 
>>> multiple
>>> spinners (aggressive adaptive spinning, or aas... which I have 
>>> mistyped as "ass"
>>> enough times to realize a better term is indeed required :-).
>>
>> An interesting (but perhaps difficult to achieve) optimization would 
>> be to spin in userspace.
>
> I couldn't think of a lightweight way to determine when the owner has 
> been scheduled out in userspace. Kernel assistance is required. You 
> could do this on the schedule() side of things, but I figured I'd get 
> some strong pushback if I tried to add a hook into descheduling that 
> flipped a bit in the futex value stating the owner was about to 
> deschedule(). Still, that might be something to explore.

In the futex value it's hopeless (since a thread can hold many locks), 
but I don't think it's unreasonable to set a bit in the thread local 
storage area.  The futex format would then need to be extended to 
contain a pointer to this bit.


>
>>
>> How many cores (or hardware threads) does this machine have? 
>
> Sorry, I meant to include that. I tested on an 8 CPU (no hardware 
> threads) 2.6 GHz Opteron 2218 (2 QuadCore CPUs) system.
>
> > At 10%
>> duty cycle you have 25 waiters behind the lock on average.  I don't 
>> think this is realistic, and it means that spinning is invoked only 
>> rarely.
>
> Perhaps some instrumentation is in order, it seems to get invoked 
> enough to achieve some 20% increase in lock/unlock iterations. Perhaps 
> another metric would be of more value - such as average wait time?

Why measure an unrealistic workload?

>
>> I'd be interested in seeing runs where the average number of waiters 
>> is 0.2, 0.5, 1, and 2, corresponding to moderate-to-bad contention.
>> 25 average waiters on compute bound code means the application needs 
>> to be rewritten, no amount of mutex tweaking will help it.
>
> Perhaps something NR_CPUS threads would be of more interest? 

That seems artificial.

> At 10% that's about .8 and at 25% the 2 of your upper limit. I could 
> add a few more duty-cycle points and make 25% the max. I'll kick that 
> off and post the results... probably tomorrow, 10M iterations takes a 
> while, but makes the results relatively stable.

Thanks.  But why not vary the number of threads as well?

>
>> Does the wakeup code select the spinning waiter, or just a random 
>> waiter?
>
> The wakeup code selects the highest priority task in fifo order to 
> wake-up - however, under contention it is most likely going to go back 
> to sleep as another waiter will steal the lock out from under it. This 
> locking strategy is unashamedly about as "unfair" as it gets.

Best to avoid the wakeup if we notice the lock was stolen.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


  reply	other threads:[~2010-04-05 22:22 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-05 20:23 [PATCH V2 0/6][RFC] futex: FUTEX_LOCK with optional adaptive spinning Darren Hart
2010-04-05 20:23 ` [PATCH 1/6] futex: replace fshared and clockrt with combined flags Darren Hart
2010-04-05 20:23 ` [PATCH 2/6] futex: add futex_q static initializer Darren Hart
2010-04-05 20:23 ` [PATCH 3/6] futex: refactor futex_lock_pi_atomic Darren Hart
2010-04-05 20:23 ` [PATCH 4/6] futex: Add FUTEX_LOCK with optional adaptive spinning Darren Hart
2010-04-06 16:55   ` Thomas Gleixner
2010-04-07 17:26     ` Darren Hart
2010-04-07 19:59       ` Thomas Gleixner
2010-04-08  3:25         ` Darren Hart
2010-04-08 23:10           ` Peter W. Morreale
2010-04-09  5:41             ` Darren Hart
2010-04-09 13:13               ` Peter W. Morreale
2010-04-05 20:23 ` [PATCH 5/6] futex: handle timeout inside adaptive lock spin Darren Hart
2010-04-06  8:27   ` Thomas Gleixner
2010-04-07 17:31     ` Darren Hart
2010-04-07 18:44       ` Gregory Haskins
2010-04-07 23:15         ` Darren Hart
2010-04-05 20:23 ` [PATCH 6/6] futex: Add aggressive adaptive spinning argument to FUTEX_LOCK Darren Hart
2010-04-08  5:58   ` Darren Hart
2010-04-05 20:48 ` [PATCH V2^W V4 0/6][RFC] futex: FUTEX_LOCK with optional adaptive spinning Darren Hart
2010-04-05 21:15 ` [PATCH V2 " Avi Kivity
2010-04-05 21:54   ` Darren Hart
2010-04-05 22:21     ` Avi Kivity [this message]
2010-04-05 22:59       ` Darren Hart
2010-04-06 13:28         ` Avi Kivity
2010-04-06 13:35           ` Peter Zijlstra
2010-04-06 13:41             ` Avi Kivity
2010-04-06 14:09               ` Peter Zijlstra
2010-04-06 16:10                 ` Avi Kivity
2010-04-06 16:53                   ` Alan Cox
2010-04-06 13:51             ` Alan Cox
2010-04-06 15:28               ` Darren Hart
2010-04-06 16:06                 ` Avi Kivity
2010-04-06 16:14                   ` Thomas Gleixner
2010-04-06 16:20                     ` Avi Kivity
2010-04-07  6:18                       ` john cooper
2010-04-08  3:33                         ` Darren Hart
2010-04-09  5:52                           ` john cooper
2010-04-06 16:54                     ` Alan Cox
2010-04-06 18:15                       ` Thomas Gleixner
2010-04-06 16:44                 ` Alan Cox
2010-04-06 17:34                   ` Ulrich Drepper
2010-04-10 23:35                     ` Alan Cox
2010-04-10 23:53                       ` Ulrich Drepper
2010-04-06 19:31                   ` Thomas Gleixner
2010-04-06 20:02                     ` Ulrich Drepper
2010-04-06 23:16                       ` Thomas Gleixner
2010-04-06 23:36                         ` Darren Hart
2010-04-07  6:08                         ` drepper
2010-04-08  3:41                           ` Darren Hart
2010-04-08  4:29                             ` drepper
2010-04-07  5:33                     ` Avi Kivity
2010-04-06 21:22         ` Darren Hart
2010-04-05 23:15       ` Darren Hart
2010-04-05 23:29         ` Chris Wright
2010-04-06 13:30         ` Avi Kivity
2010-04-06  8:48   ` Peter Zijlstra
2010-04-06 14:47     ` Ulrich Drepper
2010-04-06 14:51       ` Peter Zijlstra
2010-04-06 15:33         ` Darren Hart
2010-04-06 15:37           ` Peter Zijlstra
2010-04-06 15:29 ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BBA6279.20802@redhat.com \
    --to=avi@redhat.com \
    --cc=chris.mason@oracle.com \
    --cc=chrisw@sous-sol.org \
    --cc=dvhltc@us.ibm.com \
    --cc=eric.dumazet@gmail.com \
    --cc=ghaskins@novell.com \
    --cc=john.cooper@third-harmonic.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=pmorreale@novell.com \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=sdietrich@novell.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).