All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rik van Riel <riel@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org, chegu_vinod@hp.com, peterz@infradead.org,
	mgorman@suse.de, mingo@redhat.com, Joe Mario <jmario@redhat.com>
Subject: Re: [PATCH 7/7] numa,sched: do statistics calculation using local variables only
Date: Fri, 17 Jan 2014 22:31:37 -0500	[thread overview]
Message-ID: <52D9F599.3040508@redhat.com> (raw)
In-Reply-To: <1389993129-28180-8-git-send-email-riel@redhat.com>

On 01/17/2014 04:12 PM, riel@redhat.com wrote:
> From: Rik van Riel <riel@redhat.com>
> 
> The current code in task_numa_placement calculates the difference
> between the old and the new value, but also temporarily stores half
> of the old value in the per-process variables.
> 
> The NUMA balancing code looks at those per-process variables, and
> having other tasks temporarily see halved statistics could lead to
> unwanted numa migrations. This can be avoided by doing all the math
> in local variables.
> 
> This change also simplifies the code a little.

I am seeing what looks like a performance improvement
with this patch, so it is not just a theoretical bug.
The improvement is small, as is to be expected with
such a small race, but with two 32-warehouse specjbb
instances on a 4-node, 10core/20thread per node system,
I see the following change in performance, and reduced
numa page migrations.

Without the patch:
run 1: throughput 367660 367660, migrated 3112982
run 2: throughput 353821 355612, migrated 2881317
run 3: throughput 355027 355027, migrated 3358105
run 4: throughput 354366 354366, migrated 3466687
run 5: throughput 356186 356186, migrated 3152194
run 6: throughput 361431 361431, migrated 3336219
run 7: throughput 354704 354704, migrated 3345418
run 8: throughput 363770 363770, migrated 3642925
run 9: throughput 363380 363380, migrated 3192836
run 10: throughput 358440 358440, migrated 3354028
  avg: througphut 358968, migrated 3284271

With the patch:
run 1: throughput 360580 360580, migrated 3169872
run 2: throughput 361303 361303, migrated 3220280
run 3: throughput 367692 367692, migrated 3096093
run 4: throughput 362320 362320, migrated 2981762
run 5: throughput 364201 364201, migrated 3089107
run 6: throughput 364561 364561, migrated 2892364
run 7: throughput 360771 360771, migrated 3086638
run 8: throughput 361530 361530, migrated 2933256
run 9: throughput 365841 365841, migrated 3356944
run 10: throughput 359188 359188, migrated 3394545
  avg: througphut 362798, migrated 3122086


-- 
All rights reversed

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Rik van Riel <riel@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org, chegu_vinod@hp.com, peterz@infradead.org,
	mgorman@suse.de, mingo@redhat.com, Joe Mario <jmario@redhat.com>
Subject: Re: [PATCH 7/7] numa,sched: do statistics calculation using local variables only
Date: Fri, 17 Jan 2014 22:31:37 -0500	[thread overview]
Message-ID: <52D9F599.3040508@redhat.com> (raw)
In-Reply-To: <1389993129-28180-8-git-send-email-riel@redhat.com>

On 01/17/2014 04:12 PM, riel@redhat.com wrote:
> From: Rik van Riel <riel@redhat.com>
> 
> The current code in task_numa_placement calculates the difference
> between the old and the new value, but also temporarily stores half
> of the old value in the per-process variables.
> 
> The NUMA balancing code looks at those per-process variables, and
> having other tasks temporarily see halved statistics could lead to
> unwanted numa migrations. This can be avoided by doing all the math
> in local variables.
> 
> This change also simplifies the code a little.

I am seeing what looks like a performance improvement
with this patch, so it is not just a theoretical bug.
The improvement is small, as is to be expected with
such a small race, but with two 32-warehouse specjbb
instances on a 4-node, 10core/20thread per node system,
I see the following change in performance, and reduced
numa page migrations.

Without the patch:
run 1: throughput 367660 367660, migrated 3112982
run 2: throughput 353821 355612, migrated 2881317
run 3: throughput 355027 355027, migrated 3358105
run 4: throughput 354366 354366, migrated 3466687
run 5: throughput 356186 356186, migrated 3152194
run 6: throughput 361431 361431, migrated 3336219
run 7: throughput 354704 354704, migrated 3345418
run 8: throughput 363770 363770, migrated 3642925
run 9: throughput 363380 363380, migrated 3192836
run 10: throughput 358440 358440, migrated 3354028
  avg: througphut 358968, migrated 3284271

With the patch:
run 1: throughput 360580 360580, migrated 3169872
run 2: throughput 361303 361303, migrated 3220280
run 3: throughput 367692 367692, migrated 3096093
run 4: throughput 362320 362320, migrated 2981762
run 5: throughput 364201 364201, migrated 3089107
run 6: throughput 364561 364561, migrated 2892364
run 7: throughput 360771 360771, migrated 3086638
run 8: throughput 361530 361530, migrated 2933256
run 9: throughput 365841 365841, migrated 3356944
run 10: throughput 359188 359188, migrated 3394545
  avg: througphut 362798, migrated 3122086


-- 
All rights reversed

  reply	other threads:[~2014-01-18  3:32 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-17 21:12 [PATCH v2 0/7] pseudo-interleaving for automatic NUMA balancing riel
2014-01-17 21:12 ` riel
2014-01-17 21:12 ` [PATCH 1/7] numa,sched,mm: remove p->numa_migrate_deferred riel
2014-01-17 21:12   ` riel
2014-01-17 21:12 ` [PATCH 2/7] numa,sched: track from which nodes NUMA faults are triggered riel
2014-01-17 21:12   ` riel
2014-01-17 21:12 ` [PATCH 3/7] numa,sched: build per numa_group active node mask from faults_from statistics riel
2014-01-17 21:12   ` riel
2014-01-20 16:31   ` Peter Zijlstra
2014-01-20 16:31     ` Peter Zijlstra
2014-01-20 18:55     ` Rik van Riel
2014-01-20 18:55       ` Rik van Riel
2014-01-20 16:55   ` Peter Zijlstra
2014-01-20 16:55     ` Peter Zijlstra
2014-01-17 21:12 ` [PATCH 4/7] numa,sched: tracepoints for NUMA balancing active nodemask changes riel
2014-01-17 21:12   ` riel
2014-01-20 16:52   ` Peter Zijlstra
2014-01-20 16:52     ` Peter Zijlstra
2014-01-20 18:51     ` Rik van Riel
2014-01-20 18:51       ` Rik van Riel
2014-01-20 19:05     ` Steven Rostedt
2014-01-20 19:05       ` Steven Rostedt
2014-01-17 21:12 ` [PATCH 5/7] numa,sched,mm: use active_nodes nodemask to limit numa migrations riel
2014-01-17 21:12   ` riel
2014-01-17 21:12 ` [PATCH 6/7] numa,sched: normalize faults_from stats and weigh by CPU use riel
2014-01-17 21:12   ` riel
2014-01-20 16:57   ` Peter Zijlstra
2014-01-20 16:57     ` Peter Zijlstra
2014-01-20 19:02     ` Rik van Riel
2014-01-20 19:02       ` Rik van Riel
2014-01-20 19:10       ` Peter Zijlstra
2014-01-20 19:10         ` Peter Zijlstra
2014-01-17 21:12 ` [PATCH 7/7] numa,sched: do statistics calculation using local variables only riel
2014-01-17 21:12   ` riel
2014-01-18  3:31   ` Rik van Riel [this message]
2014-01-18  3:31     ` Rik van Riel
2014-01-18 22:05 ` [PATCH v2 0/7] pseudo-interleaving for automatic NUMA balancing Chegu Vinod
2014-01-18 22:05   ` 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=52D9F599.3040508@redhat.com \
    --to=riel@redhat.com \
    --cc=chegu_vinod@hp.com \
    --cc=jmario@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.