| Message ID | 20200502143452.7640-3-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memcg oom: don't try to kill a process if there is no process | expand |
On Sat, May 2, 2020 at 7:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > Recently Shakeel reported a issue which also confused me serveral months > earlier. Bellow is his report - > > Lowering memory.max can trigger an oom-kill if the reclaim does not > succeed. However if oom-killer does not find a process for killing, it > dumps a lot of warnings. > > Deleting a memcg does not reclaim memory from it and the memory can > linger till there is a memory pressure. One normal way to proactively > reclaim such memory is to set memory.max to 0 just before deleting the > memcg. However if some of the memcg's memory is pinned by others, this > operation can trigger an oom-kill without any process and thus can log a > lot un-needed warnings. So, ignore all such warnings from memory.max. lot *of* un-needed > > [shakeelb@google.com: commit log above] > > A better way to avoid this issue is to avoid trying to kill a process if > memcg is not populated. > Note that OOM is different with OOM kill. different *from* > OOM is a status that the > system or memcg is out of memory, while OOM kill is a result that a > process inside this memcg is killed when this memcg is in OOM status. > That is the same reason why there're both MEMCG_OOM event and > MEMCG_OOM_KILL event. If we have already known that there's nothing to > kill, i.e. the memcg is not populated, then we don't need to have a try. need to try I think adding the discussion of memory.high is also useful in the commit message. Basically why setting memory.max to 0 is better than setting memory.high to 0 before deletion. The reason is remote charging. > > Cc: Shakeel Butt <shakeelb@google.com> > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Roman Gushchin <guro@fb.com> > Cc: Greg Thelen <gthelen@google.com> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Reviewed-by: Shakeel Butt <shakeelb@google.com> > --- > mm/memcontrol.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 985edce98491..29afe3df9d98 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6102,6 +6102,10 @@ static ssize_t memory_max_write(struct kernfs_open_file *of, > } > > memcg_memory_event(memcg, MEMCG_OOM); > + > + if (!cgroup_is_populated(memcg->css.cgroup)) > + break; > + > if (!mem_cgroup_oom_kill(memcg, GFP_KERNEL, 0)) > break; > } > -- > 2.18.2 >
On Mon, May 4, 2020 at 7:00 AM Shakeel Butt <shakeelb@google.com> wrote: > > On Sat, May 2, 2020 at 7:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > Recently Shakeel reported a issue which also confused me serveral months > > earlier. Bellow is his report - > > > > Lowering memory.max can trigger an oom-kill if the reclaim does not > > succeed. However if oom-killer does not find a process for killing, it > > dumps a lot of warnings. > > > > Deleting a memcg does not reclaim memory from it and the memory can > > linger till there is a memory pressure. One normal way to proactively > > reclaim such memory is to set memory.max to 0 just before deleting the > > memcg. However if some of the memcg's memory is pinned by others, this > > operation can trigger an oom-kill without any process and thus can log a > > lot un-needed warnings. So, ignore all such warnings from memory.max. > > lot *of* un-needed > Thanks. > > > > [shakeelb@google.com: commit log above] > > > > A better way to avoid this issue is to avoid trying to kill a process if > > memcg is not populated. > > Note that OOM is different with OOM kill. > > different *from* > Thanks > > OOM is a status that the > > system or memcg is out of memory, while OOM kill is a result that a > > process inside this memcg is killed when this memcg is in OOM status. > > That is the same reason why there're both MEMCG_OOM event and > > MEMCG_OOM_KILL event. If we have already known that there's nothing to > > kill, i.e. the memcg is not populated, then we don't need to have a try. > > need to try > Thanks > I think adding the discussion of memory.high is also useful in the > commit message. Basically why setting memory.max to 0 is better than > setting memory.high to 0 before deletion. The reason is remote > charging. > Sure, will add it. > > > > Cc: Shakeel Butt <shakeelb@google.com> > > Cc: Johannes Weiner <hannes@cmpxchg.org> > > Cc: Michal Hocko <mhocko@kernel.org> > > Cc: Roman Gushchin <guro@fb.com> > > Cc: Greg Thelen <gthelen@google.com> > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > Reviewed-by: Shakeel Butt <shakeelb@google.com> > > > --- > > mm/memcontrol.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 985edce98491..29afe3df9d98 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -6102,6 +6102,10 @@ static ssize_t memory_max_write(struct kernfs_open_file *of, > > } > > > > memcg_memory_event(memcg, MEMCG_OOM); > > + > > + if (!cgroup_is_populated(memcg->css.cgroup)) > > + break; > > + > > if (!mem_cgroup_oom_kill(memcg, GFP_KERNEL, 0)) > > break; > > } > > -- > > 2.18.2 > >
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 985edce98491..29afe3df9d98 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -6102,6 +6102,10 @@ static ssize_t memory_max_write(struct kernfs_open_file *of, } memcg_memory_event(memcg, MEMCG_OOM); + + if (!cgroup_is_populated(memcg->css.cgroup)) + break; + if (!mem_cgroup_oom_kill(memcg, GFP_KERNEL, 0)) break; }
Recently Shakeel reported a issue which also confused me serveral months earlier. Bellow is his report - Lowering memory.max can trigger an oom-kill if the reclaim does not succeed. However if oom-killer does not find a process for killing, it dumps a lot of warnings. Deleting a memcg does not reclaim memory from it and the memory can linger till there is a memory pressure. One normal way to proactively reclaim such memory is to set memory.max to 0 just before deleting the memcg. However if some of the memcg's memory is pinned by others, this operation can trigger an oom-kill without any process and thus can log a lot un-needed warnings. So, ignore all such warnings from memory.max. [shakeelb@google.com: commit log above] A better way to avoid this issue is to avoid trying to kill a process if memcg is not populated. Note that OOM is different with OOM kill. OOM is a status that the system or memcg is out of memory, while OOM kill is a result that a process inside this memcg is killed when this memcg is in OOM status. That is the same reason why there're both MEMCG_OOM event and MEMCG_OOM_KILL event. If we have already known that there's nothing to kill, i.e. the memcg is not populated, then we don't need to have a try. Cc: Shakeel Butt <shakeelb@google.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@kernel.org> Cc: Roman Gushchin <guro@fb.com> Cc: Greg Thelen <gthelen@google.com> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- mm/memcontrol.c | 4 ++++ 1 file changed, 4 insertions(+)