sync_mm_rss() issues · kernelthread

David Howells

author

2010-03-08 14:00 UTC #0 lore raw

There are a couple of issues with sync_mm_rss(), as added by patch:

	commit 34e55232e59f7b19050267a05ff1226e5cd122a5
	Author: KAMEZAWA Hiroyuki <[email protected]>
	Date:   Fri Mar 5 13:41:40 2010 -0800
	Subject: mm: avoid false sharing of mm_counter

(1) You haven't implemented it for NOMMU mode. What's the right way to do this? Just give an empty function?

(2) linux/mm.h should carry the empty function as an inline when CONFIG_SPLIT_RSS_COUNTING=n, rather than it being defined as an empty function in mm/memory.c.

David

--
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>

KAMEZAWA Hiroyuki

2010-03-08 23:53 UTC #1 lore raw

Quoted 11 lines · On Mon, 08 Mar 2010 13:59:56 +0000 David Howells <[email protected]> wrote:

There are a couple of issues with sync_mm_rss(), as added by patch:
	commit 34e55232e59f7b19050267a05ff1226e5cd122a5
	Author: KAMEZAWA Hiroyuki <[email protected]>
	Date:   Fri Mar 5 13:41:40 2010 -0800
	Subject: mm: avoid false sharing of mm_counter
(1) You haven't implemented it for NOMMU mode. What's the right way to do this? Just give an empty function?

Ah, sorry. I'll prepare empty function immediately. But I have no NOMMU enviroment...

Quoted 4 lines

(2) linux/mm.h should carry the empty function as an inline when CONFIG_SPLIT_RSS_COUNTING=n, rather than it being defined as an empty function in mm/memory.c.

ok.

please wait..

Sorry,
-Kame

--
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>

KAMEZAWA Hiroyuki

2010-03-09 01:02 UTC #2 lore raw

PATCH 1/1 fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues

In reply to David Howells

David-san, could you check this ?
==
From: KAMEZAWA Hiroyuki <[email protected]>

Fix breakage in NOMMU build

commit 34e55232e59f7b19050267a05ff1226e5cd122a5 added sync_mm_rss() for syncing loosely accounted rss counters. It's for CONFIG_MMU but sync_mm_rss is called even in NOMMU enviroment (kerne/exit.c, fs/exec.c). Above commit doesn't handle it well.

This patch changes
SPLIT_RSS_COUNTING depends on SPLIT_PTLOCKS && CONFIG_MMU

And for avoid unnecessary function calls, sync_mm_rss changed to be inlined noop function in header file.

Reported-by David Howells <[email protected]>

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

---
 include/linux/mm.h       |    6 ++++++
 include/linux/mm_types.h |    2 +-
 mm/memory.c              |    3 ---
 3 files changed, 7 insertions(+), 4 deletions(-)

include/linux/mm.h +6 -0

Index: mmotm-2.6.33-Mar5/include/linux/mm.h
===================================================================
@@ -974,7 +974,13 @@ static inline void setmax_mm_hiwater_rss
 		*maxrss = hiwater_rss;
 }
 
+#if defined(SPLIT_RSS_COUNTING)
 void sync_mm_rss(struct task_struct *task, struct mm_struct *mm);
+#else
+static inline void sync_mm_rss(struct task_struct *task, struct mm_struct *mm)
+{
+}
+#endif
 
 /*
  * A callback you can register to apply pressure to ageable caches.

mm/memory.c +0 -3

Index: mmotm-2.6.33-Mar5/mm/memory.c
===================================================================
@@ -190,9 +190,6 @@ static void check_sync_rss_stat(struct t
 {
 }
 
-void sync_mm_rss(struct task_struct *task, struct mm_struct *mm)
-{
-}
 #endif
 
 /*

include/linux/mm_types.h +1 -1

Index: mmotm-2.6.33-Mar5/include/linux/mm_types.h
===================================================================
@@ -203,7 +203,7 @@ enum {
 	NR_MM_COUNTERS
 };
 
-#if USE_SPLIT_PTLOCKS
+#if USE_SPLIT_PTLOCKS && defined(CONFIG_MMU)
 #define SPLIT_RSS_COUNTING
 struct mm_rss_stat {
 	atomic_long_t count[NR_MM_COUNTERS];

--
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>

Mike Frysinger

2010-03-09 02:33 UTC #3 lore raw

Re: [BUGFIX][PATCH] fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues

Quoted 11 lines · KAMEZAWA Hiroyuki

David-san, could you check this ?
==
From: KAMEZAWA Hiroyuki <[email protected]>
Fix breakage in NOMMU build
commit 34e55232e59f7b19050267a05ff1226e5cd122a5 added sync_mm_rss() for syncing loosely accounted rss counters. It's for CONFIG_MMU but sync_mm_rss is called even in NOMMU enviroment (kerne/exit.c, fs/exec.c). Above commit doesn't handle it well.

Quoted 5 lines

This patch changes
SPLIT_RSS_COUNTING depends on SPLIT_PTLOCKS && CONFIG_MMU
And for avoid unnecessary function calls, sync_mm_rss changed to be inlined noop function in header file.

fixes Blackfin systems ...

Signed-off-by Mike Frysinger <[email protected]>

-mike

--
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>

Michal Simek

2010-03-09 11:49 UTC #4 lore raw

Re: [BUGFIX][PATCH] fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues

Quoted 15 lines · Mike Frysinger

Quoted 14 lines · KAMEZAWA Hiroyuki
David-san, could you check this ?
==
From: KAMEZAWA Hiroyuki <[email protected]>
Fix breakage in NOMMU build
commit 34e55232e59f7b19050267a05ff1226e5cd122a5 added sync_mm_rss() for syncing loosely accounted rss counters. It's for CONFIG_MMU but sync_mm_rss is called even in NOMMU enviroment (kerne/exit.c, fs/exec.c). Above commit doesn't handle it well.
This patch changes
SPLIT_RSS_COUNTING depends on SPLIT_PTLOCKS && CONFIG_MMU

Quoted 6 lines

Quoted 2 lines
And for avoid unnecessary function calls, sync_mm_rss changed to be inlined noop function in header file.
fixes Blackfin systems ...
Signed-off-by Mike Frysinger <[email protected]>

fixes Microblaze noMMU systems too.

Signed-off-by Michal Simek <[email protected]>

You can check Microblaze noMMU system via my site
http://www.monstr.eu/wiki/doku.php?id=log:log

Will be good to add your patches to linux-next before Linus tree.

Michal

Quoted 6 lines

-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 9 lines

Michal Simek, Ing. (M.Eng)
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663

--
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

2010-03-09 15:41 UTC #5 lore raw

Re: [BUGFIX][PATCH] fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues

Quoted 2 lines · Michal Simek

Quoted 1 line
Signed-off-by Mike Frysinger <[email protected]>
Signed-off-by Michal Simek <[email protected]>

?? You handled this patch?

--
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>

Mike Frysinger

2010-03-09 15:43 UTC #6 lore raw

Re: [BUGFIX][PATCH] fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues

Quoted 5 lines · Christoph Lameter

Quoted 2 lines · Michal Simek
Quoted 1 line
Signed-off-by Mike Frysinger <[email protected]>
Signed-off-by Michal Simek <[email protected]>
?? You handled this patch?

pick out the tags from the original patch
-mike

--
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>