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 <hannes@xxxxxxxxxxx> > > --- > > 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, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>