On Wed, May 07, 2014 at 04:33:34PM +0200, Michal Hocko wrote:
> On Wed 30-04-14 16:25:36, Johannes Weiner wrote:
> > The charging path currently starts out with OOM condition checks when
> > OOM is the rarest possible case.
> >
> > Rearrange this code to run OOM/task dying checks only after trying the
> > percpu charge and the res_counter charge and bail out before entering
> > reclaim. Attempting a charge does not hurt an (oom-)killed task as
> > much as every charge attempt having to check OOM conditions.
>
> OK, I've never considered those to be measurable but it is true that the
> numbers accumulate over time.
>
> So yes, this makes sense.
>
> > Also, only check __GFP_NOFAIL when the charge would actually fail.
>
> OK, but return ENOMEM as pointed below.
>
> > Signed-off-by: Johannes Weiner <han...@cmpxchg.org>
> > ---
> > mm/memcontrol.c | 31 ++++++++++++++++---------------
> > 1 file changed, 16 insertions(+), 15 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 75dfeb8fa98b..6ce59146fec7 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2598,21 +2598,6 @@ static int mem_cgroup_try_charge(struct mem_cgroup
> > *memcg,
> >
> > if (mem_cgroup_is_root(memcg))
> > goto done;
> > - /*
> > - * Unlike in global OOM situations, memcg is not in a physical
> > - * memory shortage. Allow dying and OOM-killed tasks to
> > - * bypass the last charges so that they can exit quickly and
> > - * free their memory.
> > - */
> > - if (unlikely(test_thread_flag(TIF_MEMDIE) ||
> > - fatal_signal_pending(current)))
> > - goto bypass;
>
> This is missing "memcg: do not hang on OOM when killed by userspace OOM
> access to memory reserves" - trivial to resolve.
Yep, will rebase before the next submission.
> > - if (unlikely(task_in_memcg_oom(current)))
> > - goto nomem;
> > -
> > - if (gfp_mask & __GFP_NOFAIL)
> > - oom = false;
> > retry:
> > if (consume_stock(memcg, nr_pages))
> > goto done;
> [...]
> > @@ -2662,6 +2660,9 @@ retry:
> > if (mem_cgroup_wait_acct_move(mem_over_limit))
> > goto retry;
> >
> > + if (gfp_mask & __GFP_NOFAIL)
> > + goto bypass;
> > +
>
> This is a behavior change because we have returned ENOMEM previously
__GFP_NOFAIL must never return -ENOMEM, or we'd have to rename it ;-)
It just looks like this in the patch, but this is the label code:
nomem:
if (!(gfp_mask & __GFP_NOFAIL))
return -ENOMEM;
bypass:
...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
