| Message ID | 20200504042621.10334-2-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 Mon 04-05-20 00:26:20, Yafang Shao wrote: > Rename mem_cgroup_out_of_memory() to mem_cgroup_oom_kill() to indicate > that this function is used to try to kill a process. > With this change it will cooperate better with the oom events. > function memcg event > mem_cgroup_oom() oom > mem_cgroup_oom_kill() oom_kill > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > Reviewed-by: 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> Well, I do not have a strong opinion on this. I believe the idea behind the naming was that this is a memcg counterpart for out_of_memory. So to be honest I do not consider this an improvement. > --- > mm/memcontrol.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 5beea03dd58a..985edce98491 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1570,7 +1570,7 @@ unsigned long mem_cgroup_size(struct mem_cgroup *memcg) > return page_counter_read(&memcg->memory); > } > > -static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask, > +static bool mem_cgroup_oom_kill(struct mem_cgroup *memcg, gfp_t gfp_mask, > int order) > { > struct oom_control oc = { > @@ -1799,7 +1799,7 @@ static enum oom_status mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int > * relying on the oom victim to make a forward progress and we can > * invoke the oom killer here. > * > - * Please note that mem_cgroup_out_of_memory might fail to find a > + * Please note that mem_cgroup_oom_kill might fail to find a > * victim and then we have to bail out from the charge path. > */ > if (memcg->oom_kill_disable) { > @@ -1821,7 +1821,7 @@ static enum oom_status mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int > mem_cgroup_oom_notify(memcg); > > mem_cgroup_unmark_under_oom(memcg); > - if (mem_cgroup_out_of_memory(memcg, mask, order)) > + if (mem_cgroup_oom_kill(memcg, mask, order)) > ret = OOM_SUCCESS; > else > ret = OOM_FAILED; > @@ -1879,7 +1879,7 @@ bool mem_cgroup_oom_synchronize(bool handle) > if (locked && !memcg->oom_kill_disable) { > mem_cgroup_unmark_under_oom(memcg); > finish_wait(&memcg_oom_waitq, &owait.wait); > - mem_cgroup_out_of_memory(memcg, current->memcg_oom_gfp_mask, > + mem_cgroup_oom_kill(memcg, current->memcg_oom_gfp_mask, > current->memcg_oom_order); > } else { > schedule(); > @@ -6102,7 +6102,7 @@ static ssize_t memory_max_write(struct kernfs_open_file *of, > } > > memcg_memory_event(memcg, MEMCG_OOM); > - if (!mem_cgroup_out_of_memory(memcg, GFP_KERNEL, 0)) > + if (!mem_cgroup_oom_kill(memcg, GFP_KERNEL, 0)) > break; > } > > -- > 2.18.2
Yafang Shao writes: >Rename mem_cgroup_out_of_memory() to mem_cgroup_oom_kill() to indicate >that this function is used to try to kill a process. >With this change it will cooperate better with the oom events. > function memcg event > mem_cgroup_oom() oom > mem_cgroup_oom_kill() oom_kill Hmm. The reason it's called mem_cgroup_out_of_memory() is, as Michal said, to match out_of_memory, which may or may not OOM kill. Internally, mem_cgroup_out_of_memory() may also do this depending on the state of oom_lock and the current task, so mem_cgroup_out_of_memory() is really about *deciding* what to do when generically out of memory rather than necessarily OOM killing something (although yes, that's the general outcome). Is matching the memcg event names the only reason you'd like to do this, or are there more concerns you had? If that's the only case, I think let's keep it as is for now. :-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 5beea03dd58a..985edce98491 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1570,7 +1570,7 @@ unsigned long mem_cgroup_size(struct mem_cgroup *memcg) return page_counter_read(&memcg->memory); } -static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask, +static bool mem_cgroup_oom_kill(struct mem_cgroup *memcg, gfp_t gfp_mask, int order) { struct oom_control oc = { @@ -1799,7 +1799,7 @@ static enum oom_status mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int * relying on the oom victim to make a forward progress and we can * invoke the oom killer here. * - * Please note that mem_cgroup_out_of_memory might fail to find a + * Please note that mem_cgroup_oom_kill might fail to find a * victim and then we have to bail out from the charge path. */ if (memcg->oom_kill_disable) { @@ -1821,7 +1821,7 @@ static enum oom_status mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int mem_cgroup_oom_notify(memcg); mem_cgroup_unmark_under_oom(memcg); - if (mem_cgroup_out_of_memory(memcg, mask, order)) + if (mem_cgroup_oom_kill(memcg, mask, order)) ret = OOM_SUCCESS; else ret = OOM_FAILED; @@ -1879,7 +1879,7 @@ bool mem_cgroup_oom_synchronize(bool handle) if (locked && !memcg->oom_kill_disable) { mem_cgroup_unmark_under_oom(memcg); finish_wait(&memcg_oom_waitq, &owait.wait); - mem_cgroup_out_of_memory(memcg, current->memcg_oom_gfp_mask, + mem_cgroup_oom_kill(memcg, current->memcg_oom_gfp_mask, current->memcg_oom_order); } else { schedule(); @@ -6102,7 +6102,7 @@ static ssize_t memory_max_write(struct kernfs_open_file *of, } memcg_memory_event(memcg, MEMCG_OOM); - if (!mem_cgroup_out_of_memory(memcg, GFP_KERNEL, 0)) + if (!mem_cgroup_oom_kill(memcg, GFP_KERNEL, 0)) break; }