On Tue, Jul 14, 2020 at 9:34 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote: > > On Tue 14-07-20 21:25:04, Yafang Shao wrote: > > On Tue, Jul 14, 2020 at 8:37 PM Michal Hocko <mhocko@xxxxxxxxxx> 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). > Sure. -- Thanks Yafang
