Re: [PATCH] mm, memcg: clear page protection when memcg oom group happens

Michal Hocko
2019-11-25T11:08:53Z

On Mon 25-11-19 05:14:53, Yafang Shao wrote:
> We set memory.oom.group to make all processes in this memcg are killed by
> OOM killer to free more pages. In this case, it doesn't make sense to
> protect the pages with memroy.{min, low} again if they are set.

I do not see why? What does group OOM killing has anything to do with
the reclaim protection? What is the actual problem you are trying to
solve?

> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> include/linux/memcontrol.h | 11 +++++++++++
> mm/memcontrol.c | 4 +---
> mm/oom_kill.c | 1 +
> 3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 0c762e8..f68a1a5 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -369,6 +369,13 @@ static inline unsigned long mem_cgroup_protection(struct mem_cgroup *memcg,
> READ_ONCE(memcg->memory.elow));
> }
>
> +static inline void mem_cgroup_clear_protection(struct mem_cgroup *memcg)
> +{
> +
> + page_counter_set_min(&memcg->memory, 0);
> + page_counter_set_low(&memcg->memory, 0);
> +}
> +
> enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root,
> struct mem_cgroup *memcg);
>
> @@ -850,6 +857,10 @@ static inline unsigned long mem_cgroup_protection(struct mem_cgroup *memcg,
> return 0;
> }
>
> +static inline void mem_cgroup_clear_protection(struct mem_cgroup *memcg)
> +{
> +}
> +
> static inline enum mem_cgroup_protection mem_cgroup_protected(
> struct mem_cgroup *root, struct mem_cgroup *memcg)
> {
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1c4c08b..e5ab119 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5190,9 +5190,7 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
> }
> spin_unlock(&memcg->event_list_lock);
>
> - page_counter_set_min(&memcg->memory, 0);
> - page_counter_set_low(&memcg->memory, 0);
> -
> + mem_cgroup_clear_protection(memcg);
> memcg_offline_kmem(memcg);
> wb_memcg_offline(memcg);
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 93eae76..550f830 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -988,6 +988,7 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
> mem_cgroup_print_oom_group(oom_group);
> mem_cgroup_scan_tasks(oom_group, oom_kill_memcg_member,
> (void*)message);
> + mem_cgroup_clear_protection(oom_group);
> mem_cgroup_put(oom_group);
> }
> }
> --
> 1.8.3.1

--
Michal Hocko
SUSE Labs