[-mm patch] Show memcg information during OOM (v2)

[-mm patch] Show memcg information during OOM (v2)

[Balbir Singh]

Description: Add RSS and swap to OOM output from memcg

From: Balbir Singh <balbir@linux.vnet.ibm.com>

Changelog v2..v1:

1. Add more information about task's memcg and the memcg
   over it's limit
2. Print data in KB
3. Move the print routine outside task_lock()
4. Use rcu_read_lock() around cgroup_path, strictly speaking it
   is not required, but relying on the current memcg implementation
   is not a good idea.

This patch displays memcg values like failcnt, usage and limit
when an OOM occurs due to memcg.

NOTE: In case the path exceeds 128 bytes, we omit printing the
name of the cgroups. It is possible to circumvent this problem
by using static arrays of PAGE_SIZE and we know that OOM is
serialized when invoked from the memory controller. This did
not seem like a good idea, but can be implemented if 128 bytes
seems like a severe limitation.

Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
review.

Sample output
-------------

Task in /a/x killed as a result of limit of /a
memory: usage 1048576kB, limit 1048576kB, failcnt 4183
memory+swap: usage 1400964kB, limit 9007199254740991kB, failcnt 0

Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---

 include/linux/memcontrol.h |    6 ++++
 mm/memcontrol.c            |   61 ++++++++++++++++++++++++++++++++++++++++++++
 mm/oom_kill.c              |    1 +
 3 files changed, 68 insertions(+), 0 deletions(-)


diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 326f45c..56f1af2 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -104,6 +104,8 @@ struct zone_reclaim_stat *mem_cgroup_get_reclaim_stat(struct mem_cgroup *memcg,
 						      struct zone *zone);
 struct zone_reclaim_stat*
 mem_cgroup_get_reclaim_stat_from_page(struct page *page);
+extern void mem_cgroup_print_mem_info(struct mem_cgroup *memcg,
+					struct task_struct *p);
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 extern int do_swap_account;
@@ -270,6 +272,10 @@ mem_cgroup_get_reclaim_stat_from_page(struct page *page)
 	return NULL;
 }
 
+void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
+{
+}
+
 #endif /* CONFIG_CGROUP_MEM_CONT */
 
 #endif /* _LINUX_MEMCONTROL_H */
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8e4be9c..e7f82b6 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -42,6 +42,7 @@
 
 struct cgroup_subsys mem_cgroup_subsys __read_mostly;
 #define MEM_CGROUP_RECLAIM_RETRIES	5
+#define MEM_CGROUP_OOM_BUF_SIZE		128
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled && really_do_swap_account = 0 */
@@ -813,6 +814,66 @@ bool mem_cgroup_oom_called(struct task_struct *task)
 	rcu_read_unlock();
 	return ret;
 }
+
+/**
+ * mem_cgroup_print_mem_info: Called from OOM with tasklist_lock held in
+ * read mode.
+ * @memcg: The memory cgroup that went over limit
+ * @p: Task that is going to be killed
+ *
+ * NOTE: @memcg and @p's mem_cgroup can be different when hierarchy is
+ * enabled
+ */
+void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
+{
+	struct cgroup *task_cgrp;
+	struct cgroup *mem_cgrp;
+	/*
+	 * Need a buffer on stack, can't rely on allocations.
+	 */
+	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
+	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
+	int ret;
+
+	if (!memcg)
+		return;
+
+	mem_cgrp = memcg->css.cgroup;
+	task_cgrp = mem_cgroup_from_task(p)->css.cgroup;
+
+	rcu_read_lock();
+	ret = cgroup_path(task_cgrp, task_memcg_name, MEM_CGROUP_OOM_BUF_SIZE);
+	if (ret < 0) {
+		/*
+		 * Unfortunately, we are unable to convert to a useful name
+		 * But we'll still print out the usage information
+		 */
+		rcu_read_unlock();
+		goto done;
+	}
+	ret = cgroup_path(mem_cgrp, memcg_name, MEM_CGROUP_OOM_BUF_SIZE);
+	 if (ret < 0) {
+		rcu_read_unlock();
+		goto done;
+	}
+
+	rcu_read_unlock();
+
+	printk(KERN_INFO "Task in %s killed as a result of limit of %s\n",
+			task_memcg_name, memcg_name);
+done:
+
+	printk(KERN_INFO "memory: usage %llukB, limit %llukB, failcnt %llu\n",
+		res_counter_read_u64(&memcg->res, RES_USAGE) >> 10,
+		res_counter_read_u64(&memcg->res, RES_LIMIT) >> 10,
+		res_counter_read_u64(&memcg->res, RES_FAILCNT));
+	printk(KERN_INFO "memory+swap: usage %llukB, limit %llukB, "
+		"failcnt %llu\n", 
+		res_counter_read_u64(&memcg->memsw, RES_USAGE) >> 10,
+		res_counter_read_u64(&memcg->memsw, RES_LIMIT) >> 10,
+		res_counter_read_u64(&memcg->memsw, RES_FAILCNT));
+}
+
 /*
  * Unlike exported interface, "oom" parameter is added. if oom==true,
  * oom-killer can be invoked.
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index d3b9bac..951356f 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -394,6 +394,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 		cpuset_print_task_mems_allowed(current);
 		task_unlock(current);
 		dump_stack();
+		mem_cgroup_print_mem_info(mem, current);
 		show_mem();
 		if (sysctl_oom_dump_tasks)
 			dump_tasks(mem);

-- 
	Balbir

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

Re: [-mm patch] Show memcg information during OOM (v2)

[Balbir Singh]

Checkpatch caught an additional space, so here is the patch again


Description: Add RSS and swap to OOM output from memcg

From: Balbir Singh <balbir@linux.vnet.ibm.com>

Changelog v2..v1:

1. Add more information about task's memcg and the memcg
   over it's limit
2. Print data in KB
3. Move the print routine outside task_lock()
4. Use rcu_read_lock() around cgroup_path, strictly speaking it
   is not required, but relying on the current memcg implementation
   is not a good idea.


This patch displays memcg values like failcnt, usage and limit
when an OOM occurs due to memcg.

Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
review.

Sample output
-------------

Task in /a/x killed as a result of limit of /a
memory: usage 1048576kB, limit 1048576kB, failcnt 4183
memory+swap: usage 1400964kB, limit 9007199254740991kB, failcnt 0

Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---

 include/linux/memcontrol.h |    6 ++++
 mm/memcontrol.c            |   61 ++++++++++++++++++++++++++++++++++++++++++++
 mm/oom_kill.c              |    1 +
 3 files changed, 68 insertions(+), 0 deletions(-)


diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 326f45c..56f1af2 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -104,6 +104,8 @@ struct zone_reclaim_stat *mem_cgroup_get_reclaim_stat(struct mem_cgroup *memcg,
 						      struct zone *zone);
 struct zone_reclaim_stat*
 mem_cgroup_get_reclaim_stat_from_page(struct page *page);
+extern void mem_cgroup_print_mem_info(struct mem_cgroup *memcg,
+					struct task_struct *p);
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 extern int do_swap_account;
@@ -270,6 +272,10 @@ mem_cgroup_get_reclaim_stat_from_page(struct page *page)
 	return NULL;
 }
 
+void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
+{
+}
+
 #endif /* CONFIG_CGROUP_MEM_CONT */
 
 #endif /* _LINUX_MEMCONTROL_H */
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8e4be9c..ee3bae4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -42,6 +42,7 @@
 
 struct cgroup_subsys mem_cgroup_subsys __read_mostly;
 #define MEM_CGROUP_RECLAIM_RETRIES	5
+#define MEM_CGROUP_OOM_BUF_SIZE		128
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled && really_do_swap_account = 0 */
@@ -813,6 +814,66 @@ bool mem_cgroup_oom_called(struct task_struct *task)
 	rcu_read_unlock();
 	return ret;
 }
+
+/**
+ * mem_cgroup_print_mem_info: Called from OOM with tasklist_lock held in
+ * read mode.
+ * @memcg: The memory cgroup that went over limit
+ * @p: Task that is going to be killed
+ *
+ * NOTE: @memcg and @p's mem_cgroup can be different when hierarchy is
+ * enabled
+ */
+void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
+{
+	struct cgroup *task_cgrp;
+	struct cgroup *mem_cgrp;
+	/*
+	 * Need a buffer on stack, can't rely on allocations.
+	 */
+	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
+	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
+	int ret;
+
+	if (!memcg)
+		return;
+
+	mem_cgrp = memcg->css.cgroup;
+	task_cgrp = mem_cgroup_from_task(p)->css.cgroup;
+
+	rcu_read_lock();
+	ret = cgroup_path(task_cgrp, task_memcg_name, MEM_CGROUP_OOM_BUF_SIZE);
+	if (ret < 0) {
+		/*
+		 * Unfortunately, we are unable to convert to a useful name
+		 * But we'll still print out the usage information
+		 */
+		rcu_read_unlock();
+		goto done;
+	}
+	ret = cgroup_path(mem_cgrp, memcg_name, MEM_CGROUP_OOM_BUF_SIZE);
+	 if (ret < 0) {
+		rcu_read_unlock();
+		goto done;
+	}
+
+	rcu_read_unlock();
+
+	printk(KERN_INFO "Task in %s killed as a result of limit of %s\n",
+			task_memcg_name, memcg_name);
+done:
+
+	printk(KERN_INFO "memory: usage %llukB, limit %llukB, failcnt %llu\n",
+		res_counter_read_u64(&memcg->res, RES_USAGE) >> 10,
+		res_counter_read_u64(&memcg->res, RES_LIMIT) >> 10,
+		res_counter_read_u64(&memcg->res, RES_FAILCNT));
+	printk(KERN_INFO "memory+swap: usage %llukB, limit %llukB, "
+		"failcnt %llu\n",
+		res_counter_read_u64(&memcg->memsw, RES_USAGE) >> 10,
+		res_counter_read_u64(&memcg->memsw, RES_LIMIT) >> 10,
+		res_counter_read_u64(&memcg->memsw, RES_FAILCNT));
+}
+
 /*
  * Unlike exported interface, "oom" parameter is added. if oom==true,
  * oom-killer can be invoked.
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index d3b9bac..951356f 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -394,6 +394,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 		cpuset_print_task_mems_allowed(current);
 		task_unlock(current);
 		dump_stack();
+		mem_cgroup_print_mem_info(mem, current);
 		show_mem();
 		if (sysctl_oom_dump_tasks)
 			dump_tasks(mem);

-- 
	Balbir

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

Re: [-mm patch] Show memcg information during OOM (v2)

[KAMEZAWA Hiroyuki]

On Tue, 3 Feb 2009 12:57:01 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> Checkpatch caught an additional space, so here is the patch again
> 
> 
> Description: Add RSS and swap to OOM output from memcg
> 
> From: Balbir Singh <balbir@linux.vnet.ibm.com>
> 
> Changelog v2..v1:
> 
> 1. Add more information about task's memcg and the memcg
>    over it's limit
> 2. Print data in KB
> 3. Move the print routine outside task_lock()
> 4. Use rcu_read_lock() around cgroup_path, strictly speaking it
>    is not required, but relying on the current memcg implementation
>    is not a good idea.
> 
> 
> This patch displays memcg values like failcnt, usage and limit
> when an OOM occurs due to memcg.
> 
> Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
> Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
> review.
> 

IIUC, this oom_kill is serialized by memcg_tasklist mutex.
Then, you don't have to allocate buffer on stack.


> +void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
> +{
> +	struct cgroup *task_cgrp;
> +	struct cgroup *mem_cgrp;
> +	/*
> +	 * Need a buffer on stack, can't rely on allocations.
> +	 */
> +	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> +	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> +	int ret;
> +

making this as

static char task_memcg_name[PATH_MAX];
static char memcg_name[PATH_MAX];

is ok, I think. and the patch will be more simple.

Thanks,
-kame


> +	if (!memcg)
> +		return;
> +
> +	mem_cgrp = memcg->css.cgroup;
> +	task_cgrp = mem_cgroup_from_task(p)->css.cgroup;
> +
> +	rcu_read_lock();
> +	ret = cgroup_path(task_cgrp, task_memcg_name, MEM_CGROUP_OOM_BUF_SIZE);
> +	if (ret < 0) {
> +		/*
> +		 * Unfortunately, we are unable to convert to a useful name
> +		 * But we'll still print out the usage information
> +		 */
> +		rcu_read_unlock();
> +		goto done;
> +	}
> +	ret = cgroup_path(mem_cgrp, memcg_name, MEM_CGROUP_OOM_BUF_SIZE);
> +	 if (ret < 0) {
> +		rcu_read_unlock();
> +		goto done;
> +	}
> +
> +	rcu_read_unlock();
> +
> +	printk(KERN_INFO "Task in %s killed as a result of limit of %s\n",
> +			task_memcg_name, memcg_name);
> +done:
> +
> +	printk(KERN_INFO "memory: usage %llukB, limit %llukB, failcnt %llu\n",
> +		res_counter_read_u64(&memcg->res, RES_USAGE) >> 10,
> +		res_counter_read_u64(&memcg->res, RES_LIMIT) >> 10,
> +		res_counter_read_u64(&memcg->res, RES_FAILCNT));
> +	printk(KERN_INFO "memory+swap: usage %llukB, limit %llukB, "
> +		"failcnt %llu\n",
> +		res_counter_read_u64(&memcg->memsw, RES_USAGE) >> 10,
> +		res_counter_read_u64(&memcg->memsw, RES_LIMIT) >> 10,
> +		res_counter_read_u64(&memcg->memsw, RES_FAILCNT));
> +}
> +
>  /*
>   * Unlike exported interface, "oom" parameter is added. if oom==true,
>   * oom-killer can be invoked.
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index d3b9bac..951356f 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -394,6 +394,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
>  		cpuset_print_task_mems_allowed(current);
>  		task_unlock(current);
>  		dump_stack();
> +		mem_cgroup_print_mem_info(mem, current);
>  		show_mem();
>  		if (sysctl_oom_dump_tasks)
>  			dump_tasks(mem);
> 
> -- 
> 	Balbir
> 

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

Re: [-mm patch] Show memcg information during OOM (v2)

[Balbir Singh]

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-03 17:04:27]:

> On Tue, 3 Feb 2009 12:57:01 +0530
> Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
> > Checkpatch caught an additional space, so here is the patch again
> > 
> > 
> > Description: Add RSS and swap to OOM output from memcg
> > 
> > From: Balbir Singh <balbir@linux.vnet.ibm.com>
> > 
> > Changelog v2..v1:
> > 
> > 1. Add more information about task's memcg and the memcg
> >    over it's limit
> > 2. Print data in KB
> > 3. Move the print routine outside task_lock()
> > 4. Use rcu_read_lock() around cgroup_path, strictly speaking it
> >    is not required, but relying on the current memcg implementation
> >    is not a good idea.
> > 
> > 
> > This patch displays memcg values like failcnt, usage and limit
> > when an OOM occurs due to memcg.
> > 
> > Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
> > Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
> > review.
> > 
> 
> IIUC, this oom_kill is serialized by memcg_tasklist mutex.
> Then, you don't have to allocate buffer on stack.
> 
> 
> > +void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
> > +{
> > +	struct cgroup *task_cgrp;
> > +	struct cgroup *mem_cgrp;
> > +	/*
> > +	 * Need a buffer on stack, can't rely on allocations.
> > +	 */
> > +	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > +	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > +	int ret;
> > +
> 
> making this as
> 
> static char task_memcg_name[PATH_MAX];
> static char memcg_name[PATH_MAX];
> 
> is ok, I think. and the patch will be more simple.
>

I've mentioned it in the NOTE section as well, I wanted more opinions
before going that route. I'll resend v3. 

-- 
	Balbir

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

Re: [-mm patch] Show memcg information during OOM (v2)

[Balbir Singh]

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-03 17:04:27]:

> On Tue, 3 Feb 2009 12:57:01 +0530
> Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
> > Checkpatch caught an additional space, so here is the patch again
> > 
> > 
> > Description: Add RSS and swap to OOM output from memcg
> > 
> > From: Balbir Singh <balbir@linux.vnet.ibm.com>
> > 
> > Changelog v2..v1:
> > 
> > 1. Add more information about task's memcg and the memcg
> >    over it's limit
> > 2. Print data in KB
> > 3. Move the print routine outside task_lock()
> > 4. Use rcu_read_lock() around cgroup_path, strictly speaking it
> >    is not required, but relying on the current memcg implementation
> >    is not a good idea.
> > 
> > 
> > This patch displays memcg values like failcnt, usage and limit
> > when an OOM occurs due to memcg.
> > 
> > Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
> > Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
> > review.
> > 
> 
> IIUC, this oom_kill is serialized by memcg_tasklist mutex.
> Then, you don't have to allocate buffer on stack.
> 
> 
> > +void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
> > +{
> > +	struct cgroup *task_cgrp;
> > +	struct cgroup *mem_cgrp;
> > +	/*
> > +	 * Need a buffer on stack, can't rely on allocations.
> > +	 */
> > +	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > +	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > +	int ret;
> > +
> 
> making this as
> 
> static char task_memcg_name[PATH_MAX];
> static char memcg_name[PATH_MAX];
> 
> is ok, I think. and the patch will be more simple.
>

I am having second thoughts about this one. It introduces a standard
overhead of 2 pages on x86*, while the first one will work for most
cases and all the overhead is on stack, which disappears quickly.
That is the reason I did not do it in the first place and put it as a
NOTE.
 
-- 
	Balbir

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

Re: [-mm patch] Show memcg information during OOM (v2)

[KAMEZAWA Hiroyuki]

On Tue, 3 Feb 2009 15:49:21 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-03 17:04:27]:
> 
> > On Tue, 3 Feb 2009 12:57:01 +0530
> > Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > 
> > > Checkpatch caught an additional space, so here is the patch again
> > > 
> > > 
> > > Description: Add RSS and swap to OOM output from memcg
> > > 
> > > From: Balbir Singh <balbir@linux.vnet.ibm.com>
> > > 
> > > Changelog v2..v1:
> > > 
> > > 1. Add more information about task's memcg and the memcg
> > >    over it's limit
> > > 2. Print data in KB
> > > 3. Move the print routine outside task_lock()
> > > 4. Use rcu_read_lock() around cgroup_path, strictly speaking it
> > >    is not required, but relying on the current memcg implementation
> > >    is not a good idea.
> > > 
> > > 
> > > This patch displays memcg values like failcnt, usage and limit
> > > when an OOM occurs due to memcg.
> > > 
> > > Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
> > > Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
> > > review.
> > > 
> > 
> > IIUC, this oom_kill is serialized by memcg_tasklist mutex.
> > Then, you don't have to allocate buffer on stack.
> > 
> > 
> > > +void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
> > > +{
> > > +	struct cgroup *task_cgrp;
> > > +	struct cgroup *mem_cgrp;
> > > +	/*
> > > +	 * Need a buffer on stack, can't rely on allocations.
> > > +	 */
> > > +	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > > +	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > > +	int ret;
> > > +
> > 
> > making this as
> > 
> > static char task_memcg_name[PATH_MAX];
> > static char memcg_name[PATH_MAX];
> > 
> > is ok, I think. and the patch will be more simple.
> >
> 
> I am having second thoughts about this one. It introduces a standard
> overhead of 2 pages on x86*, while the first one will work for most
> cases and all the overhead is on stack, which disappears quickly.
> That is the reason I did not do it in the first place and put it as a
> NOTE.
>  
But *128* is tooooooo short ;)
And, your patch makes "OOM Message Format" unstable.
>From system administration view, it's unacceptable.
Not printing name at all is better than "printed out sometimes you lucky"

Thanks,
-Kame



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

Re: [-mm patch] Show memcg information during OOM (v2)

[Balbir Singh]

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-03 19:28:19]:

> On Tue, 3 Feb 2009 15:49:21 +0530
> Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
> > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-02-03 17:04:27]:
> > 
> > > On Tue, 3 Feb 2009 12:57:01 +0530
> > > Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > > 
> > > > Checkpatch caught an additional space, so here is the patch again
> > > > 
> > > > 
> > > > Description: Add RSS and swap to OOM output from memcg
> > > > 
> > > > From: Balbir Singh <balbir@linux.vnet.ibm.com>
> > > > 
> > > > Changelog v2..v1:
> > > > 
> > > > 1. Add more information about task's memcg and the memcg
> > > >    over it's limit
> > > > 2. Print data in KB
> > > > 3. Move the print routine outside task_lock()
> > > > 4. Use rcu_read_lock() around cgroup_path, strictly speaking it
> > > >    is not required, but relying on the current memcg implementation
> > > >    is not a good idea.
> > > > 
> > > > 
> > > > This patch displays memcg values like failcnt, usage and limit
> > > > when an OOM occurs due to memcg.
> > > > 
> > > > Thanks go out to Johannes Weiner, Li Zefan, David Rientjes,
> > > > Kamezawa Hiroyuki, Daisuke Nishimura and KOSAKI Motohiro for
> > > > review.
> > > > 
> > > 
> > > IIUC, this oom_kill is serialized by memcg_tasklist mutex.
> > > Then, you don't have to allocate buffer on stack.
> > > 
> > > 
> > > > +void mem_cgroup_print_mem_info(struct mem_cgroup *memcg, struct task_struct *p)
> > > > +{
> > > > +	struct cgroup *task_cgrp;
> > > > +	struct cgroup *mem_cgrp;
> > > > +	/*
> > > > +	 * Need a buffer on stack, can't rely on allocations.
> > > > +	 */
> > > > +	char task_memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > > > +	char memcg_name[MEM_CGROUP_OOM_BUF_SIZE];
> > > > +	int ret;
> > > > +
> > > 
> > > making this as
> > > 
> > > static char task_memcg_name[PATH_MAX];
> > > static char memcg_name[PATH_MAX];
> > > 
> > > is ok, I think. and the patch will be more simple.
> > >
> > 
> > I am having second thoughts about this one. It introduces a standard
> > overhead of 2 pages on x86*, while the first one will work for most
> > cases and all the overhead is on stack, which disappears quickly.
> > That is the reason I did not do it in the first place and put it as a
> > NOTE.
> >  
> But *128* is tooooooo short ;)
> And, your patch makes "OOM Message Format" unstable.
> >From system administration view, it's unacceptable.
> Not printing name at all is better than "printed out sometimes you lucky"
>

OK, I have the code with PATH_MAX ready. I'll send that out.
 
-- 
	Balbir

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