* [PATCH 2/2] memcg: add accessor to mem_cgroup.css
2009-09-11 11:22 [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page() Wu Fengguang
@ 2009-09-11 11:22 ` Wu Fengguang
2009-09-12 0:51 ` KAMEZAWA Hiroyuki
2009-09-11 11:36 ` [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page() Wu Fengguang
2009-09-12 0:47 ` KAMEZAWA Hiroyuki
2 siblings, 1 reply; 5+ messages in thread
From: Wu Fengguang @ 2009-09-11 11:22 UTC (permalink / raw)
To: Andrew Morton
Cc: KOSAKI Motohiro, Hugh Dickins, Daisuke Nishimura, Balbir Singh,
KAMEZAWA Hiroyuki, Andi Kleen, linux-mm
So that an outside user can free the reference count grabbed by
try_get_mem_cgroup_from_page().
CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
CC: Hugh Dickins <hugh@veritas.com>
CC: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
CC: Balbir Singh <balbir@linux.vnet.ibm.com>
CC: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
include/linux/memcontrol.h | 7 +++++++
mm/memcontrol.c | 8 ++++++++
2 files changed, 15 insertions(+)
--- linux-mm.orig/include/linux/memcontrol.h 2009-09-11 18:16:55.000000000 +0800
+++ linux-mm/include/linux/memcontrol.h 2009-09-11 18:16:56.000000000 +0800
@@ -81,6 +81,8 @@ int mm_match_cgroup(const struct mm_stru
return cgroup == mem;
}
+extern struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *mem);
+
extern int
mem_cgroup_prepare_migration(struct page *page, struct mem_cgroup **ptr);
extern void mem_cgroup_end_migration(struct mem_cgroup *mem,
@@ -206,6 +208,11 @@ static inline int task_in_mem_cgroup(str
return 1;
}
+static inline struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *mem)
+{
+ return NULL;
+}
+
static inline int
mem_cgroup_prepare_migration(struct page *page, struct mem_cgroup **ptr)
{
--- linux-mm.orig/mm/memcontrol.c 2009-09-11 18:16:55.000000000 +0800
+++ linux-mm/mm/memcontrol.c 2009-09-11 18:18:11.000000000 +0800
@@ -282,6 +282,14 @@ mem_cgroup_zoneinfo(struct mem_cgroup *m
return &mem->info.nodeinfo[nid]->zoneinfo[zid];
}
+#ifdef CONFIG_HWPOISON_INJECT /* for now, only user is hwpoison injector */
+struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *mem)
+{
+ return &mem->css;
+}
+EXPORT_SYMBOL(mem_cgroup_css);
+#endif
+
static struct mem_cgroup_per_zone *
page_cgroup_zoneinfo(struct page_cgroup *pc)
{
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] memcg: add accessor to mem_cgroup.css
2009-09-11 11:22 ` [PATCH 2/2] memcg: add accessor to mem_cgroup.css Wu Fengguang
@ 2009-09-12 0:51 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 5+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-09-12 0:51 UTC (permalink / raw)
To: Wu Fengguang
Cc: Andrew Morton, KOSAKI Motohiro, Hugh Dickins, Daisuke Nishimura,
Balbir Singh, KAMEZAWA Hiroyuki, Andi Kleen, linux-mm
Wu Fengguang wrote:
> So that an outside user can free the reference count grabbed by
> try_get_mem_cgroup_from_page().
>
While no heavy caller for this, I(we) would like to hide mem_cgroup
under memcontrol.c
Personally, I like no #ifdef for this easy code ;)
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> CC: Hugh Dickins <hugh@veritas.com>
> CC: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> CC: Balbir Singh <balbir@linux.vnet.ibm.com>
> CC: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
> include/linux/memcontrol.h | 7 +++++++
> mm/memcontrol.c | 8 ++++++++
> 2 files changed, 15 insertions(+)
>
> --- linux-mm.orig/include/linux/memcontrol.h 2009-09-11 18:16:55.000000000
> +0800
> +++ linux-mm/include/linux/memcontrol.h 2009-09-11 18:16:56.000000000
> +0800
> @@ -81,6 +81,8 @@ int mm_match_cgroup(const struct mm_stru
> return cgroup == mem;
> }
>
> +extern struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup
> *mem);
> +
> extern int
> mem_cgroup_prepare_migration(struct page *page, struct mem_cgroup **ptr);
> extern void mem_cgroup_end_migration(struct mem_cgroup *mem,
> @@ -206,6 +208,11 @@ static inline int task_in_mem_cgroup(str
> return 1;
> }
>
> +static inline struct cgroup_subsys_state *mem_cgroup_css(struct
> mem_cgroup *mem)
> +{
> + return NULL;
> +}
> +
> static inline int
> mem_cgroup_prepare_migration(struct page *page, struct mem_cgroup **ptr)
> {
> --- linux-mm.orig/mm/memcontrol.c 2009-09-11 18:16:55.000000000 +0800
> +++ linux-mm/mm/memcontrol.c 2009-09-11 18:18:11.000000000 +0800
> @@ -282,6 +282,14 @@ mem_cgroup_zoneinfo(struct mem_cgroup *m
> return &mem->info.nodeinfo[nid]->zoneinfo[zid];
> }
>
> +#ifdef CONFIG_HWPOISON_INJECT /* for now, only user is hwpoison injector
> */
> +struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *mem)
> +{
> + return &mem->css;
> +}
> +EXPORT_SYMBOL(mem_cgroup_css);
> +#endif
> +
> static struct mem_cgroup_per_zone *
> page_cgroup_zoneinfo(struct page_cgroup *pc)
> {
>
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page()
2009-09-11 11:22 [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page() Wu Fengguang
2009-09-11 11:22 ` [PATCH 2/2] memcg: add accessor to mem_cgroup.css Wu Fengguang
@ 2009-09-11 11:36 ` Wu Fengguang
2009-09-12 0:47 ` KAMEZAWA Hiroyuki
2 siblings, 0 replies; 5+ messages in thread
From: Wu Fengguang @ 2009-09-11 11:36 UTC (permalink / raw)
To: Andrew Morton
Cc: Hugh Dickins, KOSAKI Motohiro, Daisuke Nishimura, Balbir Singh,
KAMEZAWA Hiroyuki, Andi Kleen, linux-mm
[add CC to new Hugh; will update CC inside patch]
Hi Kame and Balbir,
After your previous reviews, I tried out the pin-pfn idea.
It resulted in many code in both kernel and user space tools.
So I (and Andi) decided that the complexity of tracking pin
states is not worth it. It would be best to reuse the memcg
functionalities for testing hwpoison. It is particular handy
for some fork storm tests.
The change since the initial post is
- don't export the memcg id. we don't need it indeed, and can simply
try to hwpoison *all* memcg tracked pages.
- generate mem_cgroup_css() code only for CONFIG_HWPOISON_INJECT
Thus no user visible changes and no extra code when hwpoison is
disabled.
Thanks,
Fengguang
On Fri, Sep 11, 2009 at 07:22:21PM +0800, Wu Fengguang wrote:
> So that the hwpoison injector can get mem_cgroup for arbitrary page
> and thus know whether it is owned by some mem_cgroup task(s).
>
> Background:
>
> The hwpoison test suite need to inject hwpoison to a collection of
> selected task pages, and must not touch pages not owned by these pages
> and thus kill important system processes such as init. (But it's OK to
> mis-hwpoison free/unowned pages as well as shared clean pages.
> Mis-hwpoison of shared dirty pages will kill all tasks, so the test
> suite will target all or non of such tasks in the first place.)
>
> The memory cgroup serves this purpose well. We can put the target
> processes under the control of a memory cgroup, and tell the hwpoison
> injection code to only kill pages associated with some active memory
> cgroup.
>
> The prerequsite for doing hwpoison stress tests with mem_cgroup is,
> the mem_cgroup code tracks task pages _accurately_ (unless page is
> locked). Which we believe is/should be true.
>
> The benifits are simplification of hwpoison injector code. Also the
> mem_cgroup code will automatically be tested by hwpoison test cases.
>
> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> CC: Hugh Dickins <hugh@veritas.com>
> CC: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> CC: Balbir Singh <balbir@linux.vnet.ibm.com>
> CC: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
> include/linux/memcontrol.h | 6 ++++++
> mm/memcontrol.c | 12 +++++-------
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> --- linux-mm.orig/mm/memcontrol.c 2009-09-11 18:51:14.000000000 +0800
> +++ linux-mm/mm/memcontrol.c 2009-09-11 18:52:14.000000000 +0800
> @@ -1389,25 +1389,22 @@ static struct mem_cgroup *mem_cgroup_loo
> return container_of(css, struct mem_cgroup, css);
> }
>
> -static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page *page)
> +struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
> {
> - struct mem_cgroup *mem;
> + struct mem_cgroup *mem = NULL;
> struct page_cgroup *pc;
> unsigned short id;
> swp_entry_t ent;
>
> VM_BUG_ON(!PageLocked(page));
>
> - if (!PageSwapCache(page))
> - return NULL;
> -
> pc = lookup_page_cgroup(page);
> lock_page_cgroup(pc);
> if (PageCgroupUsed(pc)) {
> mem = pc->mem_cgroup;
> if (mem && !css_tryget(&mem->css))
> mem = NULL;
> - } else {
> + } else if (PageSwapCache(page)) {
> ent.val = page_private(page);
> id = lookup_swap_cgroup(ent);
> rcu_read_lock();
> @@ -1419,6 +1416,7 @@ static struct mem_cgroup *try_get_mem_cg
> unlock_page_cgroup(pc);
> return mem;
> }
> +EXPORT_SYMBOL(try_get_mem_cgroup_from_page);
>
> /*
> * commit a charge got by __mem_cgroup_try_charge() and makes page_cgroup to be
> @@ -1753,7 +1751,7 @@ int mem_cgroup_try_charge_swapin(struct
> */
> if (!PageSwapCache(page))
> return 0;
> - mem = try_get_mem_cgroup_from_swapcache(page);
> + mem = try_get_mem_cgroup_from_page(page);
> if (!mem)
> goto charge_cur_mm;
> *ptr = mem;
> --- linux-mm.orig/include/linux/memcontrol.h 2009-09-11 18:51:13.000000000 +0800
> +++ linux-mm/include/linux/memcontrol.h 2009-09-11 18:52:14.000000000 +0800
> @@ -68,6 +68,7 @@ extern unsigned long mem_cgroup_isolate_
> extern void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t gfp_mask);
> int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *mem);
>
> +extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page);
> extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p);
>
> static inline
> @@ -189,6 +190,11 @@ mem_cgroup_move_lists(struct page *page,
> {
> }
>
> +static inline struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
> +{
> + return NULL;
> +}
> +
> static inline int mm_match_cgroup(struct mm_struct *mm, struct mem_cgroup *mem)
> {
> return 1;
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page()
2009-09-11 11:22 [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page() Wu Fengguang
2009-09-11 11:22 ` [PATCH 2/2] memcg: add accessor to mem_cgroup.css Wu Fengguang
2009-09-11 11:36 ` [PATCH 1/2] memcg: rename and export try_get_mem_cgroup_from_page() Wu Fengguang
@ 2009-09-12 0:47 ` KAMEZAWA Hiroyuki
2 siblings, 0 replies; 5+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-09-12 0:47 UTC (permalink / raw)
To: Wu Fengguang
Cc: Andrew Morton, KOSAKI Motohiro, Hugh Dickins, Daisuke Nishimura,
Balbir Singh, KAMEZAWA Hiroyuki, Andi Kleen, linux-mm
Wu Fengguang wrote:
> So that the hwpoison injector can get mem_cgroup for arbitrary page
> and thus know whether it is owned by some mem_cgroup task(s).
>
I have no strong objections to these 2 patches.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
But it's in merge-window. I recommend you to repost again with
codes for caller(hwpoison itself).
-Kame
So if you
> Background:
>
> The hwpoison test suite need to inject hwpoison to a collection of
> selected task pages, and must not touch pages not owned by these pages
> and thus kill important system processes such as init. (But it's OK to
> mis-hwpoison free/unowned pages as well as shared clean pages.
> Mis-hwpoison of shared dirty pages will kill all tasks, so the test
> suite will target all or non of such tasks in the first place.)
>
> The memory cgroup serves this purpose well. We can put the target
> processes under the control of a memory cgroup, and tell the hwpoison
> injection code to only kill pages associated with some active memory
> cgroup.
>
> The prerequsite for doing hwpoison stress tests with mem_cgroup is,
> the mem_cgroup code tracks task pages _accurately_ (unless page is
> locked). Which we believe is/should be true.
>
> The benifits are simplification of hwpoison injector code. Also the
> mem_cgroup code will automatically be tested by hwpoison test cases.
>
> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> CC: Hugh Dickins <hugh@veritas.com>
> CC: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> CC: Balbir Singh <balbir@linux.vnet.ibm.com>
> CC: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
> include/linux/memcontrol.h | 6 ++++++
> mm/memcontrol.c | 12 +++++-------
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> --- linux-mm.orig/mm/memcontrol.c 2009-09-11 18:51:14.000000000 +0800
> +++ linux-mm/mm/memcontrol.c 2009-09-11 18:52:14.000000000 +0800
> @@ -1389,25 +1389,22 @@ static struct mem_cgroup *mem_cgroup_loo
> return container_of(css, struct mem_cgroup, css);
> }
>
> -static struct mem_cgroup *try_get_mem_cgroup_from_swapcache(struct page
> *page)
> +struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
> {
> - struct mem_cgroup *mem;
> + struct mem_cgroup *mem = NULL;
> struct page_cgroup *pc;
> unsigned short id;
> swp_entry_t ent;
>
> VM_BUG_ON(!PageLocked(page));
>
> - if (!PageSwapCache(page))
> - return NULL;
> -
> pc = lookup_page_cgroup(page);
> lock_page_cgroup(pc);
> if (PageCgroupUsed(pc)) {
> mem = pc->mem_cgroup;
> if (mem && !css_tryget(&mem->css))
> mem = NULL;
> - } else {
> + } else if (PageSwapCache(page)) {
> ent.val = page_private(page);
> id = lookup_swap_cgroup(ent);
> rcu_read_lock();
> @@ -1419,6 +1416,7 @@ static struct mem_cgroup *try_get_mem_cg
> unlock_page_cgroup(pc);
> return mem;
> }
> +EXPORT_SYMBOL(try_get_mem_cgroup_from_page);
>
> /*
> * commit a charge got by __mem_cgroup_try_charge() and makes page_cgroup
> to be
> @@ -1753,7 +1751,7 @@ int mem_cgroup_try_charge_swapin(struct
> */
> if (!PageSwapCache(page))
> return 0;
> - mem = try_get_mem_cgroup_from_swapcache(page);
> + mem = try_get_mem_cgroup_from_page(page);
> if (!mem)
> goto charge_cur_mm;
> *ptr = mem;
> --- linux-mm.orig/include/linux/memcontrol.h 2009-09-11 18:51:13.000000000
> +0800
> +++ linux-mm/include/linux/memcontrol.h 2009-09-11 18:52:14.000000000
> +0800
> @@ -68,6 +68,7 @@ extern unsigned long mem_cgroup_isolate_
> extern void mem_cgroup_out_of_memory(struct mem_cgroup *mem, gfp_t
> gfp_mask);
> int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup
> *mem);
>
> +extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page
> *page);
> extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p);
>
> static inline
> @@ -189,6 +190,11 @@ mem_cgroup_move_lists(struct page *page,
> {
> }
>
> +static inline struct mem_cgroup *try_get_mem_cgroup_from_page(struct page
> *page)
> +{
> + return NULL;
> +}
> +
> static inline int mm_match_cgroup(struct mm_struct *mm, struct mem_cgroup
> *mem)
> {
> return 1;
>
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread