linux-kernel - Re: [PATCH] mm: memcg/slab: enable slab memory accounting atomically

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201110152143.GA842337@cmpxchg.org>
Date:   Tue, 10 Nov 2020 10:21:43 -0500
From:   Johannes Weiner <hannes@...xchg.org>
To:     Roman Gushchin <guro@...com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        Shakeel Butt <shakeelb@...gle.com>,
        Michal Hocko <mhocko@...nel.org>, linux-kernel@...r.kernel.org,
        kernel-team@...com
Subject: Re: [PATCH] mm: memcg/slab: enable slab memory accounting atomically

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.