Re: [patch] mm, memcg: add oom killer delay

2013-07-10 Thread Michal Hocko
On Fri 14-06-13 03:12:52, David Rientjes wrote: > On Fri, 14 Jun 2013, Kamezawa Hiroyuki wrote: > > > Reading your discussion, I think I understand your requirements. > > The problem is that I can't think you took into all options into > > accounts and found the best way is this new oom_delay. IOW

Re: [patch] mm, memcg: add oom killer delay

2013-06-26 Thread David Rientjes
On Tue, 25 Jun 2013, Kamezawa Hiroyuki wrote: > Considering only memcg, bypassing all charge-limit-check will work. > But as you say, that will not work against global-oom. I think it will since we have per-zone memory reserves that can be bypassed in the page allocator, not to the level of PF_M

Re: [patch] mm, memcg: add oom killer delay

2013-06-24 Thread Kamezawa Hiroyuki
(2013/06/14 19:12), David Rientjes wrote: On Fri, 14 Jun 2013, Kamezawa Hiroyuki wrote: Reading your discussion, I think I understand your requirements. The problem is that I can't think you took into all options into accounts and found the best way is this new oom_delay. IOW, I can't convice o

Re: [patch] mm, memcg: add oom killer delay

2013-06-19 Thread David Rientjes
On Fri, 14 Jun 2013, David Rientjes wrote: > Even with all of the above (which is not actually that invasive of a > patch), I still think we need memory.oom_delay_millisecs. I probably made > a mistake in describing what that is addressing if it seems like it's > trying to address any of the a

Re: [patch] mm, memcg: add oom killer delay

2013-06-14 Thread David Rientjes
On Fri, 14 Jun 2013, Kamezawa Hiroyuki wrote: > Reading your discussion, I think I understand your requirements. > The problem is that I can't think you took into all options into > accounts and found the best way is this new oom_delay. IOW, I can't > convice oom-delay is the best way to handle yo

Re: [patch] mm, memcg: add oom killer delay

2013-06-13 Thread Kamezawa Hiroyuki
(2013/06/14 7:25), David Rientjes wrote: On Thu, 13 Jun 2013, Michal Hocko wrote: That's not at all the objective, the changelog quite explicitly states this is a deadlock as the result of userspace having disabled the oom killer so that its userspace oom handler can resolve the condition and i

Re: [patch] mm, memcg: add oom killer delay

2013-06-13 Thread David Rientjes
On Thu, 13 Jun 2013, Michal Hocko wrote: > > That's not at all the objective, the changelog quite explicitly states > > this is a deadlock as the result of userspace having disabled the oom > > killer so that its userspace oom handler can resolve the condition and it > > being unresponsive or u

Re: [patch] mm, memcg: add oom killer delay

2013-06-13 Thread Michal Hocko
On Wed 12-06-13 14:27:05, David Rientjes wrote: > On Wed, 12 Jun 2013, Michal Hocko wrote: > > > But the objective is to handle oom deadlocks gracefully and you cannot > > possibly miss those as they are, well, _deadlocks_. > > That's not at all the objective, the changelog quite explicitly state

Re: [patch] mm, memcg: add oom killer delay

2013-06-12 Thread David Rientjes
On Wed, 12 Jun 2013, Michal Hocko wrote: > But the objective is to handle oom deadlocks gracefully and you cannot > possibly miss those as they are, well, _deadlocks_. That's not at all the objective, the changelog quite explicitly states this is a deadlock as the result of userspace having disa

Re: [patch] mm, memcg: add oom killer delay

2013-06-12 Thread Michal Hocko
On Tue 11-06-13 13:33:40, David Rientjes wrote: > On Mon, 10 Jun 2013, Michal Hocko wrote: [...] > > Your only objection against userspace handling so far was that admin > > has no control over sub-hierarchies. But that is hardly a problem. A > > periodic tree scan would solve this easily (just to

Re: [patch] mm, memcg: add oom killer delay

2013-06-11 Thread David Rientjes
On Mon, 10 Jun 2013, Michal Hocko wrote: > > > > I don't think you yet understand the problem, which is probably my > > > > fault. > > > > I'm not insisting this be implemented in the kernel, I'm saying it's > > > > not > > > > possible to do it in userspace. > > > > > > Because you still

Re: [patch] mm, memcg: add oom killer delay

2013-06-10 Thread Michal Hocko
On Wed 05-06-13 17:09:17, David Rientjes wrote: > On Wed, 5 Jun 2013, Michal Hocko wrote: > > > > For the aforementioned reason that we give users the ability to > > > manipulate > > > their own memcg trees and userspace is untrusted. Their oom notifiers > > > cannot be run as root, not only b

Re: [patch] mm, memcg: add oom killer delay

2013-06-05 Thread David Rientjes
On Wed, 5 Jun 2013, Michal Hocko wrote: > > For the aforementioned reason that we give users the ability to manipulate > > their own memcg trees and userspace is untrusted. Their oom notifiers > > cannot be run as root, not only because of security but also because it > > would not properly is

Re: [patch] mm, memcg: add oom killer delay

2013-06-05 Thread Michal Hocko
On Tue 04-06-13 14:48:52, Johannes Weiner wrote: > On Tue, Jun 04, 2013 at 11:17:49AM +0200, Michal Hocko wrote: [...] > > Now that I am looking at this again I've realized that this > > is not correct. The task which triggers memcg OOM will not > > have memcg_oom.memcg set so it would trigger a gl

Re: [patch] mm, memcg: add oom killer delay

2013-06-05 Thread Michal Hocko
On Tue 04-06-13 23:40:16, David Rientjes wrote: > On Tue, 4 Jun 2013, Michal Hocko wrote: > > > > I'm not sure a userspace oom notifier would want to keep a > > > preallocated buffer around that is mlocked in memory for all possible > > > lengths of this file. > > > > Well, an oom handler which a

Re: [patch] mm, memcg: add oom killer delay

2013-06-04 Thread David Rientjes
On Tue, 4 Jun 2013, Michal Hocko wrote: > > I'm not sure a userspace oom notifier would want to keep a > > preallocated buffer around that is mlocked in memory for all possible > > lengths of this file. > > Well, an oom handler which allocates memory under the same restricted > memory doesn't mak

Re: [patch] mm, memcg: add oom killer delay

2013-06-04 Thread Michal Hocko
On Tue 04-06-13 14:48:52, Johannes Weiner wrote: > On Tue, Jun 04, 2013 at 11:17:49AM +0200, Michal Hocko wrote: [...] > > > diff --git a/mm/memory.c b/mm/memory.c > > > index 6dc1882..ff5e2d7 100644 > > > --- a/mm/memory.c > > > +++ b/mm/memory.c > > > @@ -1815,7 +1815,7 @@ long __get_user_pages(s

Re: [patch] mm, memcg: add oom killer delay

2013-06-04 Thread Johannes Weiner
On Tue, Jun 04, 2013 at 11:17:49AM +0200, Michal Hocko wrote: > On Mon 03-06-13 14:30:18, Johannes Weiner wrote: > > On Mon, Jun 03, 2013 at 12:48:39PM -0400, Johannes Weiner wrote: > > > On Mon, Jun 03, 2013 at 05:34:32PM +0200, Michal Hocko wrote: > [...] > > > > I am just afraid about all the ot

Re: [patch] mm, memcg: add oom killer delay

2013-06-04 Thread Michal Hocko
On Mon 03-06-13 14:17:54, David Rientjes wrote: > On Mon, 3 Jun 2013, Michal Hocko wrote: > > > > What do you suggest when you read the "tasks" file and it returns -ENOMEM > > > because kmalloc() fails because the userspace oom handler's memcg is also > > > oom? > > > > That would require that

Re: [patch] mm, memcg: add oom killer delay

2013-06-04 Thread Michal Hocko
On Mon 03-06-13 14:30:18, Johannes Weiner wrote: > On Mon, Jun 03, 2013 at 12:48:39PM -0400, Johannes Weiner wrote: > > On Mon, Jun 03, 2013 at 05:34:32PM +0200, Michal Hocko wrote: [...] > > > I am just afraid about all the other archs that do not support (from > > > quick grep it looks like: blac

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Johannes Weiner
On Mon, Jun 03, 2013 at 12:09:22PM -0700, David Rientjes wrote: > On Mon, 3 Jun 2013, Johannes Weiner wrote: > > > > It's not necessarily harder if you assign the userspace oom handlers to > > > the root of your subtree with access to more memory than the children. > > > There is no "inability"

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread KOSAKI Motohiro
> From: Johannes Weiner > Subject: [PATCH] memcg: do not sleep on OOM waitqueue with full charge context > > The memcg OOM handling is incredibly fragile because once a memcg goes > OOM, one task (kernel or userspace) is responsible for resolving the > situation. Every other task that gets caught

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread David Rientjes
On Mon, 3 Jun 2013, Michal Hocko wrote: > > What do you suggest when you read the "tasks" file and it returns -ENOMEM > > because kmalloc() fails because the userspace oom handler's memcg is also > > oom? > > That would require that you track kernel allocations which is currently > done only f

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Michal Hocko
On Mon 03-06-13 11:18:09, David Rientjes wrote: > On Sat, 1 Jun 2013, Michal Hocko wrote: [...] > > I still do not see why you cannot simply read tasks file into a > > preallocated buffer. This would be few pages even for thousands of pids. > > You do not have to track processes as they come and go

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread David Rientjes
On Mon, 3 Jun 2013, Johannes Weiner wrote: > > It's not necessarily harder if you assign the userspace oom handlers to > > the root of your subtree with access to more memory than the children. > > There is no "inability" to write a proper handler, but when you have > > dozens of individual us

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Johannes Weiner
On Mon, Jun 03, 2013 at 11:18:09AM -0700, David Rientjes wrote: > On Sat, 1 Jun 2013, Michal Hocko wrote: > > OK, I assume those groups are generally untrusted, right? So you cannot > > let them register their oom handler even via an admin interface. This > > makes it a bit complicated because it m

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Johannes Weiner
On Mon, Jun 03, 2013 at 12:48:39PM -0400, Johannes Weiner wrote: > On Mon, Jun 03, 2013 at 05:34:32PM +0200, Michal Hocko wrote: > > On Sat 01-06-13 02:11:51, Johannes Weiner wrote: > > [...] > > > From: Johannes Weiner > > > Subject: [PATCH] memcg: more robust oom handling > > > > > > The memcg

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread David Rientjes
On Sat, 1 Jun 2013, Michal Hocko wrote: > > Users obviously don't have the ability to attach processes to the root > > memcg. They are constrained to their own subtree of memcgs. > > OK, I assume those groups are generally untrusted, right? So you cannot > let them register their oom handler ev

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Michal Hocko
On Mon 03-06-13 12:48:39, Johannes Weiner wrote: > On Mon, Jun 03, 2013 at 05:34:32PM +0200, Michal Hocko wrote: > > On Sat 01-06-13 02:11:51, Johannes Weiner wrote: > > [...] > > > From: Johannes Weiner > > > Subject: [PATCH] memcg: more robust oom handling > > > > > > The memcg OOM handling is

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread David Rientjes
On Fri, 31 May 2013, Andrew Morton wrote: > > Admins may set the oom killer delay using the new interface: > > > > # echo 6 > memory.oom_delay_millisecs > > > > This will defer oom killing to the kernel only after 60 seconds has > > elapsed by putting the task to sleep for 60 seconds. >

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Johannes Weiner
On Mon, Jun 03, 2013 at 06:31:34PM +0200, Michal Hocko wrote: > On Sat 01-06-13 02:11:51, Johannes Weiner wrote: > > @@ -2076,6 +2077,7 @@ static void memcg_wakeup_oom(struct mem_cgroup *memcg) > > { > > /* for filtering, pass "memcg" as argument. */ > > __wake_up(&memcg_oom_waitq, TASK_NO

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Johannes Weiner
On Mon, Jun 03, 2013 at 05:34:32PM +0200, Michal Hocko wrote: > On Sat 01-06-13 02:11:51, Johannes Weiner wrote: > [...] > > From: Johannes Weiner > > Subject: [PATCH] memcg: more robust oom handling > > > > The memcg OOM handling is incredibly fragile because once a memcg goes > > OOM, one task

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Michal Hocko
On Sat 01-06-13 02:11:51, Johannes Weiner wrote: > @@ -2076,6 +2077,7 @@ static void memcg_wakeup_oom(struct mem_cgroup *memcg) > { > /* for filtering, pass "memcg" as argument. */ > __wake_up(&memcg_oom_waitq, TASK_NORMAL, 0, memcg); > + atomic_inc(&memcg->oom_wakeups); > } > >

Re: [patch] mm, memcg: add oom killer delay

2013-06-03 Thread Michal Hocko
On Sat 01-06-13 02:11:51, Johannes Weiner wrote: [...] > From: Johannes Weiner > Subject: [PATCH] memcg: more robust oom handling > > The memcg OOM handling is incredibly fragile because once a memcg goes > OOM, one task (kernel or userspace) is responsible for resolving the > situation. Every o

Re: [patch] mm, memcg: add oom killer delay

2013-06-01 Thread Johannes Weiner
On Sat, Jun 01, 2013 at 12:29:05PM +0200, Michal Hocko wrote: > On Sat 01-06-13 02:11:51, Johannes Weiner wrote: > [...] > > I'm currently messing around with the below patch. When a task faults > > and charges under OOM, the memcg is remembered in the task struct and > > then made to sleep on the

Re: [patch] mm, memcg: add oom killer delay

2013-06-01 Thread Michal Hocko
On Sat 01-06-13 02:11:51, Johannes Weiner wrote: [...] > I'm currently messing around with the below patch. When a task faults > and charges under OOM, the memcg is remembered in the task struct and > then made to sleep on the memcg's OOM waitqueue only after unwinding > the page fault stack. Wit

Re: [patch] mm, memcg: add oom killer delay

2013-06-01 Thread Michal Hocko
On Fri 31-05-13 12:29:17, David Rientjes wrote: > On Fri, 31 May 2013, Michal Hocko wrote: > > > > We allow users to control their own memcgs by chowning them, so they must > > > be run in the same hierarchy if they want to run their own userspace oom > > > handler. There's nothing in the kerne

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread Johannes Weiner
On Fri, May 31, 2013 at 12:29:17PM -0700, David Rientjes wrote: > On Fri, 31 May 2013, Michal Hocko wrote: > > > It's too easy to simply do even a "ps ax" in an oom memcg and make that > > > thread completely unresponsive because it allocates memory. > > > > Yes, but we are talking about oom hand

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread Andrew Morton
On Wed, 29 May 2013 18:18:10 -0700 (PDT) David Rientjes wrote: > Completely disabling the oom killer for a memcg is problematic if > userspace is unable to address the condition itself, usually because it > is unresponsive. This scenario creates a memcg deadlock: tasks are > sitting in TASK_KIL

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread David Rientjes
On Fri, 31 May 2013, Michal Hocko wrote: > > We allow users to control their own memcgs by chowning them, so they must > > be run in the same hierarchy if they want to run their own userspace oom > > handler. There's nothing in the kernel that prevents that and the user > > has no other option

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread Michal Hocko
On Fri 31-05-13 03:22:59, David Rientjes wrote: > On Fri, 31 May 2013, Michal Hocko wrote: > > > I have always discouraged people from running oom handler in the same > > memcg (or even in the same hierarchy). > > > > We allow users to control their own memcgs by chowning them, so they must > b

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread Michal Hocko
On Fri 31-05-13 03:22:59, David Rientjes wrote: > On Fri, 31 May 2013, Michal Hocko wrote: [...] > > > If the oom notifier is in the oom cgroup, it may not be able to > > > successfully read the memcg "tasks" file to even determine the set of > > > eligible processes. > > > > It would have

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread David Rientjes
On Fri, 31 May 2013, Michal Hocko wrote: > I have always discouraged people from running oom handler in the same > memcg (or even in the same hierarchy). > We allow users to control their own memcgs by chowning them, so they must be run in the same hierarchy if they want to run their own usersp

Re: [patch] mm, memcg: add oom killer delay

2013-05-31 Thread Michal Hocko
On Thu 30-05-13 13:47:30, David Rientjes wrote: > On Thu, 30 May 2013, Michal Hocko wrote: > > > > Completely disabling the oom killer for a memcg is problematic if > > > userspace is unable to address the condition itself, usually because it > > > is unresponsive. > > > > Isn't this a bug in th

Re: [patch] mm, memcg: add oom killer delay

2013-05-30 Thread David Rientjes
On Thu, 30 May 2013, Michal Hocko wrote: > > Completely disabling the oom killer for a memcg is problematic if > > userspace is unable to address the condition itself, usually because it > > is unresponsive. > > Isn't this a bug in the userspace oom handler? Why is it unresponsive? It > shouldn'

Re: [patch] mm, memcg: add oom killer delay

2013-05-30 Thread Michal Hocko
On Wed 29-05-13 18:18:10, David Rientjes wrote: > Completely disabling the oom killer for a memcg is problematic if > userspace is unable to address the condition itself, usually because it > is unresponsive. Isn't this a bug in the userspace oom handler? Why is it unresponsive? It shouldn't allo