All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Gushchin <guro@fb.com>
To: Muchun Song <songmuchun@bytedance.com>
Cc: <hannes@cmpxchg.org>, <mhocko@kernel.org>,
	<akpm@linux-foundation.org>, <shakeelb@google.com>,
	<vdavydov.dev@gmail.com>, <linux-kernel@vger.kernel.org>,
	<linux-mm@kvack.org>, <duanxiongchun@bytedance.com>,
	<fam.zheng@bytedance.com>, <bsingharora@gmail.com>,
	<shy828301@gmail.com>, <alex.shi@linux.alibaba.com>
Subject: Re: [RFC PATCH v3 01/12] mm: memcontrol: move the objcg infrastructure out of CONFIG_MEMCG_KMEM
Date: Tue, 25 May 2021 09:27:30 -0700	[thread overview]
Message-ID: <YK0lckLSLxhZMk3S@carbon.dhcp.thefacebook.com> (raw)
In-Reply-To: <20210421070059.69361-2-songmuchun@bytedance.com>

On Wed, Apr 21, 2021 at 03:00:48PM +0800, Muchun Song wrote:
> Because memory allocations pinning memcgs for a long time - it exists
> at a larger scale and is causing recurring problems in the real world:
> page cache doesn't get reclaimed for a long time, or is used by the
> second, third, fourth, ... instance of the same job that was restarted
> into a new cgroup every time. Unreclaimable dying cgroups pile up,
> waste memory, and make page reclaim very inefficient.
> 
> We can convert LRU pages and most other raw memcg pins to the objcg
> direction to fix this problem, and then the page->memcg will always
> point to an object cgroup pointer.
> 
> Therefore, the infrastructure of objcg no longer only serves
> CONFIG_MEMCG_KMEM. In this patch, we move the infrastructure of the
> objcg out of the scope of the CONFIG_MEMCG_KMEM so that the LRU pages
> can reuse it to charge pages.
> 
> We know that the LRU pages are not accounted at the root level. But
> the page->memcg_data points to the root_mem_cgroup. So the
> page->memcg_data of the LRU pages always points to a valid pointer.
> But the root_mem_cgroup dose not have an object cgroup. If we use
> obj_cgroup APIs to charge the LRU pages, we should set the
> page->memcg_data to a root object cgroup. So we also allocate an
> object cgroup for the root_mem_cgroup.

Overall the patch looks very good to me. There are few small things to enhance:

1) I'd rename it. Looking at the title I expect a trivial code move,
   however the patch is doing more than this: e.g. allocating an objcg
   for the root memcg. Something like "prepare objcg API for non-kmem usage".
2) How about obj_cgroup_release_kmem() instead of obj_cgroup_release_uncharge()?
3) The first paragraph of the commit log looks a bit vague: which allocations
   pinning memcgs? How about something like this?

Pagecache pages are charged at the allocation time and holding a reference
to the original memory cgroup until being reclaimed. Depending on the memory
pressure, specific patterns of the page sharing between different cgroups and
the cgroup creation and destruction rates, a large number of dying memory
cgroups can be pinned by pagecache pages. It makes the page reclaim less
efficient and wastes memory.


Thanks!

  reply	other threads:[~2021-05-25 16:27 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21  7:00 [RFC PATCH v3 00/12] Use obj_cgroup APIs to charge the LRU pages Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 01/12] mm: memcontrol: move the objcg infrastructure out of CONFIG_MEMCG_KMEM Muchun Song
2021-05-25 16:27   ` Roman Gushchin [this message]
2021-05-26  2:46     ` [External] " Muchun Song
2021-05-26  2:46       ` Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 02/12] mm: memcontrol: introduce compact_lock_page_lruvec_irqsave Muchun Song
2021-05-25 17:21   ` Roman Gushchin
2021-05-26  2:49     ` [External] " Muchun Song
2021-05-26  2:49       ` Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 03/12] mm: memcontrol: make lruvec lock safe when the LRU pages reparented Muchun Song
2021-05-25 17:21   ` Roman Gushchin
2021-04-21  7:00 ` [RFC PATCH v3 04/12] mm: vmscan: rework move_pages_to_lru() Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 05/12] mm: thp: introduce lock/unlock_split_queue{_irqsave}() Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 06/12] mm: thp: make deferred split queue lock safe when the LRU pages reparented Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 07/12] mm: memcontrol: make all the callers of page_memcg() safe Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 08/12] mm: memcontrol: introduce memcg_reparent_ops Muchun Song
2021-05-25 17:46   ` Roman Gushchin
2021-05-26  3:01     ` [External] " Muchun Song
2021-05-26  3:01       ` Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 09/12] mm: memcontrol: use obj_cgroup APIs to charge the LRU pages Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 10/12] mm: memcontrol: rename {un}lock_page_memcg() to {un}lock_page_objcg() Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 11/12] mm: lru: add VM_BUG_ON_PAGE to lru maintenance function Muchun Song
2021-04-21  7:00 ` [RFC PATCH v3 12/12] mm: lru: use lruvec lock to serialize memcg changes Muchun Song
2021-05-18 13:49 ` [RFC PATCH v3 00/12] Use obj_cgroup APIs to charge the LRU pages Muchun Song
2021-05-18 13:49   ` Muchun Song
2021-05-18 14:17   ` Roman Gushchin
2021-05-20  3:20     ` [External] " Muchun Song
2021-05-20  3:20       ` Muchun Song
2021-05-25 17:35       ` Roman Gushchin
2021-05-26  2:41         ` Muchun Song
2021-05-26  2:41           ` Muchun Song

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=YK0lckLSLxhZMk3S@carbon.dhcp.thefacebook.com \
    --to=guro@fb.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.shi@linux.alibaba.com \
    --cc=bsingharora@gmail.com \
    --cc=duanxiongchun@bytedance.com \
    --cc=fam.zheng@bytedance.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=shakeelb@google.com \
    --cc=shy828301@gmail.com \
    --cc=songmuchun@bytedance.com \
    --cc=vdavydov.dev@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.