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:   Fri, 30 Apr 2021 10:49:03 +1000
From:   Dave Chinner <david@fromorbit.com>
To:     Muchun Song <songmuchun@bytedance.com>
Cc:     willy@infradead.org, akpm@linux-foundation.org, hannes@cmpxchg.org,
        mhocko@kernel.org, vdavydov.dev@gmail.com, shakeelb@google.com,
        guro@fb.com, shy828301@gmail.com, alexs@kernel.org,
        alexander.h.duyck@linux.intel.com, richard.weiyang@gmail.com,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
Subject: Re: [PATCH 0/9] Shrink the list lru size on memory cgroup removal
Message-ID: <20210430004903.GF1872259@dread.disaster.area>
References: <20210428094949.43579-1-songmuchun@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210428094949.43579-1-songmuchun@bytedance.com>
Precedence: bulk

On Wed, Apr 28, 2021 at 05:49:40PM +0800, Muchun Song wrote:
> In our server, we found a suspected memory leak problem. The kmalloc-32
> consumes more than 6GB of memory. Other kmem_caches consume less than 2GB
> memory.
> 
> After our in-depth analysis, the memory consumption of kmalloc-32 slab
> cache is the cause of list_lru_one allocation.
> 
>   crash> p memcg_nr_cache_ids
>   memcg_nr_cache_ids = $2 = 24574
> 
> memcg_nr_cache_ids is very large and memory consumption of each list_lru
> can be calculated with the following formula.
> 
>   num_numa_node * memcg_nr_cache_ids * 32 (kmalloc-32)
> 
> There are 4 numa nodes in our system, so each list_lru consumes ~3MB.
> 
>   crash> list super_blocks | wc -l
>   952

The more I see people trying to work around this, the more I think
that the way memcgs have been grafted into the list_lru is back to
front.

We currently allocate scope for every memcg to be able to tracked on
every not on every superblock instantiated in the system, regardless
of whether that superblock is even accessible to that memcg.

These huge memcg counts come from container hosts where memcgs are
confined to just a small subset of the total number of superblocks
that instantiated at any given point in time.

IOWs, for these systems with huge container counts, list_lru does
not need the capability of tracking every memcg on every superblock.

What it comes down to is that the list_lru is only needed for a
given memcg if that memcg is instatiating and freeing objects on a
given list_lru.

Which makes me think we should be moving more towards "add the memcg
to the list_lru at the first insert" model rather than "instantiate
all at memcg init time just in case". The model we originally came
up with for supprting memcgs is really starting to show it's limits,
and we should address those limitations rahter than hack more
complexity into the system that does nothing to remove the
limitations that are causing the problems in the first place.

> Every mount will register 2 list lrus, one is for inode, another is for
> dentry. There are 952 super_blocks. So the total memory is 952 * 2 * 3
> MB (~5.6GB).  But the number of memory cgroup is less than 500. So I
> guess more than 12286 containers have been deployed on this machine (I
> do not know why there are so many containers, it may be a user's bug or
> the user really want to do that). But now there are less than 500
> containers in the system. And memcg_nr_cache_ids has not been reduced
> to a suitable value. This can waste a lot of memory. If we want to reduce
> memcg_nr_cache_ids, we have to reboot the server. This is not what we
> want.

Exactly my point. This model is broken and doesn't scale to large
counts of either memcgs or superblocks.

We need a different model for dynamically adding, removing and
mapping memcgs to LRU lists they are actually using so that we can
efficiently scale to tens of thousands of memcgs instances along
with tens of thousands of unique superblock instants. That's the
real problem that needs solving here.

> So this patchset will dynamically adjust the value of memcg_nr_cache_ids
> to keep healthy memory consumption.

Gigabytes of RAM for largely unused memcg list_lrus on every
superblock is not healthy. It's highly inefficient because the
infrastructure we currently have was never designed to scale to
these numbers of unique containers and superblocks...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com