[PATCH] cache align vm_stat

[PATCH] cache align vm_stat

[Dimitri Sivanich]

Avoid false sharing of the vm_stat array.

This was found to adversely affect tmpfs I/O performance.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
---
 mm/vmstat.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/mm/vmstat.c
===================================================================
--- linux.orig/mm/vmstat.c
+++ linux/mm/vmstat.c
@@ -78,7 +78,7 @@ void vm_events_fold_cpu(int cpu)
  *
  * vm_stat contains the global counters
  */
-atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
+atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;
 EXPORT_SYMBOL(vm_stat);
 
 #ifdef CONFIG_SMP

[PATCH] cache align vm_stat

[Dimitri Sivanich]

Avoid false sharing of the vm_stat array.

This was found to adversely affect tmpfs I/O performance.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
---
 mm/vmstat.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/mm/vmstat.c
===================================================================
--- linux.orig/mm/vmstat.c
+++ linux/mm/vmstat.c
@@ -78,7 +78,7 @@ void vm_events_fold_cpu(int cpu)
  *
  * vm_stat contains the global counters
  */
-atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
+atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;
 EXPORT_SYMBOL(vm_stat);
 
 #ifdef CONFIG_SMP

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Christoph Lameter]

On Mon, 24 Oct 2011, Dimitri Sivanich wrote:

> Avoid false sharing of the vm_stat array.

Acked-by: Christoph Lameter <cl@gentwo.org>

Re: [PATCH] cache align vm_stat

[Christoph Lameter]

On Mon, 24 Oct 2011, Dimitri Sivanich wrote:

> Avoid false sharing of the vm_stat array.

Acked-by: Christoph Lameter <cl@gentwo.org>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Andrew Morton]

On Wed, 26 Oct 2011 21:31:46 -0500 (CDT)
Christoph Lameter <cl@gentwo.org> wrote:

> On Mon, 24 Oct 2011, Dimitri Sivanich wrote:
> 
> > Avoid false sharing of the vm_stat array.

Did we have some nice machine-description and measurement results which
can be included in the changelog?  Such things should always be
included with a performace patch!

> Acked-by: Christoph Lameter <cl@gentwo.org>

Do christoph@lameter.com, cl@linux-foundation.org and cl@linux.com still work?

Re: [PATCH] cache align vm_stat

[Andrew Morton]

On Wed, 26 Oct 2011 21:31:46 -0500 (CDT)
Christoph Lameter <cl@gentwo.org> wrote:

> On Mon, 24 Oct 2011, Dimitri Sivanich wrote:
> 
> > Avoid false sharing of the vm_stat array.

Did we have some nice machine-description and measurement results which
can be included in the changelog?  Such things should always be
included with a performace patch!

> Acked-by: Christoph Lameter <cl@gentwo.org>

Do christoph@lameter.com, cl@linux-foundation.org and cl@linux.com still work?

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Christoph Lameter]

On Fri, 28 Oct 2011, Andrew Morton wrote:

> Do christoph@lameter.com, cl@linux-foundation.org and cl@linux.com still
> work?

Yes but christoph@lameter.com should only be used for private matters.
The other two had some hiccups in the last weeks. I think I may be going
back to linux.com once I feel confident that the issues have all been
worked out.

Re: [PATCH] cache align vm_stat

[Christoph Lameter]

On Fri, 28 Oct 2011, Andrew Morton wrote:

> Do christoph@lameter.com, cl@linux-foundation.org and cl@linux.com still
> work?

Yes but christoph@lameter.com should only be used for private matters.
The other two had some hiccups in the last weeks. I think I may be going
back to linux.com once I feel confident that the issues have all been
worked out.


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Dimitri Sivanich]

On Fri, Oct 28, 2011 at 03:54:56PM -0700, Andrew Morton wrote:
> On Wed, 26 Oct 2011 21:31:46 -0500 (CDT)
> Christoph Lameter <cl@gentwo.org> wrote:
> 
> > On Mon, 24 Oct 2011, Dimitri Sivanich wrote:
> > 
> > > Avoid false sharing of the vm_stat array.
> 
> Did we have some nice machine-description and measurement results which
> can be included in the changelog?  Such things should always be
> included with a performace patch!
> 
Tests run on a 640 cpu UV system.

With 120 threads doing parallel writes, each to different tmpfs mounts:
No patch:		~300 MB/sec
With vm_stat alignment:	~430 MB/sec

Re: [PATCH] cache align vm_stat

[Dimitri Sivanich]

On Fri, Oct 28, 2011 at 03:54:56PM -0700, Andrew Morton wrote:
> On Wed, 26 Oct 2011 21:31:46 -0500 (CDT)
> Christoph Lameter <cl@gentwo.org> wrote:
> 
> > On Mon, 24 Oct 2011, Dimitri Sivanich wrote:
> > 
> > > Avoid false sharing of the vm_stat array.
> 
> Did we have some nice machine-description and measurement results which
> can be included in the changelog?  Such things should always be
> included with a performace patch!
> 
Tests run on a 640 cpu UV system.

With 120 threads doing parallel writes, each to different tmpfs mounts:
No patch:		~300 MB/sec
With vm_stat alignment:	~430 MB/sec

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Mel Gorman]

On Thu, Oct 27, 2011 at 10:50:25AM +0200, Mel Gorman wrote:
> On Mon, Oct 24, 2011 at 11:10:35AM -0500, Dimitri Sivanich wrote:
> > Avoid false sharing of the vm_stat array.
> > 
> > This was found to adversely affect tmpfs I/O performance.
> > 
> 
> I think this fix is overly simplistic.

In a lesson on why I should not even try reviewing while attending
conferences, I totally misread what the patch is doing. It's aligning
the array to avoid false sharing with other global data, not aligning
each element. This is reasonable.

Acked-by: Mel Gorman <mel@csn.ul.ie>

Re: [PATCH] cache align vm_stat

[Mel Gorman]

On Thu, Oct 27, 2011 at 10:50:25AM +0200, Mel Gorman wrote:
> On Mon, Oct 24, 2011 at 11:10:35AM -0500, Dimitri Sivanich wrote:
> > Avoid false sharing of the vm_stat array.
> > 
> > This was found to adversely affect tmpfs I/O performance.
> > 
> 
> I think this fix is overly simplistic.

In a lesson on why I should not even try reviewing while attending
conferences, I totally misread what the patch is doing. It's aligning
the array to avoid false sharing with other global data, not aligning
each element. This is reasonable.

Acked-by: Mel Gorman <mel@csn.ul.ie>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Dimitri Sivanich]

On Thu, Oct 27, 2011 at 10:50:25AM +0200, Mel Gorman wrote:
> On Mon, Oct 24, 2011 at 11:10:35AM -0500, Dimitri Sivanich wrote:
> > Avoid false sharing of the vm_stat array.
> > 
> > This was found to adversely affect tmpfs I/O performance.
> > 
> 
> I think this fix is overly simplistic. It is moving each counter into
> its own cache line. While I accept that this will help the preformance
> of the tmpfs-based workload, it will adversely affect workloads that
> touch a lot of counters because of the increased cache footprint.

To reiterate, this change is as follows:
  -atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
  +atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;

I think you've misunderstood the effect of the fix.  It does not move
each counter into it's own cache line.  It forces the overall array to
start on a cache line boundary.  From 'nm -n vmlinux' output:
	ffffffff81e04200 d hash_lock
	ffffffff81e04240 D vm_stat
	ffffffff81e04340 d nr_files
Unless I'm missing something, vm_stat is using 4 cachelines, not 24.
And yes, the addresses of the first 3 counters are, in fact,
ffffffff81e04240, ffffffff81e04248, and ffffffff81e04250.

Also, while this patch does make some difference in performance, it is
only an incremental step in improving vm_stat contention.  More will need
to be done.

Also, I've found that separating counters into their own cacheline has less
of an effect on performance than one might think.  It really has more to do
with the number of updates that each of these counters (especially
NR_FREE_PAGES) is receiving.

> 
> 1. Is it possible to rearrange the vmstat array such that two hot
>    counters do not share a cache line?

See my comment above.

> 2. Has Andrew's suggestion to alter the per-cpu threshold based on the
>    value of the global counter to reduce conflicts been tried?

Altering the per-cpu threshold (that returned from calculate*threshold in
mm/vmstat.c) works well, but see my previous posts about the size of
threshold required to achieve reasonable scaling performance.  Values are
much higher than the currently allowed 125 (more on the order of 1000-2000).

> 
> (I'm at Linux Con at the moment so will be even slower to respond than
> usual)
> 
> -- 
> Mel Gorman
> SUSE Labs

Re: [PATCH] cache align vm_stat

[Dimitri Sivanich]

On Thu, Oct 27, 2011 at 10:50:25AM +0200, Mel Gorman wrote:
> On Mon, Oct 24, 2011 at 11:10:35AM -0500, Dimitri Sivanich wrote:
> > Avoid false sharing of the vm_stat array.
> > 
> > This was found to adversely affect tmpfs I/O performance.
> > 
> 
> I think this fix is overly simplistic. It is moving each counter into
> its own cache line. While I accept that this will help the preformance
> of the tmpfs-based workload, it will adversely affect workloads that
> touch a lot of counters because of the increased cache footprint.

To reiterate, this change is as follows:
  -atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
  +atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;

I think you've misunderstood the effect of the fix.  It does not move
each counter into it's own cache line.  It forces the overall array to
start on a cache line boundary.  From 'nm -n vmlinux' output:
	ffffffff81e04200 d hash_lock
	ffffffff81e04240 D vm_stat
	ffffffff81e04340 d nr_files
Unless I'm missing something, vm_stat is using 4 cachelines, not 24.
And yes, the addresses of the first 3 counters are, in fact,
ffffffff81e04240, ffffffff81e04248, and ffffffff81e04250.

Also, while this patch does make some difference in performance, it is
only an incremental step in improving vm_stat contention.  More will need
to be done.

Also, I've found that separating counters into their own cacheline has less
of an effect on performance than one might think.  It really has more to do
with the number of updates that each of these counters (especially
NR_FREE_PAGES) is receiving.

> 
> 1. Is it possible to rearrange the vmstat array such that two hot
>    counters do not share a cache line?

See my comment above.

> 2. Has Andrew's suggestion to alter the per-cpu threshold based on the
>    value of the global counter to reduce conflicts been tried?

Altering the per-cpu threshold (that returned from calculate*threshold in
mm/vmstat.c) works well, but see my previous posts about the size of
threshold required to achieve reasonable scaling performance.  Values are
much higher than the currently allowed 125 (more on the order of 1000-2000).

> 
> (I'm at Linux Con at the moment so will be even slower to respond than
> usual)
> 
> -- 
> Mel Gorman
> SUSE Labs

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] cache align vm_stat

[Mel Gorman]

On Mon, Oct 24, 2011 at 11:10:35AM -0500, Dimitri Sivanich wrote:
> Avoid false sharing of the vm_stat array.
> 
> This was found to adversely affect tmpfs I/O performance.
> 

I think this fix is overly simplistic. It is moving each counter into
its own cache line. While I accept that this will help the preformance
of the tmpfs-based workload, it will adversely affect workloads that
touch a lot of counters because of the increased cache footprint.

1. Is it possible to rearrange the vmstat array such that two hot
   counters do not share a cache line?
2. Has Andrew's suggestion to alter the per-cpu threshold based on the
   value of the global counter to reduce conflicts been tried?

(I'm at Linux Con at the moment so will be even slower to respond than
usual)

-- 
Mel Gorman
SUSE Labs

Re: [PATCH] cache align vm_stat

[Mel Gorman]

On Mon, Oct 24, 2011 at 11:10:35AM -0500, Dimitri Sivanich wrote:
> Avoid false sharing of the vm_stat array.
> 
> This was found to adversely affect tmpfs I/O performance.
> 

I think this fix is overly simplistic. It is moving each counter into
its own cache line. While I accept that this will help the preformance
of the tmpfs-based workload, it will adversely affect workloads that
touch a lot of counters because of the increased cache footprint.

1. Is it possible to rearrange the vmstat array such that two hot
   counters do not share a cache line?
2. Has Andrew's suggestion to alter the per-cpu threshold based on the
   value of the global counter to reduce conflicts been tried?

(I'm at Linux Con at the moment so will be even slower to respond than
usual)

-- 
Mel Gorman
SUSE Labs

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>