linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rik van Riel <riel@redhat.com>
To: Michel Lespinasse <walken@google.com>
Cc: linux-kernel@vger.kernel.org, aquini@redhat.com,
	eric.dumazet@gmail.com, lwoodman@redhat.com, jeremy@goop.org,
	Jan Beulich <JBeulich@novell.com>,
	knoel@redhat.com, chegu_vinod@hp.com,
	raghavendra.kt@linux.vnet.ibm.com, mingo@redhat.com
Subject: Re: [PATCH 4/5] x86,smp: keep spinlock delay values per hashed spinlock address
Date: Thu, 10 Jan 2013 08:05:00 -0500	[thread overview]
Message-ID: <50EEBC7C.70508@redhat.com> (raw)
In-Reply-To: <CANN689H1eb_h-eX3wiEMczrA9hrX7C9zgwWzg0N7w_UcHHA2MA@mail.gmail.com>

On 01/10/2013 08:01 AM, Michel Lespinasse wrote:
> On Tue, Jan 8, 2013 at 2:31 PM, Rik van Riel <riel@redhat.com> wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>>
>> Eric Dumazet found a regression with the first version of the spinlock
>> backoff code, in a workload where multiple spinlocks were contended,
>> each having a different wait time.
>>
>> This patch has multiple delay values per cpu, indexed on a hash
>> of the lock address, to avoid that problem.
>>
>> Eric Dumazet wrote:
>>
>> I did some tests with your patches with following configuration :
>>
>> tc qdisc add dev eth0 root htb r2q 1000 default 3
>> (to force a contention on qdisc lock, even with a multi queue net
>> device)
>>
>> and 24 concurrent "netperf -t UDP_STREAM -H other_machine -- -m 128"
>>
>> Machine : 2 Intel(R) Xeon(R) CPU X5660  @ 2.80GHz
>> (24 threads), and a fast NIC (10Gbps)
>>
>> Resulting in a 13 % regression (676 Mbits -> 595 Mbits)
>>
>> In this workload we have at least two contended spinlocks, with
>> different delays. (spinlocks are not held for the same duration)
>>
>> It clearly defeats your assumption of a single per cpu delay being OK :
>> Some cpus are spinning too long while the lock was released.
>>
>> We might try to use a hash on lock address, and an array of 16 different
>> delays so that different spinlocks have a chance of not sharing the same
>> delay.
>>
>> With following patch, I get 982 Mbits/s with same bench, so an increase
>> of 45 % instead of a 13 % regression.
>
> Note that these results were with your v1 proposal. With v3 proposal,
> on a slightly different machine (2 socket sandybridge) with a similar
> NIC, I am not seeing the regression when not using the hash table. I
> think this is because v3 got more conservative about mixed spinlock
> hold times, and converges towards the shortest of the hold times in
> that case.

Eric,

with just patches 1-3, can you still reproduce the
regression on your system?

In other words, could we get away with dropping the
complexity of patch 4, or do we still need it?


-- 
All rights reversed

  reply	other threads:[~2013-01-10 13:11 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-08 22:26 [PATCH 0/5] x86,smp: make ticket spinlock proportional backoff w/ auto tuning Rik van Riel
2013-01-08 22:30 ` [PATCH 3/5] x86,smp: auto tune spinlock backoff delay factor Rik van Riel
2013-01-10  3:13   ` Rafael Aquini
2013-01-10 12:49   ` Michel Lespinasse
2013-01-08 22:31 ` [PATCH 4/5] x86,smp: keep spinlock delay values per hashed spinlock address Rik van Riel
2013-01-10  3:14   ` Rafael Aquini
2013-01-10 13:01   ` Michel Lespinasse
2013-01-10 13:05     ` Rik van Riel [this message]
2013-01-10 13:15       ` Michel Lespinasse
2013-01-08 22:32 ` [DEBUG PATCH 5/5] x86,smp: add debugging code to track spinlock delay value Rik van Riel
2013-01-08 22:32 ` [PATCH 2/5] x86,smp: proportional backoff for ticket spinlocks Rik van Riel
2013-01-08 22:50   ` Eric Dumazet
2013-01-08 22:54     ` Rik van Riel
2013-01-10  2:30   ` Rafael Aquini
2013-01-08 22:32 ` [PATCH 1/5] x86,smp: move waiting on contended ticket lock out of line Rik van Riel
2013-01-08 22:43   ` Eric Dumazet
2013-01-10 17:38   ` Raghavendra K T
2013-01-09 12:50 ` [PATCH 0/5] x86,smp: make ticket spinlock proportional backoff w/ auto tuning Raghavendra K T
2013-01-10  2:27   ` Rafael Aquini
2013-01-10 17:36     ` Raghavendra K T
2013-01-11 20:11       ` Rik van Riel
2013-01-13 18:07         ` Raghavendra K T
2013-01-10 15:19 ` Mike Galbraith
2013-01-10 15:31   ` Rik van Riel
2013-01-10 19:30     ` Mike Galbraith
2013-01-24 13:28       ` Ingo Molnar
2013-01-10 22:24 ` Chegu Vinod

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=50EEBC7C.70508@redhat.com \
    --to=riel@redhat.com \
    --cc=JBeulich@novell.com \
    --cc=aquini@redhat.com \
    --cc=chegu_vinod@hp.com \
    --cc=eric.dumazet@gmail.com \
    --cc=jeremy@goop.org \
    --cc=knoel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lwoodman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=raghavendra.kt@linux.vnet.ibm.com \
    --cc=walken@google.com \
    /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).