On Thu, Aug 13, 2020 at 11:20:33AM -0400, Johannes Weiner wrote:
> On Thu, Aug 13, 2020 at 04:46:54PM +1000, Stephen Rothwell wrote:
> > [ 0.055220][ T0] WARNING: CPU: 0 PID: 0 at mm/memcontrol.c:5220 mem_cgroup_css_alloc+0x350/0x904
>
> > [The line numbers in the final linux next are 5226 and 5141 due to
> > later patches.]
> >
> > Introduced (or exposed) by commit
> >
> > 3e38e0aaca9e ("mm: memcg: charge memcg percpu memory to the parent cgroup")
> >
> > This commit actually adds the WARN_ON, so it either adds the bug that
> > sets it off, or the bug already existed.
> >
> > Unfotunately, the version of this patch in linux-next up tuntil today
> > is different. :-(
>
> Sorry, I made a last-minute request to include these checks in that
> patch to make the code a bit more robust, but they trigger a false
> positive here. Let's remove them.
>
> ---
>
> From de8ea7c96c056c3cbe7b93995029986a158fb9cd Mon Sep 17 00:00:00 2001
> From: Johannes Weiner <hannes@xxxxxxxxxxx>
> Date: Thu, 13 Aug 2020 10:40:54 -0400
> Subject: [PATCH] mm: memcontrol: fix warning when allocating the root cgroup
>
> Commit 3e38e0aaca9e ("mm: memcg: charge memcg percpu memory to the
> parent cgroup") adds memory tracking to the memcg kernel structures
> themselves to make cgroups liable for the memory they are consuming
> through the allocation of child groups (which can be significant).
>
> This code is a bit awkward as it's spread out through several
> functions: The outermost function does memalloc_use_memcg(parent) to
> set up current->active_memcg, which designates which cgroup to charge,
> and the inner functions pass GFP_ACCOUNT to request charging for
> specific allocations. To make sure this dependency is satisfied at all
> times - to make sure we don't randomly charge whoever is calling the
> functions - the inner functions warn on !current->active_memcg.
>
> However, this triggers a false warning when the root memcg itself is
> allocated. No parent exists in this case, and so current->active_memcg
> is rightfully NULL. It's a false positive, not indicative of a bug.
>
> Delete the warnings for now, we can revisit this later.
>
> Fixes: 3e38e0aaca9e ("mm: memcg: charge memcg percpu memory to the parent cgroup")
> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
Acked-by: Roman Gushchin <guro@xxxxxx>
Thanks!
> ---
> mm/memcontrol.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index d59fd9af6e63..9d87082e64aa 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5137,9 +5137,6 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
> if (!pn)
> return 1;
>
> - /* We charge the parent cgroup, never the current task */
> - WARN_ON_ONCE(!current->active_memcg);
> -
> pn->lruvec_stat_local = alloc_percpu_gfp(struct lruvec_stat,
> GFP_KERNEL_ACCOUNT);
> if (!pn->lruvec_stat_local) {
> @@ -5222,9 +5219,6 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
> goto fail;
> }
>
> - /* We charge the parent cgroup, never the current task */
> - WARN_ON_ONCE(!current->active_memcg);
> -
> memcg->vmstats_local = alloc_percpu_gfp(struct memcg_vmstats_percpu,
> GFP_KERNEL_ACCOUNT);
> if (!memcg->vmstats_local)
> --
> 2.28.0
>
