Re: [Resend PATCH 2/6] mm/memcg: remove useless check on page->mem_cgroup

From: Michal Hocko
Date: Tue Aug 11 2020 - 07:30:12 EST


subject line looks like a left over. It doesn't match the path. Did you
mean
memcg: bail out early from swap accounting when memcg is disabled?

Btw. if this patch was first in the series then you wouldn't need to
mention the warnings that would trigger based on your previous patch.
I am fine with both ways but mentioning the warning is usefule.

On Tue 11-08-20 19:10:28, Alex Shi wrote:
> If we disabled memcg by cgroup_disable=memory, the swap charges are
> still called. Let's return from the funcs earlier and keep WARN_ON
> monitor.
>
> Signed-off-by: Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx>
> Reviewed-by: Roman Gushchin <guro@xxxxxx>
> Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
> Cc: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: cgroups@xxxxxxxxxxxxxxx
> Cc: linux-mm@xxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
> mm/memcontrol.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 299382fc55a9..419cf565f40b 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7098,6 +7098,9 @@ void mem_cgroup_swapout(struct page *page, swp_entry_t entry)
> VM_BUG_ON_PAGE(PageLRU(page), page);
> VM_BUG_ON_PAGE(page_count(page), page);
>
> + if (mem_cgroup_disabled())
> + return;
> +
> if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
> return;
>
> @@ -7163,6 +7166,9 @@ int mem_cgroup_try_charge_swap(struct page *page, swp_entry_t entry)
> struct mem_cgroup *memcg;
> unsigned short oldid;
>
> + if (mem_cgroup_disabled())
> + return 0;
> +
> if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> return 0;
>
> --
> 1.8.3.1

--
Michal Hocko
SUSE Labs