Re: [patch 1/2] mm, memcg: avoid oom notification when current needsaccess to memory reserves

From: Michal Hocko
Date: Tue Dec 17 2013 - 11:23:48 EST


On Fri 13-12-13 15:55:44, David Rientjes wrote:
> On Thu, 12 Dec 2013, Michal Hocko wrote:
[...]
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index c72b03bf9679..fee25c5934d2 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2692,7 +2693,8 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
> > * MEMDIE process.
> > */
> > if (unlikely(test_thread_flag(TIF_MEMDIE)
> > - || fatal_signal_pending(current)))
> > + || fatal_signal_pending(current))
> > + || current->flags & PF_EXITING)
> > goto bypass;
> >
> > if (unlikely(task_in_memcg_oom(current)))
> >
> > rather than the later checks down the oom_synchronize paths. The comment
> > already mentions dying process...
> >
>
> This is scary because it doesn't even try to reclaim memcg memory before
> allowing the allocation to succeed.

Why should it reclaim in the first place when it simply is on the way to
release memory. In other words why should it increase the memory
pressure when it is in fact releasing it?

I am really puzzled here. On one hand you are strongly arguing for not
notifying when we know we can prevent from OOM action and on the other
hand you are ok to get vmpressure/thresholds notification when an
exiting task triggers reclaim.

So I am really lost in what you are trying to achieve here. It sounds a
bit arbirtrary.

> I think we could even argue that we should move the
> fatal_signal_pending(current) check to later and the only condition we
> should really be bypassing here is TIF_MEMDIE since it will only get
> set when reclaim has already failed.

Any arguments?

--
Michal Hocko
SUSE Labs
--
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/