Re: [PATCH] mm, oom: check memcg margin for parallel oom

[Michal Hocko <mhocko@kernel.org>]

On Tue 14-07-20 21:25:04, Yafang Shao wrote:
> On Tue, Jul 14, 2020 at 8:37 PM Michal Hocko <mhocko@kernel.org> wrote:
[...]
> > > @@ -1560,16 +1560,31 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> > >               .gfp_mask = gfp_mask,
> > >               .order = order,
> > >       };
> > > -     bool ret;
> > > +     bool ret = true;
> > >
> > >       if (mutex_lock_killable(&oom_lock))
> > >               return true;
> > > +
> > >       /*
> > >        * A few threads which were not waiting at mutex_lock_killable() can
> > >        * fail to bail out. Therefore, check again after holding oom_lock.
> > >        */
> > > -     ret = should_force_charge() || out_of_memory(&oc);
> > > +     if (should_force_charge())
> > > +             goto out;
> > > +
> > > +     /*
> > > +      * Different tasks may be doing parallel oom, so after hold the
> > > +      * oom_lock the task should check the memcg margin again to check
> > > +      * whether other task has already made progress.
> > > +      */
> > > +     if (mem_cgroup_margin(memcg) >= (1 << order))
> > > +             goto out;
> >
> > Is there any reason why you simply haven't done this? (+ your comment
> > which is helpful).
> >
> 
> No strong reason.
> I just think that threads of a multi-thread task are more likely to do
> parallel OOM, so I checked it first.
> I can change it as you suggested below,  as it is more simple.

I would rather go with simplicity. This is a super slow path so ordering
of checks shouldn't matter much (if at all).

-- 
Michal Hocko
SUSE Labs