use local_t for page statistics

PATCH In review Benjamin LaHaise 6 participants 13 messages Jan 6, 2006 – Jan 9, 2006

Subsystems (8)

Memory management MM core Driver core Page allocator +4 more subsystems

lore

Share & tools

lore

Benjamin LaHaise

author

2006-01-06 21:53 UTC #0 lore raw

PATCH 1/1 use local_t for page statistics

The patch below converts the mm page_states counters to use local_t. mod_page_state shows up in a few profiles on x86 and x86-64 due to the disable/enable interrupts operations touching the flags register. On both my laptop (Pentium M) and P4 test box this results in about 10 additional /bin/bash -c exit 0 executions per second (P4 went from ~759/s to ~771/s). Tested on x86 and x86-64. Oh, also add a pgcow statistic for the number of COW page faults.

Signed-off-by Benjamin LaHaise <[email protected]>

arch/i386/mm/pgtable.c +5 -5

index 9db3242..293ba9a 100644
@@ -59,11 +59,11 @@ void show_mem(void)
 	printk(KERN_INFO "%d pages swap cached\n", cached);
 
 	get_page_state(&ps);
-	printk(KERN_INFO "%lu pages dirty\n", ps.nr_dirty);
-	printk(KERN_INFO "%lu pages writeback\n", ps.nr_writeback);
-	printk(KERN_INFO "%lu pages mapped\n", ps.nr_mapped);
-	printk(KERN_INFO "%lu pages slab\n", ps.nr_slab);
-	printk(KERN_INFO "%lu pages pagetables\n", ps.nr_page_table_pages);
+	printk(KERN_INFO "%lu pages dirty\n", local_read(&ps.nr_dirty));
+	printk(KERN_INFO "%lu pages writeback\n", local_read(&ps.nr_writeback));
+	printk(KERN_INFO "%lu pages mapped\n", local_read(&ps.nr_mapped));
+	printk(KERN_INFO "%lu pages slab\n", local_read(&ps.nr_slab));
+	printk(KERN_INFO "%lu pages pagetables\n", local_read(&ps.nr_page_table_pages));
 }
 
 /*

drivers/base/node.c +4 -4

index 16c513a..5ae1ead 100644
@@ -81,10 +81,10 @@ static ssize_t node_read_meminfo(struct 
 		       nid, K(i.freehigh),
 		       nid, K(i.totalram - i.totalhigh),
 		       nid, K(i.freeram - i.freehigh),
-		       nid, K(ps.nr_dirty),
-		       nid, K(ps.nr_writeback),
-		       nid, K(ps.nr_mapped),
-		       nid, K(ps.nr_slab));
+		       nid, K(local_read(&ps.nr_dirty)),
+		       nid, K(local_read(&ps.nr_writeback)),
+		       nid, K(local_read(&ps.nr_mapped)),
+		       nid, K(local_read(&ps.nr_slab)));
 	n += hugetlb_report_node_meminfo(nid, buf + n);
 	return n;
 }

fs/proc/proc_misc.c +6 -6

index 5b6b0b6..e76b11f 100644
@@ -135,7 +135,7 @@ static int meminfo_read_proc(char *page,
 /*
  * display in kilobytes.
  */
-#define K(x) ((x) << (PAGE_SHIFT - 10))
+#define K(x) ((unsigned long)(x) << (PAGE_SHIFT - 10))
 	si_meminfo(&i);
 	si_swapinfo(&i);
 	committed = atomic_read(&vm_committed_space);

@@ -188,13 +188,13 @@ static int meminfo_read_proc(char *page,
 		K(i.freeram-i.freehigh),
 		K(i.totalswap),
 		K(i.freeswap),
-		K(ps.nr_dirty),
-		K(ps.nr_writeback),
-		K(ps.nr_mapped),
-		K(ps.nr_slab),
+		K(local_read(&ps.nr_dirty)),
+		K(local_read(&ps.nr_writeback)),
+		K(local_read(&ps.nr_mapped)),
+		K(local_read(&ps.nr_slab)),
 		K(allowed),
 		K(committed),
-		K(ps.nr_page_table_pages),
+		K(local_read(&ps.nr_page_table_pages)),
 		(unsigned long)VMALLOC_TOTAL >> 10,
 		vmi.used >> 10,
 		vmi.largest_chunk >> 10

include/asm-x86_64/local.h +0 -0

include/linux/page-flags.h +55 ~~-53~~

133 lines not on this page · raw message

mm/memory.c +2 -0

index 7197f9b..db3faea 100644
@@ -1410,6 +1410,7 @@ static inline void cow_user_page(struct 
 		return;
 		
 	}
+	inc_page_state(pgcow);
 	copy_user_highpage(dst, src, va);
 }

@@ -2067,6 +2068,7 @@ retry:
 		page = alloc_page_vma(GFP_HIGHUSER, vma, address);
 		if (!page)
 			goto oom;
+		inc_page_state(pgcow);
 		copy_user_highpage(page, new_page, address);
 		page_cache_release(new_page);
 		new_page = page;

mm/page_alloc.c +37 ~~-37~~

180 lines not on this page · raw message

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

Andrew Morton

2006-01-07 00:31 UTC #1 lore raw

Quoted 8 lines · Benjamin LaHaise

The patch below converts the mm page_states counters to use local_t. mod_page_state shows up in a few profiles on x86 and x86-64 due to the disable/enable interrupts operations touching the flags register. On both my laptop (Pentium M) and P4 test box this results in about 10 additional /bin/bash -c exit 0 executions per second (P4 went from ~759/s to ~771/s). Tested on x86 and x86-64. Oh, also add a pgcow statistic for the number of COW page faults.

Bah. I think this is a better approach than the just-merged mm-page_state-opt.patch, so I should revert that patch first?

From: Nick Piggin <[email protected]>

Optimise page_state manipulations by introducing interrupt unsafe accessors to page_state fields. Callers must provide their own locking (either disable interrupts or not update from interrupt context).

Switch over the hot callsites that can easily be moved under interrupts off sections.

Signed-off-by Nick Piggin <[email protected]>

Cc Hugh Dickins <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

---

 include/linux/page-flags.h |   43 ++++++++++++----
 mm/page_alloc.c            |   89 +++++++++++++++++++----------------
 mm/rmap.c                  |   10 ++-
 mm/vmscan.c                |   27 +++++-----
 4 files changed, 104 insertions(+), 65 deletions(-)

include/linux/page-flags.h renamed from include/linux/page-flags.h~mm-page_state-opt +32 ~~-11~~

diff -puN include/linux/page-flags.h~mm-page_state-opt include/linux/page-flags.h
@@ -144,22 +144,33 @@ struct page_state {
 extern void get_page_state(struct page_state *ret);
 extern void get_page_state_node(struct page_state *ret, int node);
 extern void get_full_page_state(struct page_state *ret);
-extern unsigned long __read_page_state(unsigned long offset);
-extern void __mod_page_state(unsigned long offset, unsigned long delta);
+extern unsigned long read_page_state_offset(unsigned long offset);
+extern void mod_page_state_offset(unsigned long offset, unsigned long delta);
+extern void __mod_page_state_offset(unsigned long offset, unsigned long delta);
 
 #define read_page_state(member) \
-	__read_page_state(offsetof(struct page_state, member))
+	read_page_state_offset(offsetof(struct page_state, member))
 
 #define mod_page_state(member, delta)	\
-	__mod_page_state(offsetof(struct page_state, member), (delta))
+	mod_page_state_offset(offsetof(struct page_state, member), (delta))
 
-#define inc_page_state(member)	mod_page_state(member, 1UL)
-#define dec_page_state(member)	mod_page_state(member, 0UL - 1)
-#define add_page_state(member,delta) mod_page_state(member, (delta))
-#define sub_page_state(member,delta) mod_page_state(member, 0UL - (delta))
+#define __mod_page_state(member, delta)	\
+	__mod_page_state_offset(offsetof(struct page_state, member), (delta))
 
-#define mod_page_state_zone(zone, member, delta)			\
- do {									\
+#define inc_page_state(member)		mod_page_state(member, 1UL)
+#define dec_page_state(member)		mod_page_state(member, 0UL - 1)
+#define add_page_state(member,delta)	mod_page_state(member, (delta))
+#define sub_page_state(member,delta)	mod_page_state(member, 0UL - (delta))
+
+#define __inc_page_state(member)	__mod_page_state(member, 1UL)
+#define __dec_page_state(member)	__mod_page_state(member, 0UL - 1)
+#define __add_page_state(member,delta)	__mod_page_state(member, (delta))
+#define __sub_page_state(member,delta)	__mod_page_state(member, 0UL - (delta))
+
+#define page_state(member) (*__page_state(offsetof(struct page_state, member)))
+
+#define state_zone_offset(zone, member)					\
+({									\
 	unsigned offset;						\
 	if (is_highmem(zone))						\
 		offset = offsetof(struct page_state, member##_high);	\

@@ -169,7 +180,17 @@ extern void __mod_page_state(unsigned lo
 		offset = offsetof(struct page_state, member##_dma32);	\
 	else								\
 		offset = offsetof(struct page_state, member##_dma);	\
-	__mod_page_state(offset, (delta));				\
+	offset;								\
+})
+
+#define __mod_page_state_zone(zone, member, delta)			\
+ do {									\
+	__mod_page_state_offset(state_zone_offset(zone, member), (delta)); \
+ } while (0)
+
+#define mod_page_state_zone(zone, member, delta)			\
+ do {									\
+	mod_page_state_offset(state_zone_offset(zone, member), (delta)); \
  } while (0)
 
 /*

mm/page_alloc.c renamed from mm/page_alloc.c~mm-page_state-opt +50 ~~-39~~

178 lines not on this page · raw message

mm/rmap.c renamed from mm/rmap.c~mm-page_state-opt +7 -3

diff -puN mm/rmap.c~mm-page_state-opt mm/rmap.c
@@ -451,7 +451,11 @@ static void __page_set_anon_rmap(struct 
 
 	page->index = linear_page_index(vma, address);
 
-	inc_page_state(nr_mapped);
+	/*
+	 * nr_mapped state can be updated without turning off
+	 * interrupts because it is not modified via interrupt.
+	 */
+	__inc_page_state(nr_mapped);
 }
 
 /**

@@ -498,7 +502,7 @@ void page_add_file_rmap(struct page *pag
 	BUG_ON(!pfn_valid(page_to_pfn(page)));
 
 	if (atomic_inc_and_test(&page->_mapcount))
-		inc_page_state(nr_mapped);
+		__inc_page_state(nr_mapped);
 }
 
 /**

@@ -522,7 +526,7 @@ void page_remove_rmap(struct page *page)
 		 */
 		if (page_test_and_clear_dirty(page))
 			set_page_dirty(page);
-		dec_page_state(nr_mapped);
+		__dec_page_state(nr_mapped);
 	}
 }

mm/vmscan.c renamed from mm/vmscan.c~mm-page_state-opt +15 ~~-12~~

diff -puN mm/vmscan.c~mm-page_state-opt mm/vmscan.c
@@ -645,16 +645,17 @@ static void shrink_cache(struct zone *zo
 			goto done;
 
 		max_scan -= nr_scan;
-		if (current_is_kswapd())
-			mod_page_state_zone(zone, pgscan_kswapd, nr_scan);
-		else
-			mod_page_state_zone(zone, pgscan_direct, nr_scan);
 		nr_freed = shrink_list(&page_list, sc);
-		if (current_is_kswapd())
-			mod_page_state(kswapd_steal, nr_freed);
-		mod_page_state_zone(zone, pgsteal, nr_freed);
 
-		spin_lock_irq(&zone->lru_lock);
+		local_irq_disable();
+		if (current_is_kswapd()) {
+			__mod_page_state_zone(zone, pgscan_kswapd, nr_scan);
+			__mod_page_state(kswapd_steal, nr_freed);
+		} else
+			__mod_page_state_zone(zone, pgscan_direct, nr_scan);
+		__mod_page_state_zone(zone, pgsteal, nr_freed);
+
+		spin_lock(&zone->lru_lock);
 		/*
 		 * Put back any unfreeable pages.
 		 */

@@ -816,11 +817,13 @@ refill_inactive_zone(struct zone *zone, 
 		}
 	}
 	zone->nr_active += pgmoved;
-	spin_unlock_irq(&zone->lru_lock);
-	pagevec_release(&pvec);
+	spin_unlock(&zone->lru_lock);
+
+	__mod_page_state_zone(zone, pgrefill, pgscanned);
+	__mod_page_state(pgdeactivate, pgdeactivate);
+	local_irq_enable();
 
-	mod_page_state_zone(zone, pgrefill, pgscanned);
-	mod_page_state(pgdeactivate, pgdeactivate);
+	pagevec_release(&pvec);
 }
 
 /*

Marcelo Tosatti

2006-01-07 01:00 UTC #2 lore raw

Quoted 10 lines · Andrew Morton

Quoted 8 lines · Benjamin LaHaise
The patch below converts the mm page_states counters to use local_t. mod_page_state shows up in a few profiles on x86 and x86-64 due to the disable/enable interrupts operations touching the flags register. On both my laptop (Pentium M) and P4 test box this results in about 10 additional /bin/bash -c exit 0 executions per second (P4 went from ~759/s to ~771/s). Tested on x86 and x86-64. Oh, also add a pgcow statistic for the number of COW page faults.

Quoted 2 lines

Bah. I think this is a better approach than the just-merged mm-page_state-opt.patch, so I should revert that patch first?

Don't think so - local_t operations are performed atomically, which is not required for most hotpath page statistics operations since proper locks are already held.

What is wanted for these cases are simple inc/dec (non-atomic) instructions, which is what Nick's patch does by introducing __mod_page_state.

Ben, have you tested mm-page_state-opt.patch? It should get rid of most "flags" save/restore on stack.

Nick Piggin

2006-01-07 02:52 UTC #3 lore raw

In reply to Andrew Morton

Quoted 8 lines · Andrew Morton

Quoted 1 line · Benjamin LaHaise
The patch below converts the mm page_states counters to use local_t. mod_page_state shows up in a few profiles on x86 and x86-64 due to the disable/enable interrupts operations touching the flags register. On both my laptop (Pentium M) and P4 test box this results in about 10 additional /bin/bash -c exit 0 executions per second (P4 went from ~759/s to ~771/s). Tested on x86 and x86-64. Oh, also add a pgcow statistic for the number of COW page faults.
Bah. I think this is a better approach than the just-merged mm-page_state-opt.patch, so I should revert that patch first?

No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for local_t, and ppc64 uses 3 counters per-cpu thus tripling the cache footprint.

Nick

Signature · 9 lines

SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com 

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

Andi Kleen

2006-01-07 03:01 UTC #4 lore raw

Quoted 2 lines · Nick Piggin

No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for

well, they're just broken and need to be fixed to not do that.

Also I bet with some tricks a seqlock like setup could be made to work.

Quoted 2 lines

local_t, and ppc64 uses 3 counters per-cpu thus tripling the cache footprint.

and ppc64 has big caches so this also shouldn't be a problem.

-Andi

Nick Piggin

2006-01-07 03:19 UTC #5 lore raw

Quoted 9 lines · Andi Kleen

Quoted 2 lines · Nick Piggin
No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for
well, they're just broken and need to be fixed to not do that.

How?

Quoted 2 lines

Also I bet with some tricks a seqlock like setup could be made to work.

I asked you how before. If you can come up with a way then it indeed might be a good solution... The problem I see with seqlock is that it is only fast in the read path. That path is not the issue here.

Quoted 7 lines

Quoted 3 lines
local_t, and ppc64 uses 3 counters per-cpu thus tripling the cache footprint.
and ppc64 has big caches so this also shouldn't be a problem.

Well it is even less of a problem for them now, by about 1/3.

Performance-wise there is really no benefit for even i386 or x86-64 to move to local_t now either so I don't see what the fuss is about.

Signature · 3 lines

SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com

Andi Kleen

2006-01-07 03:25 UTC #6 lore raw

Quoted 4 lines · Nick Piggin

Quoted 3 lines · Andi Kleen
On Saturday 07 January 2006 03:52, Nick Piggin wrote:

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for
well, they're just broken and need to be fixed to not do that.
How?

If anything use the 3x duplicated data setup, not atomic operations.

Quoted 6 lines

Quoted 3 lines
Also I bet with some tricks a seqlock like setup could be made to work.
I asked you how before. If you can come up with a way then it indeed might be a good solution...

I'll try to work something up.

Quoted 2 lines

The problem I see with seqlock is that it
is only fast in the read path. That path is not the issue here.

The common case - not getting interrupted would be fast.

Quoted 5 lines

Quoted 5 lines
Quoted 3 lines
local_t, and ppc64 uses 3 counters per-cpu thus tripling the cache footprint.

Quoted 7 lines

Quoted 2 lines
and ppc64 has big caches so this also shouldn't be a problem.
Well it is even less of a problem for them now, by about 1/3.
Performance-wise there is really no benefit for even i386 or x86-64 to move to local_t now either so I don't see what the fuss is about.

Actually P4 doesn't like CLI/STI. For AMD and P-M it's not that much an issue, but NetBurst really doesn't like it.

-Andi

Nick Piggin

2006-01-07 03:48 UTC #7 lore raw

Quoted 12 lines · Andi Kleen

Quoted 10 lines · Nick Piggin
Quoted 8 lines · Andi Kleen
Quoted 2 lines · Nick Piggin
No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
well, they're just broken and need to be fixed to not do that.
How?
If anything use the 3x duplicated data setup, not atomic operations.

At a 3x cache footprint cost? (and probably more than 3x for icache, though I haven't checked) And I think hardware trends are against us. (Also, does it have race issues with nested interrupts that Andrew noticed?)

Quoted 10 lines

Quoted 6 lines
Quoted 3 lines
Also I bet with some tricks a seqlock like setup could be made to work.
I asked you how before. If you can come up with a way then it indeed might be a good solution...
I'll try to work something up.

Cool, I'd be interested to see.

Quoted 6 lines

Quoted 2 lines
The problem I see with seqlock is that it is only fast in the read path. That path is not the issue here.
The common case - not getting interrupted would be fast.

The problem is that you can never do the final store without risking a race with an interrupt. Because it is not a read-path.

The closest think I can see to a seqlock would be ll/sc operations, at which point you're back to atomic ops.

Quoted 8 lines

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
local_t, and ppc64 uses 3 counters per-cpu thus tripling the cache footprint.
and ppc64 has big caches so this also shouldn't be a problem.

Quoted 9 lines

Quoted 4 lines
Well it is even less of a problem for them now, by about 1/3.
Performance-wise there is really no benefit for even i386 or x86-64 to move to local_t now either so I don't see what the fuss is about.
Actually P4 doesn't like CLI/STI. For AMD and P-M it's not that much an issue, but NetBurst really doesn't like it.

Yes, it was worth over a second of real time and ~ 7% total kernel time on kbuild on a P4.

(git: a74609fafa2e5cc31d558012abaaa55ec9ad9da4)

AMD and PM I didn't test but the improvement might still be noticable, if much smaller.

Signature · 3 lines

SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com

Andi Kleen

2006-01-07 04:03 UTC #8 lore raw

Quoted 4 lines · Nick Piggin

At a 3x cache footprint cost? (and probably more than 3x for icache, though I haven't checked) And I think hardware trends are against us. (Also, does it have race issues with nested interrupts that Andrew noticed?)

Well the alternative would be to just let them turn off interrupts. If that's cheap for them that's fine too. And would be equivalent to what the current high level code does.

If you worry about icache footprint it can be even done out of line.

-Andi

Christoph Lameter

2006-01-09 20:54 UTC #9 lore raw

In reply to Andi Kleen

Quoted 6 lines · Andi Kleen

Quoted 2 lines · Nick Piggin
No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for
well, they're just broken and need to be fixed to not do that.

I tried to use local_t on ia64 for page statistics and have to agree with Nick. local_t has highly arch specific semantics.

Andrew Morton

2006-01-07 03:07 UTC #10 lore raw

In reply to Nick Piggin

Quoted 13 lines · Nick Piggin

Quoted 11 lines · Andrew Morton
Quoted 7 lines · Benjamin LaHaise
The patch below converts the mm page_states counters to use local_t. mod_page_state shows up in a few profiles on x86 and x86-64 due to the disable/enable interrupts operations touching the flags register. On both my laptop (Pentium M) and P4 test box this results in about 10 additional /bin/bash -c exit 0 executions per second (P4 went from ~759/s to ~771/s). Tested on x86 and x86-64. Oh, also add a pgcow statistic for the number of COW page faults.

Quoted 9 lines

Quoted 3 lines
Bah. I think this is a better approach than the just-merged mm-page_state-opt.patch, so I should revert that patch first?
No. On many load/store architectures there is no good way to do local_t, so something like ppc32 or ia64 just uses all atomic operations for local_t, and ppc64 uses 3 counters per-cpu thus tripling the cache footprint.

Yes, local_t seems a bit half-assed at present. And a bit broken with interrupt nesting.

Surely 64-bit architectures would be better off using atomic64_t rather than that v[3] monstrosity.

Benjamin LaHaise

author

2006-01-09 18:26 UTC #11 lore raw

In reply to Andrew Morton

Quoted 2 lines · Andrew Morton

Bah. I think this is a better approach than the just-merged mm-page_state-opt.patch, so I should revert that patch first?

After going over things, I think that I'll redo my patch on top of that one, as it means that architectures that can optimize away the save/restore of irq flags will be able to benefit from that. Maybe after all this is said and done we can clean things up sufficiently to be able to inline the inc/dec where it is simple enough to do so.

-ben

Signature · 8 lines

"You know, I've seen some crystals do some pretty trippy shit, man."
Don't Email: <[email protected]>.

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

Christoph Lameter

2006-01-09 20:52 UTC #12 lore raw

Quoted 9 lines · Benjamin LaHaise

Quoted 2 lines · Andrew Morton
Bah. I think this is a better approach than the just-merged mm-page_state-opt.patch, so I should revert that patch first?
After going over things, I think that I'll redo my patch on top of that one, as it means that architectures that can optimize away the save/restore of irq flags will be able to benefit from that. Maybe after all this is said and done we can clean things up sufficiently to be able to inline the inc/dec where it is simple enough to do so.

Could you also have a look at my zone counters and event counter patchsets? This will split up the page statistics into important counters that we use (which are then per zone) and the ones that are only displayed via proc). It would really help to implement numa awareness in the VM if we could have per zone vm counters. The zone reclaim patchset would already benefit from it.

Zoned counters:

http://marc.theaimsgroup.com/?l=linux-kernel&m=113511649910826&w=2

Fast event counters:

http://marc.theaimsgroup.com/?l=linux-mm&m=113512324804851&w=2