Re: [RFC][PATCH]Per-cgroup OOM handler

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Mon, 10 Nov 2008 20:42:23 -0800
Ying Han <yinghan@google.com> wrote:

> Thank you for your comments.
> On Wed, Nov 5, 2008 at 9:34 PM, KAMEZAWA Hiroyuki <
> kamezawa.hiroyu@jp.fujitsu.com> wrote:
>     Here is how we do the one-tick-wait in cgroup_should_oom() in oom_kill.c
>     >-------if (!ret) {
>     >------->-------/* If we're not going to OOM, we should sleep for a
>     >------->------- * bit to give userspace a chance to respond before we
>     >------->------- * go back and try to reclaim again */
>     >------->-------schedule_timeout_uninterruptible(1);
>     >-------}
>    and it works well in-house so far as i mentioned earlier. what's
> important here is not "sleeping for one tick", the idea here is to
> reschedule the ooming thread so the oom handler can make action ( like
> adding memory node to the cpuset) and the subsequent page allocator in
> get_page_from_freelist() can use it.
> 
Can't we avoid this kind of magical one-tick wait ?

> 
> > (Before OOM, the system tend to wait in congestion_wait() or some.)
> 
>    I am not sure how the call to congestion_wait() relevant to the
> "one-tick-wait"? We are simply just trying to reschedule the ooming task,
> that the oom handler has waken up to have chance doing something.
> 
if lucky.


> >
> >
> > OOM-handler shoule be in another cpuset or mlocked in this case
> 
> The oom-handler is in the same cgroup as the ooming task. That is why it's
> called per-cgroup oom-handler. However, there's probably a livelock if the
> userspace oom handler is the one that triggers the oom and detach/reattaches
> without ever freeing or adding memory. For this case, either we can detect
> in the kernel by doing something like if(current == pid) or just leave the
> problem up to userspace( the oom handler shouldn't detach itself after
> getting the ooming notification, it is considered to be a user bug? ).
> 
Hmm, from discussion of mem_notify handler in Feb/March of this year,
oom-hanlder cannot works well if memory is near to OOM, in general.
Then, mlockall was recomemded to handler.
(and it must not do file access.)

I wonder creating small cpuset (and isolated node) for oom-handler may be
another help.


> >
> > I'm wondering
> >  - freeeze-all-threads-in-group-at-oom
> >  - free emergency memory to page allocator which was pooled at cgroup
> > creation
> >    rather than 1-tick wait
> >
> > BTW, it seems this patch allows task detach/attach always. it's safe(and
> > sane) ?
> 
>    yes, we allows task detach/attach. So far we don't see any race condition
> except the livelock
> i mentioned above. Any particular scenario can think of now? thanks
> 
I don't find it ;)
BTW, shouldn't we disable preempt(or irq) before taking spinlocks ?

> > +static int cgroup_should_oom(void)
> > +{
> > +     int ret = 1; /* OOM by default */
> > +     struct oom_cgroup *cs;
> > +
> > +     task_lock(current);
> > +     cs = oom_cgroup_from_task(current);
> > +

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>