From: Johannes Weiner <hannes@cmpxchg.org>
To: "Michal Koutný" <mkoutny@suse.com>
Cc: Richard Palethorpe <rpalethorpe@suse.com>,
Roman Gushchin <guro@fb.com>,
ltp@lists.linux.it, Andrew Morton <akpm@linux-foundation.org>,
Shakeel Butt <shakeelb@google.com>,
Christoph Lameter <cl@linux.com>,
Michal Hocko <mhocko@kernel.org>, Tejun Heo <tj@kernel.org>,
Vlastimil Babka <vbabka@suse.cz>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] mm: memcg/slab: Stop reparented obj_cgroups from charging root
Date: Fri, 16 Oct 2020 10:53:08 -0400 [thread overview]
Message-ID: <20201016145308.GA312010@cmpxchg.org> (raw)
In-Reply-To: <20201016094702.GA95052@blackbook>
On Fri, Oct 16, 2020 at 11:47:02AM +0200, Michal Koutný wrote:
> Hello.
>
> On Wed, Oct 14, 2020 at 08:07:49PM +0100, Richard Palethorpe <rpalethorpe@suse.com> wrote:
> > SLAB objects which outlive their memcg are moved to their parent
> > memcg where they may be uncharged. However if they are moved to the
> > root memcg, uncharging will result in negative page counter values as
> > root has no page counters.
> Why do you think those are reparented objects? If those are originally
> charged in a non-root cgroup, then the charge value should be propagated up the
> hierarchy, including root memcg, so if they're later uncharged in root
> after reparenting, it should still break even. (Or did I miss some stock
> imbalance?)
Looking a bit closer at this code, it's kind of a mess right now.
The central try_charge() function charges recursively all the way up
to and including the root. But not if it's called directly on the
root, in which case it bails and does nothing.
kmem and objcg use try_charge(), so they have the same
behavior. get_obj_cgroup_from_current() does it's own redundant
filtering for root_mem_cgroup, whereas get_mem_cgroup_from_current()
does not, but its callsite __memcg_kmem_charge_page() does.
We should clean this up one way or another: either charge the root or
don't, but do it consistently.
Since we export memory.stat at the root now, we should probably just
always charge the root instead of special-casing it all over the place
and risking bugs.
Indeed, it looks like there is at least one bug where the root-level
memory.stat shows non-root slab objects, but not root ones, whereas it
shows all anon and cache pages, root or no root.
next prev parent reply other threads:[~2020-10-16 14:54 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-14 19:07 [RFC PATCH] mm: memcg/slab: Stop reparented obj_cgroups from charging root Richard Palethorpe
2020-10-14 20:08 ` Roman Gushchin
2020-10-16 5:40 ` Richard Palethorpe
2020-10-16 9:47 ` Michal Koutný
2020-10-16 10:41 ` Richard Palethorpe
2020-10-16 15:05 ` Richard Palethorpe
2020-10-16 17:26 ` Michal Koutný
2020-10-16 14:53 ` Johannes Weiner [this message]
2020-10-16 17:02 ` Roman Gushchin
2020-10-16 17:15 ` Michal Koutný
2020-10-19 8:45 ` Richard Palethorpe
2020-10-19 9:58 ` [PATCH v3] " Richard Palethorpe
2020-10-19 16:58 ` Shakeel Butt
2020-10-20 5:52 ` Richard Palethorpe
2020-10-20 13:49 ` Richard Palethorpe
2020-10-20 16:56 ` Shakeel Butt
2020-10-21 20:32 ` Roman Gushchin
2020-10-20 17:24 ` Michal Koutný
2020-10-22 7:04 ` Richard Palethorpe
2020-10-22 12:28 ` [PATCH v4] " Richard Palethorpe
2020-10-22 16:37 ` Shakeel Butt
2020-10-22 17:25 ` Roman Gushchin
2020-10-22 23:59 ` Shakeel Butt
2020-10-23 0:40 ` Roman Gushchin
2020-10-23 15:44 ` Johannes Weiner
2020-10-23 16:41 ` Shakeel Butt
2020-10-26 7:32 ` Richard Palethorpe
2020-10-26 23:14 ` Roman Gushchin
2020-10-19 22:28 ` [RFC PATCH] " Roman Gushchin
2020-10-20 6:04 ` Richard Palethorpe
2020-10-20 12:02 ` Richard Palethorpe
2020-10-20 14:48 ` Richard Palethorpe
2020-10-20 16:27 ` Michal Koutný
2020-10-20 17:07 ` Roman Gushchin
2020-10-20 18:18 ` Johannes Weiner
2020-10-21 19:33 ` Roman Gushchin
2020-10-23 16:30 ` Johannes Weiner
2020-11-10 1:27 ` Roman Gushchin
2020-11-10 15:11 ` Shakeel Butt
2020-11-10 19:13 ` Roman Gushchin
2020-11-20 17:46 ` Michal Koutný
2020-11-03 13:22 ` Michal Hocko
2020-11-03 21:30 ` Roman Gushchin
2020-10-20 16:55 ` Shakeel Butt
2020-10-20 17:17 ` Roman Gushchin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201016145308.GA312010@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=guro@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=ltp@lists.linux.it \
--cc=mhocko@kernel.org \
--cc=mkoutny@suse.com \
--cc=rpalethorpe@suse.com \
--cc=shakeelb@google.com \
--cc=tj@kernel.org \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).