reducing page_cgroup size

RFC PATCH In review KAMEZAWA Hiroyuki 2 participants 12 messages Mar 28, 2012 – Apr 18, 2012

Subsystems (4)

Memory management MM core Page allocator memcg

lore

Share & tools

lore

KAMEZAWA Hiroyuki

author

2012-03-28 10:46 UTC #0 lore raw

v2 cover reducing page_cgroup size

Hi, here is v2 (still RFC again because I changed many parts.) I'd like to post v3 without 'RFC' tag after Lsf-MM Summit.

This series is for reducing size of 'struct page_cgroup' to 8 bytes. v2 contains 6 patches and did clean-ups and fixes race in v1 and adds a trial to integrate struct page_cgroup into struct page.

each patches are...

1/6 ....add methods to access pc->mem_cgroup
2/6 ....add pc_set_mem_cgroup_and_flags() to set ->mem_cgroup and flags by one call. 3/6 ....add PageCgroupReset() for handling a race.
4/6 ....reduce size of struct page_cgroup by removing ->mem_cgroup
5/6 ....remove unnecessary memory barrier
6/6 ....add CONFIG_INTEGRATED_PAGE_CGROUP to place page_cgroup in struct page.

Regards,
-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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

KAMEZAWA Hiroyuki

author

2012-03-28 10:49 UTC #1 lore raw

RFC PATCH v2 1/6 memcg: add methods to access pc->mem_cgroup

In order to encode pc->mem_cgroup and pc->flags to be in a word, access function to pc->mem_cgroup is required.

This patch replaces access to pc->mem_cgroup with
pc_to_mem_cgroup(pc) : pc->mem_cgroup
pc_set_mem_cgroup(pc, memcg) : pc->mem_cgroup = memcg

Changelog:
- rebased onto linux-next-Mar 27 2012 handle THP move_page

Acked-by Michal Hocko <[email protected]>

Acked-by Glauber Costa <[email protected]>

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

---
 include/linux/page_cgroup.h |   12 +++++++
 mm/memcontrol.c             |   71 ++++++++++++++++++++++--------------------
 2 files changed, 49 insertions(+), 34 deletions(-)

include/linux/page_cgroup.h +12 -0

index a88cdba..92768cb 100644
@@ -82,6 +82,18 @@ static inline void unlock_page_cgroup(struct page_cgroup *pc)
 	bit_spin_unlock(PCG_LOCK, &pc->flags);
 }
 
+
+static inline struct mem_cgroup* pc_to_mem_cgroup(struct page_cgroup *pc)
+{
+	return pc->mem_cgroup;
+}
+
+static inline void
+pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
+{
+	pc->mem_cgroup = memcg;
+}
+
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct page_cgroup;

mm/memcontrol.c +37 ~~-34~~

234 lines not on this page · raw message

Signature · 9 lines

1.7.4.1


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

KAMEZAWA Hiroyuki

author

2012-03-28 10:53 UTC #2 lore raw

RFC PATCH v2 2/6 memcg: add pc_set_mem_cgroup_and_flags()

In reply to KAMEZAWA Hiroyuki

Consolidate a code for setting pc->mem_cgroup and USED bit which requires smp_wmb(). And remove a macro PCGF_NOCOPY_AT_SPLIT which isn't helpful to read code, now.

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

---
 include/linux/page_cgroup.h |   18 ++++++++++++++++++
 mm/memcontrol.c             |   18 ++++--------------
 2 files changed, 22 insertions(+), 14 deletions(-)

include/linux/page_cgroup.h +18 -0

index 92768cb..2707809 100644
@@ -1,6 +1,8 @@
 #ifndef __LINUX_PAGE_CGROUP_H
 #define __LINUX_PAGE_CGROUP_H
 
+#include <linux/smp.h>
+
 enum {
 	/* flags for mem_cgroup */
 	PCG_LOCK,  /* Lock for pc->mem_cgroup and following bits. */

@@ -94,6 +96,22 @@ pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
 	pc->mem_cgroup = memcg;
 }
 
+static inline void
+pc_set_mem_cgroup_and_flags(struct page_cgroup *pc, struct mem_cgroup *memcg,
+			unsigned long flags)
+{
+	pc->mem_cgroup = memcg;
+	/*
+	 * We access a page_cgroup asynchronously without lock_page_cgroup().
+	 * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
+	 * is accessed after testing USED bit. To make pc's mem_cgroup visible
+	 * before USED bit, we need memory barrier here.
+	 * See mem_cgroup_add_lru_list(), etc.
+	 */
+	smp_wmb();
+	pc->flags = flags;
+}
+
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct page_cgroup;

mm/memcontrol.c +4 ~~-14~~

index 8077460..d366b60 100644
@@ -2511,16 +2511,7 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
 		}
 	}
 
-	pc_set_mem_cgroup(pc, memcg);
-	/*
-	 * We access a page_cgroup asynchronously without lock_page_cgroup().
-	 * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
-	 * is accessed after testing USED bit. To make pc's mem_cgroup visible
-	 * before USED bit, we need memory barrier here.
-	 * See mem_cgroup_add_lru_list(), etc.
- 	 */
-	smp_wmb();
-	SetPageCgroupUsed(pc);
+	pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED) | BIT(PCG_LOCK));
 
 	if (lrucare) {
 		if (was_on_lru) {

@@ -2549,7 +2540,6 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-#define PCGF_NOCOPY_AT_SPLIT ((1 << PCG_LOCK) | (1 << PCG_MIGRATION))
 /*
  * Because tail pages are not marked as "used", set it. We're under
  * zone->lru_lock, 'splitting on pmd' and compound_lock.

@@ -2565,11 +2555,11 @@ void mem_cgroup_split_huge_fixup(struct page *head)
 
 	if (mem_cgroup_disabled())
 		return;
+	if (!PageCgroupUsed(head_pc))
+		return;
 	for (i = 1; i < HPAGE_PMD_NR; i++) {
 		pc = head_pc + i;
-		pc_set_mem_cgroup(pc, memcg);
-		smp_wmb();/* see __commit_charge() */
-		pc->flags = head_pc->flags & ~PCGF_NOCOPY_AT_SPLIT;
+		pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED));
 	}
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */

Signature · 9 lines

1.7.4.1


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

Ying Han

2012-04-17 21:17 UTC #3 lore raw

Re: [RFC][PATCH 2/6] memcg: add pc_set_mem_cgroup_and_flags()

Quoted 53 lines · KAMEZAWA Hiroyuki

Consolidate a code for setting pc->mem_cgroup and USED bit which requires smp_wmb(). And remove a macro PCGF_NOCOPY_AT_SPLIT which isn't helpful to read code, now.

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

---
include/linux/page_cgroup.h | 18 ++++++++++++++++++ mm/memcontrol.c | 18 ++++--------------

 2 files changed, 22 insertions(+), 14 deletions(-)

include/linux/page_cgroup.h +2 -0

index 92768cb..2707809 100644
@@ -1,6 +1,8 @@
  #ifndef __LINUX_PAGE_CGROUP_H
  #define __LINUX_PAGE_CGROUP_H
 
+#include <linux/smp.h>
+
  enum {
        /* flags for mem_cgroup */

PCG_LOCK, /* Lock for pc->mem_cgroup and following bits. */

include/linux/page_cgroup.h +16 -0

@@ -94,6 +96,22 @@ pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
        pc->mem_cgroup = memcg;
  }
 
+static inline void
+pc_set_mem_cgroup_and_flags(struct page_cgroup *pc, struct mem_cgroup *memcg,
+                       unsigned long flags)
+{
+       pc->mem_cgroup = memcg;
+       /*
+        * We access a page_cgroup asynchronously without lock_page_cgroup().
+        * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
+        * is accessed after testing USED bit. To make pc's mem_cgroup visible
+        * before USED bit, we need memory barrier here.
+        * See mem_cgroup_add_lru_list(), etc.
+        */
+       smp_wmb();
+       pc->flags = flags;
+}
+
  #else /* CONFIG_CGROUP_MEM_RES_CTLR */
  struct page_cgroup;

mm/memcontrol.c +0 -0

index 8077460..d366b60 100644
@@ -2511,16 +2511,7 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
                }
        }

mm/memcontrol.c static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, · line 2523

Quoted 10 lines

mm/memcontrol.c +0 ~~-10~~

-       pc_set_mem_cgroup(pc, memcg);
-       /*
-        * We access a page_cgroup asynchronously without lock_page_cgroup().
-        * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
-        * is accessed after testing USED bit. To make pc's mem_cgroup visible
-        * before USED bit, we need memory barrier here.
-        * See mem_cgroup_add_lru_list(), etc.
-        */
-       smp_wmb();
-       SetPageCgroupUsed(pc);

I might be confused. We removed this SetPageCgroupUsed() but not adding it back elsewhere ?

--Ying

Quoted 28 lines

mm/memcontrol.c +4 -5

+       pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED) | BIT(PCG_LOCK));
 
        if (lrucare) {
                if (was_on_lru) {

@@ -2549,7 +2540,6 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
 
  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-#define PCGF_NOCOPY_AT_SPLIT ((1 << PCG_LOCK) | (1 << PCG_MIGRATION))
  /*
  * Because tail pages are not marked as "used", set it. We're under
  * zone->lru_lock, 'splitting on pmd' and compound_lock.

@@ -2565,11 +2555,11 @@ void mem_cgroup_split_huge_fixup(struct page *head)
 
        if (mem_cgroup_disabled())
                return;
+       if (!PageCgroupUsed(head_pc))
+               return;
        for (i = 1; i < HPAGE_PMD_NR; i++) {
                pc = head_pc + i;
-               pc_set_mem_cgroup(pc, memcg);
-               smp_wmb();/* see __commit_charge() */
-               pc->flags = head_pc->flags & ~PCGF_NOCOPY_AT_SPLIT;
+               pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED));
        }
  }
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
--

Quoted 3 lines

1.7.4.1

KAMEZAWA Hiroyuki

author

2012-04-18 07:25 UTC #4 lore raw

Re: [RFC][PATCH 2/6] memcg: add pc_set_mem_cgroup_and_flags()

Quoted 69 lines · Ying Han

Quoted 53 lines · KAMEZAWA Hiroyuki

Consolidate a code for setting pc->mem_cgroup and USED bit which requires smp_wmb(). And remove a macro PCGF_NOCOPY_AT_SPLIT which isn't helpful to read code, now.

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

---
 include/linux/page_cgroup.h |   18 ++++++++++++++++++
 mm/memcontrol.c             |   18 ++++--------------
 2 files changed, 22 insertions(+), 14 deletions(-)

include/linux/page_cgroup.h +18 -0

index 92768cb..2707809 100644
@@ -1,6 +1,8 @@
 #ifndef __LINUX_PAGE_CGROUP_H
 #define __LINUX_PAGE_CGROUP_H
 
+#include <linux/smp.h>
+
 enum {
       /* flags for mem_cgroup */
       PCG_LOCK,  /* Lock for pc->mem_cgroup and following bits. */

@@ -94,6 +96,22 @@ pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
       pc->mem_cgroup = memcg;
 }
 
+static inline void
+pc_set_mem_cgroup_and_flags(struct page_cgroup *pc, struct mem_cgroup *memcg,
+                       unsigned long flags)
+{
+       pc->mem_cgroup = memcg;
+       /*
+        * We access a page_cgroup asynchronously without lock_page_cgroup().
+        * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
+        * is accessed after testing USED bit. To make pc's mem_cgroup visible
+        * before USED bit, we need memory barrier here.
+        * See mem_cgroup_add_lru_list(), etc.
+        */
+       smp_wmb();
+       pc->flags = flags;
+}
+
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct page_cgroup;

mm/memcontrol.c +0 -0

index 8077460..d366b60 100644
@@ -2511,16 +2511,7 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
               }
       }

mm/memcontrol.c static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, · line 2523

Quoted 10 lines

mm/memcontrol.c +0 ~~-10~~

-       pc_set_mem_cgroup(pc, memcg);
-       /*
-        * We access a page_cgroup asynchronously without lock_page_cgroup().
-        * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
-        * is accessed after testing USED bit. To make pc's mem_cgroup visible
-        * before USED bit, we need memory barrier here.
-        * See mem_cgroup_add_lru_list(), etc.
-        */
-       smp_wmb();
-       SetPageCgroupUsed(pc);

I might be confused. We removed this SetPageCgroupUsed() but not adding it back elsewhere ?

Quoted 3 lines

--Ying

Quoted 1 line

mm/memcontrol.c +1 -0

+       pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED) | BIT(PCG_LOCK));

Added here. This sets

| memcg pointer | Used | Locked |

Thanks,
-Kame

Ying Han

2012-04-18 18:18 UTC #5 lore raw

Re: [RFC][PATCH 2/6] memcg: add pc_set_mem_cgroup_and_flags()

Quoted 71 lines · KAMEZAWA Hiroyuki

Quoted 69 lines · Ying Han

Quoted 53 lines · KAMEZAWA Hiroyuki

Consolidate a code for setting pc->mem_cgroup and USED bit which requires smp_wmb(). And remove a macro PCGF_NOCOPY_AT_SPLIT which isn't helpful to read code, now.

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

---
include/linux/page_cgroup.h | 18 ++++++++++++++++++ mm/memcontrol.c | 18 ++++--------------

 2 files changed, 22 insertions(+), 14 deletions(-)

include/linux/page_cgroup.h +2 -0

index 92768cb..2707809 100644
@@ -1,6 +1,8 @@
  #ifndef __LINUX_PAGE_CGROUP_H
  #define __LINUX_PAGE_CGROUP_H
 
+#include <linux/smp.h>
+
  enum {
        /* flags for mem_cgroup */

PCG_LOCK, /* Lock for pc->mem_cgroup and following bits. */

include/linux/page_cgroup.h +16 -0

@@ -94,6 +96,22 @@ pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
        pc->mem_cgroup = memcg;
  }
 
+static inline void
+pc_set_mem_cgroup_and_flags(struct page_cgroup *pc, struct mem_cgroup *memcg,
+                       unsigned long flags)
+{
+       pc->mem_cgroup = memcg;
+       /*
+        * We access a page_cgroup asynchronously without lock_page_cgroup().
+        * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
+        * is accessed after testing USED bit. To make pc's mem_cgroup visible
+        * before USED bit, we need memory barrier here.
+        * See mem_cgroup_add_lru_list(), etc.
+        */
+       smp_wmb();
+       pc->flags = flags;
+}
+
  #else /* CONFIG_CGROUP_MEM_RES_CTLR */
  struct page_cgroup;

mm/memcontrol.c +0 -0

index 8077460..d366b60 100644
@@ -2511,16 +2511,7 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
                }
        }

mm/memcontrol.c static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, · line 2523

Quoted 10 lines

mm/memcontrol.c +0 ~~-10~~

-       pc_set_mem_cgroup(pc, memcg);
-       /*
-        * We access a page_cgroup asynchronously without lock_page_cgroup().
-        * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
-        * is accessed after testing USED bit. To make pc's mem_cgroup visible
-        * before USED bit, we need memory barrier here.
-        * See mem_cgroup_add_lru_list(), etc.
-        */
-       smp_wmb();
-       SetPageCgroupUsed(pc);

I might be confused. We removed this SetPageCgroupUsed() but not adding it back elsewhere ?

Quoted 8 lines

Quoted 3 lines
--Ying
Quoted 1 line
mm/memcontrol.c +1 -0
+       pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED) | BIT(PCG_LOCK));
Added here. This sets
| memcg pointer | Used | Locked |

Ah. I missed that part.

Thanks

--Ying

Quoted 8 lines

Thanks,
-Kame

KAMEZAWA Hiroyuki

author

2012-03-28 10:59 UTC #6 lore raw

RFC PATCH v2 3/6 memcg: add PageCgroupReset()

In reply to KAMEZAWA Hiroyuki

A commit "memcg: simplify LRU handling by new rule" removes PCG_ACCT_LRU. and the bug introduced by it was fixed by "memcg: fix GPF when cgroup removal races with last exit"

This was for reducing flags on pc->flags....Now, we have 3bits of flags. but this patch adds a new flag, I'm sorry. (Considering alignment of kmalloc(), we'll able to have 5 bits..)

This patch adds PCG_RESET which is similar to PCG_ACCT_LRU. This is set when mem_cgroup_add_lru_list() finds we cannot trust the pc's mem_cgroup.

The reason why this patch adds a (renamed) flag again is for merging pc->flags and pc->mem_cgroup. Assume pc's mem_cgroup is encoded as

	mem_cgroup = pc->flags & ~0x7

Updating multiple bits of pc->flags without talking lock_page_cgroup() is very dangerous. And mem_cgroup_add_lru_list() updates pc->mem_cgroup without taking lock. Then I add RESET bit. After this, pc_to_mem_cgroup() is written as

	if (PageCgroupReset(pc))
		return root_mem_cgroup;
	return pc->mem_cgroup;

This update of Reset bit can be done in atomic by set_bit(). And cleared when USED bit is set.

Considering kmalloc()'s alignment, having 4bits of flags will be ok....

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

---
 include/linux/page_cgroup.h |   15 ++++++++-------
 mm/memcontrol.c             |    5 +++--
 2 files changed, 11 insertions(+), 9 deletions(-)

include/linux/page_cgroup.h +8 -7

index 2707809..3f3b4ff 100644
@@ -8,6 +8,7 @@ enum {
 	PCG_LOCK,  /* Lock for pc->mem_cgroup and following bits. */
 	PCG_USED, /* this object is in use. */
 	PCG_MIGRATION, /* under page migration */
+	PCG_RESET,     /* have been reset to root_mem_cgroup */
 	__NR_PCG_FLAGS,
 };

@@ -70,6 +71,9 @@ SETPCGFLAG(Migration, MIGRATION)
 CLEARPCGFLAG(Migration, MIGRATION)
 TESTPCGFLAG(Migration, MIGRATION)
 
+TESTPCGFLAG(Reset, RESET)
+SETPCGFLAG(Reset, RESET)
+
 static inline void lock_page_cgroup(struct page_cgroup *pc)
 {
 	/*

@@ -84,16 +88,13 @@ static inline void unlock_page_cgroup(struct page_cgroup *pc)
 	bit_spin_unlock(PCG_LOCK, &pc->flags);
 }
 
+extern struct mem_cgroup*  root_mem_cgroup;
 
 static inline struct mem_cgroup* pc_to_mem_cgroup(struct page_cgroup *pc)
 {
-	return pc->mem_cgroup;
-}
-
-static inline void
-pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
-{
-	pc->mem_cgroup = memcg;
+	if (likely(!PageCgroupReset(pc)))
+		return pc->mem_cgroup;
+	return root_mem_cgroup;
 }
 
 static inline void

mm/memcontrol.c +3 -2

index d366b60..622fd2e 100644
@@ -1080,7 +1080,8 @@ struct lruvec *mem_cgroup_lru_add_list(struct zone *zone, struct page *page,
 	 * of pc's mem_cgroup safe.
 	 */
 	if (!PageCgroupUsed(pc) && memcg != root_mem_cgroup) {
-		pc_set_mem_cgroup(pc, root_mem_cgroup);
+		/* this reset bit is cleared when the page is charged */
+		SetPageCgroupReset(pc);
 		memcg = root_mem_cgroup;
 	}

@@ -2626,7 +2627,7 @@ static int mem_cgroup_move_account(struct page *page,
 		__mem_cgroup_cancel_charge(from, nr_pages);
 
 	/* caller should have done css_get */
-	pc_set_mem_cgroup(pc, to);
+	pc_set_mem_cgroup_and_flags(pc, to, BIT(PCG_USED) | BIT(PCG_LOCK));
 	mem_cgroup_charge_statistics(to, anon, nr_pages);
 	/*
 	 * We charges against "to" which may not have any tasks. Then, "to"

Signature · 9 lines

1.7.4.1


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

Ying Han

2012-04-17 21:25 UTC #7 lore raw

Re: [RFC][PATCH 3/6] memcg: add PageCgroupReset()

Quoted 10 lines · KAMEZAWA Hiroyuki

A commit "memcg: simplify LRU handling by new rule" removes PCG_ACCT_LRU. and the bug introduced by it was fixed by "memcg: fix GPF when cgroup removal races with last exit"
This was for reducing flags on pc->flags....Now, we have 3bits of flags. but this patch adds a new flag, I'm sorry. (Considering alignment of kmalloc(), we'll able to have 5 bits..)
This patch adds PCG_RESET which is similar to PCG_ACCT_LRU.

This is set

Quoted 1 line

when mem_cgroup_add_lru_list() finds we cannot trust the pc's mem_cgroup.

Do we still need the new flag? I assume some of the upcoming patches will provide the guarantee of pc->mem_cgroup.

--Ying

Quoted 93 lines

The reason why this patch adds a (renamed) flag again is for merging pc->flags and pc->mem_cgroup. Assume pc's mem_cgroup is encoded as

mem_cgroup = pc->flags & ~0x7

if (PageCgroupReset(pc))
return root_mem_cgroup;
return pc->mem_cgroup;

This update of Reset bit can be done in atomic by set_bit(). And cleared when USED bit is set.

Considering kmalloc()'s alignment, having 4bits of flags will be ok....

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

---
include/linux/page_cgroup.h | 15 ++++++++------- mm/memcontrol.c | 5 +++--

 2 files changed, 11 insertions(+), 9 deletions(-)

include/linux/page_cgroup.h +0 -0

index 2707809..3f3b4ff 100644
@@ -8,6 +8,7 @@ enum {

PCG_LOCK, /* Lock for pc->mem_cgroup and following bits. */ PCG_USED, /* this object is in use. */ PCG_MIGRATION, /* under page migration */
+ PCG_RESET, /* have been reset to root_mem_cgroup */ __NR_PCG_FLAGS,
};

include/linux/page_cgroup.h +7 -7

@@ -70,6 +71,9 @@ SETPCGFLAG(Migration, MIGRATION)
  CLEARPCGFLAG(Migration, MIGRATION)
  TESTPCGFLAG(Migration, MIGRATION)
 
+TESTPCGFLAG(Reset, RESET)
+SETPCGFLAG(Reset, RESET)
+
  static inline void lock_page_cgroup(struct page_cgroup *pc)
  {
        /*

@@ -84,16 +88,13 @@ static inline void unlock_page_cgroup(struct page_cgroup *pc)
        bit_spin_unlock(PCG_LOCK, &pc->flags);
  }
 
+extern struct mem_cgroup*  root_mem_cgroup;
 
  static inline struct mem_cgroup* pc_to_mem_cgroup(struct page_cgroup *pc)
  {
-       return pc->mem_cgroup;
-}
-
-static inline void
-pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
-{
-       pc->mem_cgroup = memcg;
+       if (likely(!PageCgroupReset(pc)))
+               return pc->mem_cgroup;
+       return root_mem_cgroup;
  }

static inline void

mm/memcontrol.c +3 -3

index d366b60..622fd2e 100644
@@ -1080,7 +1080,8 @@ struct lruvec *mem_cgroup_lru_add_list(struct zone *zone, struct page *page,
         * of pc's mem_cgroup safe.
         */
        if (!PageCgroupUsed(pc) && memcg != root_mem_cgroup) {
-               pc_set_mem_cgroup(pc, root_mem_cgroup);
+               /* this reset bit is cleared when the page is charged */
+               SetPageCgroupReset(pc);
                memcg = root_mem_cgroup;
        }

@@ -2626,7 +2627,7 @@ static int mem_cgroup_move_account(struct page *page,
                __mem_cgroup_cancel_charge(from, nr_pages);
 
        /* caller should have done css_get */
-       pc_set_mem_cgroup(pc, to);
+       pc_set_mem_cgroup_and_flags(pc, to, BIT(PCG_USED) | BIT(PCG_LOCK));
        mem_cgroup_charge_statistics(to, anon, nr_pages);
        /*
         * We charges against "to" which may not have any tasks. Then, "to"
--

Quoted 3 lines

1.7.4.1

KAMEZAWA Hiroyuki

author

2012-04-18 07:36 UTC #8 lore raw

Re: [RFC][PATCH 3/6] memcg: add PageCgroupReset()

Quoted 15 lines · Ying Han

Quoted 10 lines · KAMEZAWA Hiroyuki
A commit "memcg: simplify LRU handling by new rule" removes PCG_ACCT_LRU. and the bug introduced by it was fixed by "memcg: fix GPF when cgroup removal races with last exit"
This was for reducing flags on pc->flags....Now, we have 3bits of flags. but this patch adds a new flag, I'm sorry. (Considering alignment of kmalloc(), we'll able to have 5 bits..)
This patch adds PCG_RESET which is similar to PCG_ACCT_LRU.

Quoted 6 lines

This is set
Quoted 1 line
when mem_cgroup_add_lru_list() finds we cannot trust the pc's mem_cgroup.
Do we still need the new flag? I assume some of the upcoming patches will provide the guarantee of pc->mem_cgroup.

If per-lruvec locking can do reference-count GC, memcg will never be freed there are pages which points to the memcg, and allows us to remove whole this 'move to root' logic, I agree. We'll not require a new flag.

It means that memcg cannot be freed until there are not page(_cgroup) which points to dead memcg. mem_cgroup_reset_owner() is removed, now.

We need to handle 2 cases.
- newly allocated pages which linked to memcg before use.
- unused pages but added to lru by some lazy logic.

And make a guarantee
- pages added to LRU of dead memcg will be freed or moved to other cgroup, soon.

I have no good idea.

Thanks,
-Kame

Quoted 87 lines

--Ying

Quoted 86 lines

The reason why this patch adds a (renamed) flag again is for merging pc->flags and pc->mem_cgroup. Assume pc's mem_cgroup is encoded as

       mem_cgroup = pc->flags & ~0x7

       if (PageCgroupReset(pc))
               return root_mem_cgroup;
       return pc->mem_cgroup;

This update of Reset bit can be done in atomic by set_bit(). And cleared when USED bit is set.

Considering kmalloc()'s alignment, having 4bits of flags will be ok....

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

---
 include/linux/page_cgroup.h |   15 ++++++++-------
 mm/memcontrol.c             |    5 +++--
 2 files changed, 11 insertions(+), 9 deletions(-)

include/linux/page_cgroup.h +8 -7

index 2707809..3f3b4ff 100644
@@ -8,6 +8,7 @@ enum {
       PCG_LOCK,  /* Lock for pc->mem_cgroup and following bits. */
       PCG_USED, /* this object is in use. */
       PCG_MIGRATION, /* under page migration */
+       PCG_RESET,     /* have been reset to root_mem_cgroup */
       __NR_PCG_FLAGS,
 };

@@ -70,6 +71,9 @@ SETPCGFLAG(Migration, MIGRATION)
 CLEARPCGFLAG(Migration, MIGRATION)
 TESTPCGFLAG(Migration, MIGRATION)
 
+TESTPCGFLAG(Reset, RESET)
+SETPCGFLAG(Reset, RESET)
+
 static inline void lock_page_cgroup(struct page_cgroup *pc)
 {
       /*

@@ -84,16 +88,13 @@ static inline void unlock_page_cgroup(struct page_cgroup *pc)
       bit_spin_unlock(PCG_LOCK, &pc->flags);
 }
 
+extern struct mem_cgroup*  root_mem_cgroup;
 
 static inline struct mem_cgroup* pc_to_mem_cgroup(struct page_cgroup *pc)
 {
-       return pc->mem_cgroup;
-}
-
-static inline void
-pc_set_mem_cgroup(struct page_cgroup *pc, struct mem_cgroup *memcg)
-{
-       pc->mem_cgroup = memcg;
+       if (likely(!PageCgroupReset(pc)))
+               return pc->mem_cgroup;
+       return root_mem_cgroup;
 }
 
 static inline void

mm/memcontrol.c +2 -1

index d366b60..622fd2e 100644
@@ -1080,7 +1080,8 @@ struct lruvec *mem_cgroup_lru_add_list(struct zone *zone, struct page *page,
        * of pc's mem_cgroup safe.
        */
       if (!PageCgroupUsed(pc) && memcg != root_mem_cgroup) {
-               pc_set_mem_cgroup(pc, root_mem_cgroup);
+               /* this reset bit is cleared when the page is charged */
+               SetPageCgroupReset(pc);
               memcg = root_mem_cgroup;
       }

@@ -2626,7 +2627,7 @@ static int mem_cgroup_move_account(struct page *page,
               __mem_cgroup_cancel_charge(from, nr_pages);

Quoted 12 lines

Quoted 10 lines

mm/memcontrol.c +1 -2

       /* caller should have done css_get */
-       pc_set_mem_cgroup(pc, to);
+       pc_set_mem_cgroup_and_flags(pc, to, BIT(PCG_USED) | BIT(PCG_LOCK));
       mem_cgroup_charge_statistics(to, anon, nr_pages);
       /*
        * We charges against "to" which may not have any tasks. Then, "to"
--

1.7.4.1

KAMEZAWA Hiroyuki

author

2012-03-28 11:01 UTC #9 lore raw

RFC PATCH v2 4/6 memcg: remove mem_cgroup pointer from page_cgroup

In reply to KAMEZAWA Hiroyuki

This patch removes pc->mem_cgroup and merge the pointer into flags as

63 4 0
| pointer to memcg..........|flags|

After this, memory cgroup's overhead is 8(4)bytes per a page.

Changelog:
- added BUILD_BUG_ON()
- update comments in Kconfig

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

---
 include/linux/page_cgroup.h |   21 +++++++++------------
 init/Kconfig                |    2 +-
 2 files changed, 10 insertions(+), 13 deletions(-)

include/linux/page_cgroup.h +9 ~~-12~~

index 3f3b4ff..7e3a3c7 100644
@@ -3,6 +3,10 @@
 
 #include <linux/smp.h>
 
+/*
+ * These flags are encoded into low bits of unsigned long,
+ * ORed with a pointer to mem_cgroup.
+ */
 enum {
 	/* flags for mem_cgroup */
 	PCG_LOCK,  /* Lock for pc->mem_cgroup and following bits. */

@@ -12,6 +16,8 @@ enum {
 	__NR_PCG_FLAGS,
 };
 
+#define PCG_FLAGS_MASK	((1 << __NR_PCG_FLAGS) - 1)
+
 #ifndef __GENERATING_BOUNDS_H
 #include <generated/bounds.h>

@@ -27,7 +33,6 @@ enum {
  */
 struct page_cgroup {
 	unsigned long flags;
-	struct mem_cgroup *mem_cgroup;
 };
 
 void __meminit pgdat_page_cgroup_init(struct pglist_data *pgdat);

@@ -93,7 +98,7 @@ extern struct mem_cgroup*  root_mem_cgroup;
 static inline struct mem_cgroup* pc_to_mem_cgroup(struct page_cgroup *pc)
 {
 	if (likely(!PageCgroupReset(pc)))
-		return pc->mem_cgroup;
+		return (struct mem_cgroup*)(pc->flags & ~PCG_FLAGS_MASK);
 	return root_mem_cgroup;
 }

@@ -101,16 +106,8 @@ static inline void
 pc_set_mem_cgroup_and_flags(struct page_cgroup *pc, struct mem_cgroup *memcg,
 			unsigned long flags)
 {
-	pc->mem_cgroup = memcg;
-	/*
-	 * We access a page_cgroup asynchronously without lock_page_cgroup().
-	 * Especially when a page_cgroup is taken from a page, pc's mem_cgroup
-	 * is accessed after testing USED bit. To make pc's mem_cgroup visible
-	 * before USED bit, we need memory barrier here.
-	 * See mem_cgroup_add_lru_list(), etc.
-	 */
-	smp_wmb();
-	pc->flags = flags;
+	BUILD_BUG_ON(__NR_PCG_FLAGS > 5); /* assume 32byte alignment. */
+	pc->flags = (unsigned long)memcg | flags;
 }
 
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */

init/Kconfig +1 -1

index 6cfd71d..e0bfe92 100644
@@ -651,7 +651,7 @@ config CGROUP_MEM_RES_CTLR
 
 	  Note that setting this option increases fixed memory overhead
 	  associated with each page of memory in the system. By this,
-	  20(40)bytes/PAGE_SIZE on 32(64)bit system will be occupied by memory
+	  4(8)bytes/PAGE_SIZE on 32(64)bit system will be occupied by memory
 	  usage tracking struct at boot. Total amount of this is printed out
 	  at boot.

Signature · 9 lines

1.7.4.1


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

KAMEZAWA Hiroyuki

author

2012-03-28 11:02 UTC #10 lore raw

RFC PATCH v2 5/6 memcg: remove unnecessary memory barrier.

In reply to KAMEZAWA Hiroyuki

Now, Used bit and a pointer to memory cgroup are set at once. memory barrier for Used bit -> pc->mem_cgroup is not necessary.

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

---
 mm/memcontrol.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

mm/memcontrol.c +0 -2

index 622fd2e..767bef3 100644
@@ -1258,8 +1258,6 @@ mem_cgroup_get_reclaim_stat_from_page(struct page *page)
 	pc = lookup_page_cgroup(page);
 	if (!PageCgroupUsed(pc))
 		return NULL;
-	/* Ensure pc's mem_cgroup is visible after reading PCG_USED. */
-	smp_rmb();
 	mz = page_cgroup_zoneinfo(pc_to_mem_cgroup(pc), page);
 	return &mz->reclaim_stat;
 }

Signature · 9 lines

1.7.4.1


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

KAMEZAWA Hiroyuki

author

2012-03-28 11:08 UTC #11 lore raw

RFC PATCH v2 6/6 memcg: config for integrate page_cgroup into memmap

In reply to KAMEZAWA Hiroyuki

This patch is an experimental patch.
Considering 32bit archs, I think this should be CONFIG option... ==
Now, struct page_cgroup is 8byte object and allocated per a page. This patch adds a config option to allocate page_cgroup in struct page.

By this
Pros.
- lookup_page_cgroup() is almost 0 cost.
- implementation seems very natural...
Cons.
- size of 'struct page' will be increased (to 64bytes in typical case)
- cgroup_disable=memory will not allow user to avoid 8bytes of overhead.

Tested 'kernel make' on tmpfs.

Config=n
Performance counter stats for 'make -j 8':

1,180,857,100,495 instructions # 0.00 insns per cycle

       923,084,678 cache-misses

      71.346377273 seconds time elapsed

Config=y
Performance counter stats for 'make -j 8':

1,178,404,304,530 instructions # 0.00 insns per cycle

       911,098,615 cache-misses

      71.607477840 seconds time elapsed

seems instructions and cache-misses decreased to some extent. But no visible change in total execution time...

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

---
 include/linux/mm_types.h    |    4 +++-
 include/linux/page_cgroup.h |   33 ++++++++++++++++++++++++++++++++-
 init/Kconfig                |   14 ++++++++++++++
 mm/memcontrol.c             |    3 ++-
 mm/page_alloc.c             |    1 +
 mm/page_cgroup.c            |    3 ++-
 6 files changed, 54 insertions(+), 4 deletions(-)

include/linux/mm_types.h +3 -1

index 76bbdaf..2beda78 100644
@@ -141,7 +141,9 @@ struct page {
 #ifdef CONFIG_WANT_PAGE_DEBUG_FLAGS
 	unsigned long debug_flags;	/* Use atomic bitops on this */
 #endif
-
+#ifdef CONFIG_INTEGRATED_PAGE_CGROUP
+	unsigned long page_cgroup;	/* see page_cgroup.h */
+#endif
 #ifdef CONFIG_KMEMCHECK
 	/*
 	 * kmemcheck wants to track the status of each byte in a page; this

include/linux/page_cgroup.h +32 -1

index 7e3a3c7..0e02632 100644
@@ -35,8 +35,9 @@ struct page_cgroup {
 	unsigned long flags;
 };
 
-void __meminit pgdat_page_cgroup_init(struct pglist_data *pgdat);
 
+#ifndef CONFIG_INTEGRATED_PAGE_CGROUP
+void __meminit pgdat_page_cgroup_init(struct pglist_data *pgdat);
 #ifdef CONFIG_SPARSEMEM
 static inline void __init page_cgroup_init_flatmem(void)
 {

@@ -51,6 +52,36 @@ static inline void __init page_cgroup_init(void)
 
 struct page_cgroup *lookup_page_cgroup(struct page *page);
 struct page *lookup_cgroup_page(struct page_cgroup *pc);
+static inline void memmap_init_cgroup(struct page *page)
+{
+}
+#else
+static inline struct page_cgroup *lookup_page_cgroup(struct page *page)
+{
+	return (struct page_cgroup*)&page->page_cgroup;
+}
+
+static inline struct page *lookup_cgroup_page(struct page_cgroup *pc)
+{
+	return container_of((unsigned long*)pc, struct page, page_cgroup);
+}
+
+static inline void memmap_init_cgroup(struct page *page)
+{
+	page->page_cgroup = 0;
+}
+
+static inline void __init page_cgroup_init_flatmem(void)
+{
+}
+static inline void __init page_cgroup_init(void)
+{
+}
+
+static inline void pgdat_page_cgroup_init(struct pglist_data *pgdat)
+{
+}
+#endif
 
 #define TESTPCGFLAG(uname, lname)			\
 static inline int PageCgroup##uname(struct page_cgroup *pc)	\

init/Kconfig +14 -0

index e0bfe92..99514c2 100644
@@ -694,6 +694,20 @@ config CGROUP_MEM_RES_CTLR_SWAP_ENABLED
 	  For those who want to have the feature enabled by default should
 	  select this option (if, for some reason, they need to disable it
 	  then swapaccount=0 does the trick).
+
+config INTEGRATED_PAGE_CGROUP
+	bool "record memory cgroup information into struct page"
+	depends on CGROUP_MEM_RES_CTLR
+	default n
+	help
+	  Memory Resource Controller consumes 4/(8 if 64bit)bytes per page.
+	  It's independent of 'struct page'. If you say Y here, memory cgroup
+	  information is recorded into struct page and increase size of it
+	  4/8 bytes. With this, cpu overheads in runtime will be reduced
+	  but you cannot avoid above overheads of 4/8 bytes per page by boot
+	  option. If unsure, say N.
+
+
 config CGROUP_MEM_RES_CTLR_KMEM
 	bool "Memory Resource Controller Kernel Memory accounting (EXPERIMENTAL)"
 	depends on CGROUP_MEM_RES_CTLR && EXPERIMENTAL

mm/memcontrol.c +2 -1

index 767bef3..0c5b15c 100644
@@ -2557,7 +2557,8 @@ void mem_cgroup_split_huge_fixup(struct page *head)
 	if (!PageCgroupUsed(head_pc))
 		return;
 	for (i = 1; i < HPAGE_PMD_NR; i++) {
-		pc = head_pc + i;
+		/* page struct is contiguous in hugepage. */
+		pc = lookup_page_cgroup(head + i);
 		pc_set_mem_cgroup_and_flags(pc, memcg, BIT(PCG_USED));
 	}
 }

mm/page_alloc.c +1 -0

index 0b37873..9be94df 100644
@@ -3682,6 +3682,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		if (!is_highmem_idx(zone))
 			set_page_address(page, __va(pfn << PAGE_SHIFT));
 #endif
+		memmap_init_cgroup(page);
 	}
 }

mm/page_cgroup.c +2 -1

index 1ccbd71..036c8ea 100644
@@ -11,6 +11,7 @@
 #include <linux/swapops.h>
 #include <linux/kmemleak.h>
 
+#ifndef CONFIG_INTEGRATED_PAGE_CGROUP
 static unsigned long total_usage;
 
 #if !defined(CONFIG_SPARSEMEM)

@@ -315,7 +316,7 @@ void __meminit pgdat_page_cgroup_init(struct pglist_data *pgdat)
 }
 
 #endif
-
+#endif
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP

Signature · 10 lines

1.7.4.1



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