All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Gushchin <roman.gushchin@linux.dev>
To: Muchun Song <songmuchun@bytedance.com>
Cc: hannes@cmpxchg.org, mhocko@kernel.org, shakeelb@google.com,
	akpm@linux-foundation.org, cgroups@vger.kernel.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	duanxiongchun@bytedance.com, longman@redhat.com
Subject: Re: [PATCH v5 07/11] mm: memcontrol: make all the callers of {folio,page}_memcg() safe
Date: Sun, 19 Jun 2022 12:37:32 -0700	[thread overview]
Message-ID: <Yq96/NEanbbUUUIW@castle> (raw)
In-Reply-To: <20220530074919.46352-8-songmuchun@bytedance.com>

On Mon, May 30, 2022 at 03:49:15PM +0800, Muchun Song wrote:
> When we use objcg APIs to charge the LRU pages, the page will not hold
> a reference to the memcg associated with the page. So the caller of the
> {folio,page}_memcg() should hold an rcu read lock or obtain a reference
> to the memcg associated with the page to protect memcg from being
> released. So introduce get_mem_cgroup_from_{page,folio}() to obtain a
> reference to the memory cgroup associated with the page.
> 
> In this patch, make all the callers hold an rcu read lock or obtain a
> reference to the memcg to protect memcg from being released when the LRU
> pages reparented.
> 
> We do not need to adjust the callers of {folio,page}_memcg() during
> the whole process of mem_cgroup_move_task(). Because the cgroup migration
> and memory cgroup offlining are serialized by @cgroup_mutex. In this
> routine, the LRU pages cannot be reparented to its parent memory cgroup.
> So {folio,page}_memcg() is stable and cannot be released.
> 
> This is a preparation for reparenting the LRU pages.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>

Acked-by: Roman Gushchin <roman.gushchin@linux.dev>

The locking seems to be correct. I'm slightly worried about a potential
perf degradation, especially on dying cgroups, where css_get() is relatively
expensive. I hope getting it into mm-unstable will help to determine
whether it's actually a problem.

Thanks!

WARNING: multiple messages have this Message-ID (diff)
From: Roman Gushchin <roman.gushchin-fxUVXftIFDnyG1zEObXtfA@public.gmane.org>
To: Muchun Song <songmuchun-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org>
Cc: hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org,
	mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	duanxiongchun-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org,
	longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Subject: Re: [PATCH v5 07/11] mm: memcontrol: make all the callers of {folio,page}_memcg() safe
Date: Sun, 19 Jun 2022 12:37:32 -0700	[thread overview]
Message-ID: <Yq96/NEanbbUUUIW@castle> (raw)
In-Reply-To: <20220530074919.46352-8-songmuchun-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org>

On Mon, May 30, 2022 at 03:49:15PM +0800, Muchun Song wrote:
> When we use objcg APIs to charge the LRU pages, the page will not hold
> a reference to the memcg associated with the page. So the caller of the
> {folio,page}_memcg() should hold an rcu read lock or obtain a reference
> to the memcg associated with the page to protect memcg from being
> released. So introduce get_mem_cgroup_from_{page,folio}() to obtain a
> reference to the memory cgroup associated with the page.
> 
> In this patch, make all the callers hold an rcu read lock or obtain a
> reference to the memcg to protect memcg from being released when the LRU
> pages reparented.
> 
> We do not need to adjust the callers of {folio,page}_memcg() during
> the whole process of mem_cgroup_move_task(). Because the cgroup migration
> and memory cgroup offlining are serialized by @cgroup_mutex. In this
> routine, the LRU pages cannot be reparented to its parent memory cgroup.
> So {folio,page}_memcg() is stable and cannot be released.
> 
> This is a preparation for reparenting the LRU pages.
> 
> Signed-off-by: Muchun Song <songmuchun-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org>

Acked-by: Roman Gushchin <roman.gushchin-fxUVXftIFDnyG1zEObXtfA@public.gmane.org>

The locking seems to be correct. I'm slightly worried about a potential
perf degradation, especially on dying cgroups, where css_get() is relatively
expensive. I hope getting it into mm-unstable will help to determine
whether it's actually a problem.

Thanks!

  reply	other threads:[~2022-06-19 19:37 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-30  7:49 [PATCH v5 00/11] Use obj_cgroup APIs to charge the LRU pages Muchun Song
2022-05-30  7:49 ` Muchun Song
2022-05-30  7:49 ` [PATCH v5 01/11] mm: memcontrol: remove dead code and comments Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-06-10 23:01   ` Roman Gushchin
2022-06-10 23:01     ` Roman Gushchin
2022-05-30  7:49 ` [PATCH v5 02/11] mm: rename unlock_page_lruvec{_irq, _irqrestore} to lruvec_unlock{_irq, _irqrestore} Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-06-10 23:08   ` Roman Gushchin
2022-06-10 23:08     ` Roman Gushchin
2022-05-30  7:49 ` [PATCH v5 03/11] mm: memcontrol: prepare objcg API for non-kmem usage Muchun Song
2022-06-01 17:34   ` Michal Koutný
2022-06-01 17:34     ` Michal Koutný
2022-06-01 18:33     ` Roman Gushchin
2022-06-01 18:33       ` Roman Gushchin
2022-06-02 10:48       ` Muchun Song
2022-06-02 10:48         ` Muchun Song
2022-06-02  3:06     ` Muchun Song
2022-06-02  3:06       ` Muchun Song
2022-06-10 23:13   ` Roman Gushchin
2022-06-10 23:13     ` Roman Gushchin
2022-05-30  7:49 ` [PATCH v5 04/11] mm: memcontrol: make lruvec lock safe when LRU pages are reparented Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-05-30  7:49 ` [PATCH v5 05/11] mm: vmscan: rework move_pages_to_lru() Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-05-30  7:49 ` [PATCH v5 06/11] mm: thp: make split queue lock safe when LRU pages are reparented Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-05-30  7:49 ` [PATCH v5 07/11] mm: memcontrol: make all the callers of {folio,page}_memcg() safe Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-06-19 19:37   ` Roman Gushchin [this message]
2022-06-19 19:37     ` Roman Gushchin
2022-06-20  6:13     ` Muchun Song
2022-06-20  6:13       ` Muchun Song
2022-05-30  7:49 ` [PATCH v5 08/11] mm: memcontrol: introduce memcg_reparent_ops Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-06-19 19:47   ` Roman Gushchin
2022-06-19 19:47     ` Roman Gushchin
2022-06-20  7:14     ` Muchun Song
2022-05-30  7:49 ` [PATCH v5 09/11] mm: memcontrol: use obj_cgroup APIs to charge the LRU pages Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-06-01 17:34   ` Michal Koutný
2022-06-01 17:34     ` Michal Koutný
2022-06-02  4:14     ` Muchun Song
2022-06-02  4:14       ` Muchun Song
2022-06-19 20:32   ` Roman Gushchin
2022-06-19 20:32     ` Roman Gushchin
2022-05-30  7:49 ` [PATCH v5 10/11] mm: lru: add VM_BUG_ON_FOLIO to lru maintenance function Muchun Song
2022-06-19 19:49   ` Roman Gushchin
2022-06-19 19:49     ` Roman Gushchin
2022-05-30  7:49 ` [PATCH v5 11/11] mm: lru: use lruvec lock to serialize memcg changes Muchun Song
2022-05-30  7:49   ` Muchun Song
2022-05-30 21:17 ` [PATCH v5 00/11] Use obj_cgroup APIs to charge the LRU pages Andrew Morton
2022-05-31  2:26   ` Muchun Song
2022-05-31  2:26     ` Muchun Song
2022-05-31  2:46   ` Roman Gushchin
2022-05-31  2:46     ` Roman Gushchin
2022-05-31  2:41 ` Waiman Long
2022-05-31  7:29   ` Muchun Song
2022-05-31  7:29     ` Muchun Song
2022-06-09  2:43 ` Muchun Song
2022-06-09  2:43   ` Muchun Song
2022-06-09  2:53   ` Roman Gushchin
2022-06-09  3:09     ` Muchun Song
2022-06-09  3:09       ` 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=Yq96/NEanbbUUUIW@castle \
    --to=roman.gushchin@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=duanxiongchun@bytedance.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=mhocko@kernel.org \
    --cc=shakeelb@google.com \
    --cc=songmuchun@bytedance.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.