Message-ID: <20200513005741.GA67541@carbon.dhcp.thefacebook.com>
Date:   Tue, 12 May 2020 17:57:41 -0700
From:   Roman Gushchin <guro@...com>
To:     Christopher Lameter <cl@...ux.com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Michal Hocko <mhocko@...nel.org>, <linux-mm@...ck.org>,
        <kernel-team@...com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 04/19] mm: slub: implement SLUB version of
 obj_to_index()

On Fri, May 08, 2020 at 09:35:54PM +0000, Christoph Lameter wrote:
> On Mon, 4 May 2020, Roman Gushchin wrote:
> 
> > On Sat, May 02, 2020 at 11:54:09PM +0000, Christoph Lameter wrote:
> > > On Thu, 30 Apr 2020, Roman Gushchin wrote:
> > >
> > > > Sorry, but what exactly do you mean?
> > >
> > > I think the right approach is to add a pointer to each slab object for
> > > memcg support.
> > >
> >
> > As I understand, embedding the memcg pointer will hopefully make allocations
> > cheaper in terms of CPU, but will require more memory. And you think that
> > it's worth it. Is it a correct understanding?
> 
> It definitely makes the code less complex. The additional memory is
> minimal. In many cases you have already some space wasted at the end of
> the object that could be used for the pointer.
> 
> > Can you, please, describe a bit more detailed how it should be done
> > from your point of view?
> 
> Add it to the metadata at the end of the object. Like the debugging
> information or the pointer for RCU freeing.

Enabling debugging metadata currently disables the cache merging.
I doubt that it's acceptable to sacrifice the cache merging in order
to embed the memcg pointer?

> 
> > I mean where to store the pointer, should it be SLAB/SLUB-specific code
> > or a generic code, what do to with kmallocs alignments, should we
> > merge slabs which had a different size before and now have the same
> > because of the memcg pointer and aligment, etc.
> 
> Both SLAB and SLUB have the same capabilities there. Slabs that had
> different sizes before will now have different sizes as well. So the
> merging does not change.

See above. Or should I add it to the object itself before the metadata?

> 
> > I'm happy to follow your advice and perform some tests to get an idea of
> > how significant the memory overhead is and how big are CPU savings.
> > I guess with these numbers it will be easy to make a decision.
> 
> Sure. The main issue are the power of two kmalloc caches and how to add
> the pointer to these caches in order not to waste memory. SLAB has done
> this in the past by creating additional structues in a page frame.

But isn't it then similar to what I'm doing now?

Btw, I'm trying to build up a prototype with an embedded memcg pointer,
but it seems to be way more tricky than I thought. It requires changes to
shrinkers (as they rely on getting the memcg pointer by an arbitrary
kernel address, not necessarily aligned to the head of slab allocation),
figuring out cache merging, adding SLAB support, natural alignment of
kmallocs etc.

Figuring out all these details will likely take several weeks, so the whole
thing will be delayed for one-two major releases (in the best case). Given that
the current implementation saves ~40% of slab memory, I think there is some value
in delivering it as it is. So I wonder if the idea of embedding the pointer
should be considered a blocker, or it can be implemented of top of the proposed
code (given it's not a user-facing api or something like this)?

Thanks!

Powered by blists - more mailing lists