On Mon 17-06-19 18:56:47, Tetsuo Handa wrote: > On 2019/06/17 15:33, Michal Hocko wrote: > > On Sat 15-06-19 09:11:37, Shakeel Butt wrote: > >> On Sat, Jun 15, 2019 at 6:50 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote: > > [...] > >>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c > >>> index 5a58778c91d4..43eb479a5dc7 100644 > >>> --- a/mm/oom_kill.c > >>> +++ b/mm/oom_kill.c > >>> @@ -161,8 +161,8 @@ static bool oom_unkillable_task(struct task_struct *p, > >>> return true; > >>> > >>> /* When mem_cgroup_out_of_memory() and p is not member of the group */ > >>> - if (memcg && !task_in_mem_cgroup(p, memcg)) > >>> - return true; > >>> + if (memcg) > >>> + return false; > >> > >> This will break the dump_tasks() usage of oom_unkillable_task(). We > >> can change dump_tasks() to traverse processes like > >> mem_cgroup_scan_tasks() for memcg OOMs. > > > > Right you are. Doing a similar trick to the oom victim selection is > > indeed better. We should really strive to not doing a global process > > iteration when we can do a targeted scan. Care to send a patch? > > I posted a patch that (as a side effect) avoids oom_unkillable_task() from dump_tasks() at > https://lore.kernel.org/linux-mm/1558519686-16057-2-git-send-email-penguin-kernel@xxxxxxxxxxxxxxxxxxx/ . > What do you think? I am sorry but I didn't get to look at this series yet. Anyway, changing the dump_tasks to use a cgroup iterator for the memcg oom sounds like a straight forward thing to do without making much more changes around. Global task list iteration under a single RCU is a more complex problem that is not limited to the OOM path. -- Michal Hocko SUSE Labs
