* [PATCH] memcg: remove unnecessary thp check at page stat accounting
@ 2012-01-19 7:14 KAMEZAWA Hiroyuki
2012-01-19 9:34 ` Michal Hocko
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: KAMEZAWA Hiroyuki @ 2012-01-19 7:14 UTC (permalink / raw)
To: linux-mm; +Cc: linux-kernel, akpm, Michal Hocko, hannes, Hugh Dickins, Ying Han
Thank you very much for reviewing previous RFC series.
This is a patch against memcg-devel and linux-next (can by applied without HUNKs).
==
>From 64641b360839b029bb353fbd95f7554cc806ed05 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date: Thu, 12 Jan 2012 16:08:33 +0900
Subject: [PATCH] memcg: remove unnecessary thp check in mem_cgroup_update_page_stat()
commit 58b318ecf(memcg-devel)
memcg: make mem_cgroup_split_huge_fixup() more efficient
removes move_lock_page_cgroup() in thp-split path.
So, We do not have to check PageTransHuge in mem_cgroup_update_page_stat
and fallback into the locked accounting because both move charge and thp
split up are done with compound_lock so they cannot race. update vs.
move is protected by the mem_cgroup_stealed sufficiently.
PageTransHuge pages shouldn't appear in this code path currently because
we are tracking only file pages at the moment but later we are planning
to track also other pages (e.g. mlocked ones).
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
mm/memcontrol.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5073474..fb2dfc3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1801,7 +1801,7 @@ void mem_cgroup_update_page_stat(struct page *page,
if (unlikely(!memcg || !PageCgroupUsed(pc)))
goto out;
/* pc->mem_cgroup is unstable ? */
- if (unlikely(mem_cgroup_stealed(memcg)) || PageTransHuge(page)) {
+ if (unlikely(mem_cgroup_stealed(memcg))) {
/* take a lock against to access pc->mem_cgroup */
move_lock_page_cgroup(pc, &flags);
need_unlock = true;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] memcg: remove unnecessary thp check at page stat accounting
2012-01-19 7:14 [PATCH] memcg: remove unnecessary thp check at page stat accounting KAMEZAWA Hiroyuki
@ 2012-01-19 9:34 ` Michal Hocko
2012-01-20 0:29 ` KAMEZAWA Hiroyuki
2012-01-20 3:25 ` [PATCH v3] " KAMEZAWA Hiroyuki
2012-01-23 20:11 ` [PATCH] " Ying Han
2 siblings, 1 reply; 8+ messages in thread
From: Michal Hocko @ 2012-01-19 9:34 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-mm, linux-kernel, akpm, hannes, Hugh Dickins, Ying Han
On Thu 19-01-12 16:14:45, KAMEZAWA Hiroyuki wrote:
> Thank you very much for reviewing previous RFC series.
> This is a patch against memcg-devel and linux-next (can by applied without HUNKs).
>
> ==
>
> From 64641b360839b029bb353fbd95f7554cc806ed05 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 12 Jan 2012 16:08:33 +0900
> Subject: [PATCH] memcg: remove unnecessary thp check in mem_cgroup_update_page_stat()
>
> commit 58b318ecf(memcg-devel)
> memcg: make mem_cgroup_split_huge_fixup() more efficient
> removes move_lock_page_cgroup() in thp-split path.
I wouldn't refer to something which will change its commit id by its
SHA. I guess the subject is sufficient. Btw. do we really need to
mention this? Is it just to make sure that this doesn't get merged
withtout the mentioned patch?
> So, We do not have to check PageTransHuge in mem_cgroup_update_page_stat
> and fallback into the locked accounting because both move charge and thp
> split up are done with compound_lock so they cannot race. update vs.
> move is protected by the mem_cgroup_stealed sufficiently.
>
> PageTransHuge pages shouldn't appear in this code path currently because
> we are tracking only file pages at the moment but later we are planning
> to track also other pages (e.g. mlocked ones).
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Other than that
Acked-by: Michal Hocko <mhocko@suse.cz>
> ---
> mm/memcontrol.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 5073474..fb2dfc3 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1801,7 +1801,7 @@ void mem_cgroup_update_page_stat(struct page *page,
> if (unlikely(!memcg || !PageCgroupUsed(pc)))
> goto out;
> /* pc->mem_cgroup is unstable ? */
> - if (unlikely(mem_cgroup_stealed(memcg)) || PageTransHuge(page)) {
> + if (unlikely(mem_cgroup_stealed(memcg))) {
> /* take a lock against to access pc->mem_cgroup */
> move_lock_page_cgroup(pc, &flags);
> need_unlock = true;
> --
> 1.7.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] memcg: remove unnecessary thp check at page stat accounting
2012-01-19 9:34 ` Michal Hocko
@ 2012-01-20 0:29 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 8+ messages in thread
From: KAMEZAWA Hiroyuki @ 2012-01-20 0:29 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-mm, linux-kernel, akpm, hannes, Hugh Dickins, Ying Han
On Thu, 19 Jan 2012 10:34:10 +0100
Michal Hocko <mhocko@suse.cz> wrote:
> On Thu 19-01-12 16:14:45, KAMEZAWA Hiroyuki wrote:
> > Thank you very much for reviewing previous RFC series.
> > This is a patch against memcg-devel and linux-next (can by applied without HUNKs).
> >
> > ==
> >
> > From 64641b360839b029bb353fbd95f7554cc806ed05 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Thu, 12 Jan 2012 16:08:33 +0900
> > Subject: [PATCH] memcg: remove unnecessary thp check in mem_cgroup_update_page_stat()
> >
> > commit 58b318ecf(memcg-devel)
> > memcg: make mem_cgroup_split_huge_fixup() more efficient
> > removes move_lock_page_cgroup() in thp-split path.
>
> I wouldn't refer to something which will change its commit id by its
> SHA. I guess the subject is sufficient.
> Btw. do we really need to
> mention this? Is it just to make sure that this doesn't get merged
> withtout the mentioned patch?
>
Hmm, ok. Yes, just informing this patch depends on that patch.
> > So, We do not have to check PageTransHuge in mem_cgroup_update_page_stat
> > and fallback into the locked accounting because both move charge and thp
> > split up are done with compound_lock so they cannot race. update vs.
> > move is protected by the mem_cgroup_stealed sufficiently.
> >
> > PageTransHuge pages shouldn't appear in this code path currently because
> > we are tracking only file pages at the moment but later we are planning
> > to track also other pages (e.g. mlocked ones).
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> Other than that
> Acked-by: Michal Hocko <mhocko@suse.cz>
>
Thank you.
-Kame
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] memcg: remove unnecessary thp check at page stat accounting
2012-01-19 7:14 [PATCH] memcg: remove unnecessary thp check at page stat accounting KAMEZAWA Hiroyuki
2012-01-19 9:34 ` Michal Hocko
@ 2012-01-20 3:25 ` KAMEZAWA Hiroyuki
2012-01-20 8:43 ` Michal Hocko
2012-01-23 20:11 ` [PATCH] " Ying Han
2 siblings, 1 reply; 8+ messages in thread
From: KAMEZAWA Hiroyuki @ 2012-01-20 3:25 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-mm, linux-kernel, akpm, Michal Hocko, hannes, Hugh Dickins,
Ying Han
Updated description.
==
>From a6395205d9f517af7963ff61d66efbcf1c64b2a5 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Date: Thu, 12 Jan 2012 16:08:33 +0900
Subject: [PATCH v3] memcg: remove unnecessary check in mem_cgroup_update_page_stat()
This patch is a fix for
memcg: make mem_cgroup_split_huge_fixup() more efficient
Above patch removes move_lock_page_cgroup(). So, we do not have
to check PageTransHuge in mem_cgroup_update_page_stat and fallback into
the locked accounting because both move_account and thp split are done
with compound_lock so they cannot race.
The race between update vs. move is protected by mem_cgroup_stealed,
PageTransHuge pages shouldn't appear in this code path currently because
we are tracking only file pages at the moment but later we are planning
to track also other pages (e.g. mlocked ones).
Changelog:
- updated description.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
mm/memcontrol.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3dbff4d..ff24520 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1867,7 +1867,7 @@ void mem_cgroup_update_page_stat(struct page *page,
if (unlikely(!memcg || !PageCgroupUsed(pc)))
goto out;
/* pc->mem_cgroup is unstable ? */
- if (unlikely(mem_cgroup_stealed(memcg)) || PageTransHuge(page)) {
+ if (unlikely(mem_cgroup_stealed(memcg))) {
/* take a lock against to access pc->mem_cgroup */
move_lock_page_cgroup(pc, &flags);
need_unlock = true;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] memcg: remove unnecessary thp check at page stat accounting
2012-01-20 3:25 ` [PATCH v3] " KAMEZAWA Hiroyuki
@ 2012-01-20 8:43 ` Michal Hocko
0 siblings, 0 replies; 8+ messages in thread
From: Michal Hocko @ 2012-01-20 8:43 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-mm, linux-kernel, akpm, hannes, Hugh Dickins, Ying Han
On Fri 20-01-12 12:25:12, KAMEZAWA Hiroyuki wrote:
> Updated description.
> ==
> From a6395205d9f517af7963ff61d66efbcf1c64b2a5 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 12 Jan 2012 16:08:33 +0900
> Subject: [PATCH v3] memcg: remove unnecessary check in mem_cgroup_update_page_stat()
>
> This patch is a fix for
I would rather call it a follow up for this patch. It doesn't fixes it.
> memcg: make mem_cgroup_split_huge_fixup() more efficient
>
> Above patch removes move_lock_page_cgroup(). So, we do not have
> to check PageTransHuge in mem_cgroup_update_page_stat and fallback into
> the locked accounting because both move_account and thp split are done
> with compound_lock so they cannot race.
> The race between update vs. move is protected by mem_cgroup_stealed,
>
> PageTransHuge pages shouldn't appear in this code path currently because
> we are tracking only file pages at the moment but later we are planning
> to track also other pages (e.g. mlocked ones).
>
> Changelog:
> - updated description.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Anyway
Acked-by: Michal Hocko <mhocko@suse.cz>
Thanks
> ---
> mm/memcontrol.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 3dbff4d..ff24520 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1867,7 +1867,7 @@ void mem_cgroup_update_page_stat(struct page *page,
> if (unlikely(!memcg || !PageCgroupUsed(pc)))
> goto out;
> /* pc->mem_cgroup is unstable ? */
> - if (unlikely(mem_cgroup_stealed(memcg)) || PageTransHuge(page)) {
> + if (unlikely(mem_cgroup_stealed(memcg))) {
> /* take a lock against to access pc->mem_cgroup */
> move_lock_page_cgroup(pc, &flags);
> need_unlock = true;
> --
> 1.7.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] memcg: remove unnecessary thp check at page stat accounting
2012-01-19 7:14 [PATCH] memcg: remove unnecessary thp check at page stat accounting KAMEZAWA Hiroyuki
2012-01-19 9:34 ` Michal Hocko
2012-01-20 3:25 ` [PATCH v3] " KAMEZAWA Hiroyuki
@ 2012-01-23 20:11 ` Ying Han
2012-01-24 8:41 ` Johannes Weiner
2 siblings, 1 reply; 8+ messages in thread
From: Ying Han @ 2012-01-23 20:11 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-mm, linux-kernel, akpm, Michal Hocko, hannes, Hugh Dickins
On Wed, Jan 18, 2012 at 11:14 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> Thank you very much for reviewing previous RFC series.
> This is a patch against memcg-devel and linux-next (can by applied without HUNKs).
>
> ==
>
> From 64641b360839b029bb353fbd95f7554cc806ed05 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Date: Thu, 12 Jan 2012 16:08:33 +0900
> Subject: [PATCH] memcg: remove unnecessary thp check in mem_cgroup_update_page_stat()
>
> commit 58b318ecf(memcg-devel)
> memcg: make mem_cgroup_split_huge_fixup() more efficient
> removes move_lock_page_cgroup() in thp-split path.
>
> So, We do not have to check PageTransHuge in mem_cgroup_update_page_stat
> and fallback into the locked accounting because both move charge and thp
> split up are done with compound_lock so they cannot race. update vs.
> move is protected by the mem_cgroup_stealed sufficiently.
Sorry, i don't see we changed the "move charge" to "move account" ?
--Ying
>
> PageTransHuge pages shouldn't appear in this code path currently because
> we are tracking only file pages at the moment but later we are planning
> to track also other pages (e.g. mlocked ones).
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> mm/memcontrol.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 5073474..fb2dfc3 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1801,7 +1801,7 @@ void mem_cgroup_update_page_stat(struct page *page,
> if (unlikely(!memcg || !PageCgroupUsed(pc)))
> goto out;
> /* pc->mem_cgroup is unstable ? */
> - if (unlikely(mem_cgroup_stealed(memcg)) || PageTransHuge(page)) {
> + if (unlikely(mem_cgroup_stealed(memcg))) {
> /* take a lock against to access pc->mem_cgroup */
> move_lock_page_cgroup(pc, &flags);
> need_unlock = true;
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] memcg: remove unnecessary thp check at page stat accounting
2012-01-23 20:11 ` [PATCH] " Ying Han
@ 2012-01-24 8:41 ` Johannes Weiner
2012-01-25 18:54 ` Ying Han
0 siblings, 1 reply; 8+ messages in thread
From: Johannes Weiner @ 2012-01-24 8:41 UTC (permalink / raw)
To: Ying Han
Cc: KAMEZAWA Hiroyuki, linux-mm, linux-kernel, akpm, Michal Hocko,
Hugh Dickins
On Mon, Jan 23, 2012 at 12:11:11PM -0800, Ying Han wrote:
> On Wed, Jan 18, 2012 at 11:14 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > Thank you very much for reviewing previous RFC series.
> > This is a patch against memcg-devel and linux-next (can by applied without HUNKs).
> >
> > ==
> >
> > From 64641b360839b029bb353fbd95f7554cc806ed05 Mon Sep 17 00:00:00 2001
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Date: Thu, 12 Jan 2012 16:08:33 +0900
> > Subject: [PATCH] memcg: remove unnecessary thp check in mem_cgroup_update_page_stat()
> >
> > commit 58b318ecf(memcg-devel)
> > memcg: make mem_cgroup_split_huge_fixup() more efficient
> > removes move_lock_page_cgroup() in thp-split path.
> >
> > So, We do not have to check PageTransHuge in mem_cgroup_update_page_stat
> > and fallback into the locked accounting because both move charge and thp
> > split up are done with compound_lock so they cannot race. update vs.
> > move is protected by the mem_cgroup_stealed sufficiently.
>
> Sorry, i don't see we changed the "move charge" to "move account" ?
move_account() moves charges. IMO, it's the function that is a
misnomer and "moving charges" is less ambiguous since we account
several different things in the memory controller.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] memcg: remove unnecessary thp check at page stat accounting
2012-01-24 8:41 ` Johannes Weiner
@ 2012-01-25 18:54 ` Ying Han
0 siblings, 0 replies; 8+ messages in thread
From: Ying Han @ 2012-01-25 18:54 UTC (permalink / raw)
To: Johannes Weiner
Cc: KAMEZAWA Hiroyuki, linux-mm, linux-kernel, akpm, Michal Hocko,
Hugh Dickins
On Tue, Jan 24, 2012 at 12:41 AM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> On Mon, Jan 23, 2012 at 12:11:11PM -0800, Ying Han wrote:
>> On Wed, Jan 18, 2012 at 11:14 PM, KAMEZAWA Hiroyuki
>> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> > Thank you very much for reviewing previous RFC series.
>> > This is a patch against memcg-devel and linux-next (can by applied without HUNKs).
>> >
>> > ==
>> >
>> > From 64641b360839b029bb353fbd95f7554cc806ed05 Mon Sep 17 00:00:00 2001
>> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> > Date: Thu, 12 Jan 2012 16:08:33 +0900
>> > Subject: [PATCH] memcg: remove unnecessary thp check in mem_cgroup_update_page_stat()
>> >
>> > commit 58b318ecf(memcg-devel)
>> > memcg: make mem_cgroup_split_huge_fixup() more efficient
>> > removes move_lock_page_cgroup() in thp-split path.
>> >
>> > So, We do not have to check PageTransHuge in mem_cgroup_update_page_stat
>> > and fallback into the locked accounting because both move charge and thp
>> > split up are done with compound_lock so they cannot race. update vs.
>> > move is protected by the mem_cgroup_stealed sufficiently.
>>
>> Sorry, i don't see we changed the "move charge" to "move account" ?
>
> move_account() moves charges. IMO, it's the function that is a
> misnomer and "moving charges" is less ambiguous since we account
> several different things in the memory controller.
Hmm, that works then.
Acked-by: Ying Han<yinghan@google.com>
--Ying
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-01-25 18:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-19 7:14 [PATCH] memcg: remove unnecessary thp check at page stat accounting KAMEZAWA Hiroyuki
2012-01-19 9:34 ` Michal Hocko
2012-01-20 0:29 ` KAMEZAWA Hiroyuki
2012-01-20 3:25 ` [PATCH v3] " KAMEZAWA Hiroyuki
2012-01-20 8:43 ` Michal Hocko
2012-01-23 20:11 ` [PATCH] " Ying Han
2012-01-24 8:41 ` Johannes Weiner
2012-01-25 18:54 ` Ying Han
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).