From: David Rientjes <rientjes@google.com> Date: 2012-11-20 01:44:39
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
Signed-off-by: David Rientjes <redacted>
---
include/linux/memcontrol.h | 9 ++++++++-
mm/memcontrol.c | 9 ++++-----
2 files changed, 12 insertions(+), 6 deletions(-)
@@ -181,7 +181,14 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,gfp_tgfp_mask,unsignedlong*total_scanned);-voidmem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+void__mem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+staticinlinevoidmem_cgroup_count_vm_event(structmm_struct*mm,+enumvm_event_itemidx)+{+if(mem_cgroup_disabled()||!mm)+return;+__mem_cgroup_count_vm_event(mm,idx);+}#ifdef CONFIG_TRANSPARENT_HUGEPAGEvoidmem_cgroup_split_huge_fixup(structpage*head);#endif
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
Signed-off-by: David Rientjes <redacted>
From: Michal Hocko <hidden> Date: 2012-11-20 07:07:33
On Mon 19-11-12 17:44:34, David Rientjes wrote:
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
Signed-off-by: David Rientjes <rientjes@google.com>
@@ -181,7 +181,14 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,gfp_tgfp_mask,unsignedlong*total_scanned);-voidmem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+void__mem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+staticinlinevoidmem_cgroup_count_vm_event(structmm_struct*mm,+enumvm_event_itemidx)+{+if(mem_cgroup_disabled()||!mm)+return;+__mem_cgroup_count_vm_event(mm,idx);+}#ifdef CONFIG_TRANSPARENT_HUGEPAGEvoidmem_cgroup_split_huge_fixup(structpage*head);#endif
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michal Hocko
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Glauber Costa <hidden> Date: 2012-11-20 08:34:37
On 11/20/2012 08:23 AM, Kamezawa Hiroyuki wrote:
(2012/11/20 10:44), David Rientjes wrote:
quoted
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when
memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
Signed-off-by: David Rientjes <redacted>
Acked-By: KAMEZAWA Hiroyuki <redacted>
I am fine with this as well.
Acked-by: Glauber Costa <redacted>
From: Johannes Weiner <hannes@cmpxchg.org> Date: 2012-11-20 17:27:11
On Mon, Nov 19, 2012 at 05:44:34PM -0800, David Rientjes wrote:
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
Signed-off-by: David Rientjes <redacted>
From: Andrew Morton <akpm@linux-foundation.org> Date: 2012-11-20 21:49:35
On Mon, 19 Nov 2012 17:44:34 -0800 (PST)
David Rientjes [off-list ref] wrote:
quoted hunk
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
...
@@ -181,7 +181,14 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,gfp_tgfp_mask,unsignedlong*total_scanned);-voidmem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+void__mem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+staticinlinevoidmem_cgroup_count_vm_event(structmm_struct*mm,+enumvm_event_itemidx)+{+if(mem_cgroup_disabled()||!mm)+return;+__mem_cgroup_count_vm_event(mm,idx);+}
Does the !mm case occur frequently enough to justify inlining it, or
should that test remain out-of-line?
On Mon, 19 Nov 2012 17:44:34 -0800 (PST)
David Rientjes [off-list ref] wrote:
quoted
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
...
@@ -181,7 +181,14 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,gfp_tgfp_mask,unsignedlong*total_scanned);-voidmem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+void__mem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+staticinlinevoidmem_cgroup_count_vm_event(structmm_struct*mm,+enumvm_event_itemidx)+{+if(mem_cgroup_disabled()||!mm)+return;+__mem_cgroup_count_vm_event(mm,idx);+}
Does the !mm case occur frequently enough to justify inlining it, or
should that test remain out-of-line?
From: David Rientjes <rientjes@google.com> Date: 2012-11-21 02:48:56
Move the check for !mm out of line as suggested by Andrew.
Signed-off-by: David Rientjes <rientjes@google.com>
---
include/linux/memcontrol.h | 2 +-
mm/memcontrol.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
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>
From: Michal Hocko <hidden> Date: 2012-11-21 08:35:24
On Tue 20-11-12 13:49:32, Andrew Morton wrote:
On Mon, 19 Nov 2012 17:44:34 -0800 (PST)
David Rientjes [off-list ref] wrote:
quoted
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
...
@@ -181,7 +181,14 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,gfp_tgfp_mask,unsignedlong*total_scanned);-voidmem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+void__mem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+staticinlinevoidmem_cgroup_count_vm_event(structmm_struct*mm,+enumvm_event_itemidx)+{+if(mem_cgroup_disabled()||!mm)+return;+__mem_cgroup_count_vm_event(mm,idx);+}
Does the !mm case occur frequently enough to justify inlining it, or
should that test remain out-of-line?
Now that you've asked about it I started looking around and I cannot see
how mm can ever be NULL. The condition is there since the very beginning
(456f998e memcg: add the pagefault count into memcg stats) but all the
callers are page fault handlers and those shouldn't have mm==NULL.
Or is there anything obvious I am missing?
Ying, the whole thread starts https://lkml.org/lkml/2012/11/19/545 but
the primary question is why we need !mm test for mem_cgroup_count_vm_event
at all.
Thanks!
--
Michal Hocko
SUSE Labs
On Mon, 19 Nov 2012 17:44:34 -0800 (PST)
David Rientjes [off-list ref] wrote:
quoted
While profiling numa/core v16 with cgroup_disable=memory on the command
line, I noticed mem_cgroup_count_vm_event() still showed up as high as
0.60% in perftop.
This occurs because the function is called extremely often even when memcg
is disabled.
To fix this, inline the check for mem_cgroup_disabled() so we avoid the
unnecessary function call if memcg is disabled.
...
@@ -181,7 +181,14 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,gfp_tgfp_mask,unsignedlong*total_scanned);-voidmem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+void__mem_cgroup_count_vm_event(structmm_struct*mm,enumvm_event_itemidx);+staticinlinevoidmem_cgroup_count_vm_event(structmm_struct*mm,+enumvm_event_itemidx)+{+if(mem_cgroup_disabled()||!mm)+return;+__mem_cgroup_count_vm_event(mm,idx);+}
Does the !mm case occur frequently enough to justify inlining it, or
should that test remain out-of-line?
Now that you've asked about it I started looking around and I cannot see
how mm can ever be NULL. The condition is there since the very beginning
(456f998e memcg: add the pagefault count into memcg stats) but all the
callers are page fault handlers and those shouldn't have mm==NULL.
Or is there anything obvious I am missing?
Ying, the whole thread starts https://lkml.org/lkml/2012/11/19/545 but
the primary question is why we need !mm test for mem_cgroup_count_vm_event
at all.
Here's a guess: as Ying's 456f998e patch started out in akpm's tree,
shmem.c was calling mem_cgroup_count_vm_event(current->mm, PGMAJFAULT).
Then I insisted that was inconsistent with how we usually account when
one task touches another's address space, and rearranged it to work on
vma->vm_mm instead.
Done the original way, if the touching task were a kernel daemon (KSM's
ksmd comes to my mind), then the current->mm could well have been NULL.
I agree with you that it looks redundant now.
Hugh
--
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>
Does the !mm case occur frequently enough to justify inlining it, or
should that test remain out-of-line?
Now that you've asked about it I started looking around and I cannot see
how mm can ever be NULL. The condition is there since the very beginning
(456f998e memcg: add the pagefault count into memcg stats) but all the
callers are page fault handlers and those shouldn't have mm==NULL.
Or is there anything obvious I am missing?
Ying, the whole thread starts https://lkml.org/lkml/2012/11/19/545 but
the primary question is why we need !mm test for mem_cgroup_count_vm_event
at all.
Here's a guess: as Ying's 456f998e patch started out in akpm's tree,
shmem.c was calling mem_cgroup_count_vm_event(current->mm, PGMAJFAULT).
Then I insisted that was inconsistent with how we usually account when
one task touches another's address space, and rearranged it to work on
vma->vm_mm instead.
Thanks Hugh!
Done the original way, if the touching task were a kernel daemon (KSM's
ksmd comes to my mind), then the current->mm could well have been NULL.
I agree with you that it looks redundant now.
Andrew could you please pick this up?
---
From 619b1ab26c3e96944f6c60256cf7920671bafa5b Mon Sep 17 00:00:00 2001
From: Michal Hocko <redacted>
Date: Thu, 29 Nov 2012 14:20:58 +0100
Subject: [PATCH] memcg: do not check for mm in mem_cgroup_count_vm_event
mm given to mem_cgroup_count_vm_event cannot be NULL because the
function is either called from the page fault path or vma->vm_mm is
used. So the check can be dropped.
The check has been introduced by 456f998e (memcg: add the pagefault
count into memcg stats) because the originally proposed patch used
current->mm for shmem but this has been changed to vma->vm_mm later on
without the check being removed (thanks to Hugh for this recollection).
Signed-off-by: Michal Hocko <redacted>
---
include/linux/memcontrol.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)