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

[Balbir Singh]

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

> On Tue, 3 Feb 2009 15:49:21 +0530
> Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> [2009-02-03 17:04:27]:
> > 
> > > On Tue, 3 Feb 2009 12:57:01 +0530
> > > Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > > > 
> > > > 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/