Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 10 Nov 2020 10:21:43 -0500
From:   Johannes Weiner <hannes@cmpxchg.org>
To:     Roman Gushchin <guro@fb.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        Shakeel Butt <shakeelb@google.com>,
        Michal Hocko <mhocko@kernel.org>, linux-kernel@vger.kernel.org,
        kernel-team@fb.com
Subject: Re: [PATCH] mm: memcg/slab: enable slab memory accounting atomically
Message-ID: <20201110152143.GA842337@cmpxchg.org>
References: <20201110010615.1273043-1-guro@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201110010615.1273043-1-guro@fb.com>
Precedence: bulk

On Mon, Nov 09, 2020 at 05:06:15PM -0800, Roman Gushchin wrote:
> Many kernel memory accounting paths are guarded by the
> memcg_kmem_enabled_key static key. It changes it's state during
> the onlining of the first non-root cgroup. However is doesn't
> happen atomically: before all call sites will become patched
> some charges/uncharges can be skipped, resulting in an unbalanced
> charge. The problem is mostly a theoretical issue, unlikely
> having a noticeable impact ion the real life.

memcg_online_kmem() is called from .css_alloc rather than .css_online,
at a time where memcg is brandnew and the css hasn't been set up and
published yet (the refcount isn't even initialized for css_tryget()).

I don't really see how charges could race with that.

We may want to rename memcg_online_kmem() to memcg_alloc_kmem() or
something.

> Before the rework of the slab controller we relied at setting
> kmemcg_id after enabling the memcg_kmem_enabled_key static key.
> Now we can use the setting of memcg->objcg to enable the slab
> memory accounting atomically.

I suspect that code comment has been out of date since commit
0b8f73e104285a4badf9d768d1c39b06d77d1f97.