From: Muchun Song <songmuchun@bytedance.com>
To: guro@fb.com, hannes@cmpxchg.org, mhocko@kernel.org,
akpm@linux-foundation.org, shakeelb@google.com,
vdavydov.dev@gmail.com
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
duanxiongchun@bytedance.com, fam.zheng@bytedance.com,
bsingharora@gmail.com, shy828301@gmail.com, alexs@kernel.org,
smuchun@gmail.com, zhengqi.arch@bytedance.com,
Muchun Song <songmuchun@bytedance.com>
Subject: [PATCH v3 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented
Date: Wed, 16 Feb 2022 19:51:23 +0800 [thread overview]
Message-ID: <20220216115132.52602-4-songmuchun@bytedance.com> (raw)
In-Reply-To: <20220216115132.52602-1-songmuchun@bytedance.com>
The diagram below shows how to make the folio lruvec lock safe when LRU
pages are reparented.
folio_lruvec_lock(folio)
retry:
lruvec = folio_lruvec(folio);
// The folio is reparented at this time.
spin_lock(&lruvec->lru_lock);
if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio)))
// Acquired the wrong lruvec lock and need to retry.
// Because this folio is on the parent memcg lruvec list.
goto retry;
// If we reach here, it means that folio_memcg(folio) is stable.
memcg_reparent_objcgs(memcg)
// lruvec belongs to memcg and lruvec_parent belongs to parent memcg.
spin_lock(&lruvec->lru_lock);
spin_lock(&lruvec_parent->lru_lock);
// Move all the pages from the lruvec list to the parent lruvec list.
spin_unlock(&lruvec_parent->lru_lock);
spin_unlock(&lruvec->lru_lock);
After we acquire the lruvec lock, we need to check whether the folio is
reparented. If so, we need to reacquire the new lruvec lock. On the
routine of the LRU pages reparenting, we will also acquire the lruvec
lock (will be implemented in the later patch). So folio_memcg() cannot
be changed when we hold the lruvec lock.
Since lruvec_memcg(lruvec) is always equal to folio_memcg(folio) after
we hold the lruvec lock, lruvec_memcg_debug() check is pointless. So
remove it.
This is a preparation for reparenting the LRU pages.
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
include/linux/memcontrol.h | 18 +++----------
mm/compaction.c | 10 +++++++-
mm/memcontrol.c | 63 +++++++++++++++++++++++++++++-----------------
mm/swap.c | 4 +++
4 files changed, 56 insertions(+), 39 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 81a2720653d0..961e9f9b6567 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -737,7 +737,9 @@ static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg,
* folio_lruvec - return lruvec for isolating/putting an LRU folio
* @folio: Pointer to the folio.
*
- * This function relies on folio->mem_cgroup being stable.
+ * The lruvec can be changed to its parent lruvec when the page reparented.
+ * The caller need to recheck if it cares about this changes (just like
+ * folio_lruvec_lock() does).
*/
static inline struct lruvec *folio_lruvec(struct folio *folio)
{
@@ -756,15 +758,6 @@ struct lruvec *folio_lruvec_lock_irq(struct folio *folio);
struct lruvec *folio_lruvec_lock_irqsave(struct folio *folio,
unsigned long *flags);
-#ifdef CONFIG_DEBUG_VM
-void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio);
-#else
-static inline
-void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio)
-{
-}
-#endif
-
static inline
struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state *css){
return css ? container_of(css, struct mem_cgroup, css) : NULL;
@@ -1227,11 +1220,6 @@ static inline struct lruvec *folio_lruvec(struct folio *folio)
return &pgdat->__lruvec;
}
-static inline
-void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio)
-{
-}
-
static inline struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg)
{
return NULL;
diff --git a/mm/compaction.c b/mm/compaction.c
index 58d0e91cde49..eebe55e596fd 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -515,6 +515,8 @@ compact_folio_lruvec_lock_irqsave(struct folio *folio, unsigned long *flags,
{
struct lruvec *lruvec;
+ rcu_read_lock();
+retry:
lruvec = folio_lruvec(folio);
/* Track if the lock is contended in async mode */
@@ -527,7 +529,13 @@ compact_folio_lruvec_lock_irqsave(struct folio *folio, unsigned long *flags,
spin_lock_irqsave(&lruvec->lru_lock, *flags);
out:
- lruvec_memcg_debug(lruvec, folio);
+ if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
+ spin_unlock_irqrestore(&lruvec->lru_lock, *flags);
+ goto retry;
+ }
+
+ /* See the comments in folio_lruvec_lock(). */
+ rcu_read_unlock();
return lruvec;
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6501f5b6df4b..7c7672631456 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1178,23 +1178,6 @@ int mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
return ret;
}
-#ifdef CONFIG_DEBUG_VM
-void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio)
-{
- struct mem_cgroup *memcg;
-
- if (mem_cgroup_disabled())
- return;
-
- memcg = folio_memcg(folio);
-
- if (!memcg)
- VM_BUG_ON_FOLIO(lruvec_memcg(lruvec) != root_mem_cgroup, folio);
- else
- VM_BUG_ON_FOLIO(lruvec_memcg(lruvec) != memcg, folio);
-}
-#endif
-
/**
* folio_lruvec_lock - Lock the lruvec for a folio.
* @folio: Pointer to the folio.
@@ -1209,10 +1192,24 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio)
*/
struct lruvec *folio_lruvec_lock(struct folio *folio)
{
- struct lruvec *lruvec = folio_lruvec(folio);
+ struct lruvec *lruvec;
+
+ rcu_read_lock();
+retry:
+ lruvec = folio_lruvec(folio);
spin_lock(&lruvec->lru_lock);
- lruvec_memcg_debug(lruvec, folio);
+
+ if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
+ spin_unlock(&lruvec->lru_lock);
+ goto retry;
+ }
+
+ /*
+ * Preemption is disabled in the internal of spin_lock, which can serve
+ * as RCU read-side critical sections.
+ */
+ rcu_read_unlock();
return lruvec;
}
@@ -1232,10 +1229,20 @@ struct lruvec *folio_lruvec_lock(struct folio *folio)
*/
struct lruvec *folio_lruvec_lock_irq(struct folio *folio)
{
- struct lruvec *lruvec = folio_lruvec(folio);
+ struct lruvec *lruvec;
+ rcu_read_lock();
+retry:
+ lruvec = folio_lruvec(folio);
spin_lock_irq(&lruvec->lru_lock);
- lruvec_memcg_debug(lruvec, folio);
+
+ if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
+ spin_unlock_irq(&lruvec->lru_lock);
+ goto retry;
+ }
+
+ /* See the comments in folio_lruvec_lock(). */
+ rcu_read_unlock();
return lruvec;
}
@@ -1257,10 +1264,20 @@ struct lruvec *folio_lruvec_lock_irq(struct folio *folio)
struct lruvec *folio_lruvec_lock_irqsave(struct folio *folio,
unsigned long *flags)
{
- struct lruvec *lruvec = folio_lruvec(folio);
+ struct lruvec *lruvec;
+ rcu_read_lock();
+retry:
+ lruvec = folio_lruvec(folio);
spin_lock_irqsave(&lruvec->lru_lock, *flags);
- lruvec_memcg_debug(lruvec, folio);
+
+ if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
+ spin_unlock_irqrestore(&lruvec->lru_lock, *flags);
+ goto retry;
+ }
+
+ /* See the comments in folio_lruvec_lock(). */
+ rcu_read_unlock();
return lruvec;
}
diff --git a/mm/swap.c b/mm/swap.c
index bcf3ac288b56..9c2bcc2651c6 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -305,6 +305,10 @@ void lru_note_cost(struct lruvec *lruvec, bool file, unsigned int nr_pages)
void lru_note_cost_folio(struct folio *folio)
{
+ /*
+ * The rcu read lock is held by the caller, so we do not need to
+ * care about the lruvec returned by folio_lruvec() being released.
+ */
lru_note_cost(folio_lruvec(folio), folio_is_file_lru(folio),
folio_nr_pages(folio));
}
--
2.11.0
next prev parent reply other threads:[~2022-02-16 11:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-16 11:51 [PATCH v3 00/12] Use obj_cgroup APIs to charge the LRU pages Muchun Song
2022-02-16 11:51 ` [PATCH v3 01/12] mm: memcontrol: prepare objcg API for non-kmem usage Muchun Song
2022-02-16 11:51 ` [PATCH v3 02/12] mm: memcontrol: introduce compact_folio_lruvec_lock_irqsave Muchun Song
2022-02-16 11:51 ` Muchun Song [this message]
2022-02-16 11:51 ` [PATCH v3 04/12] mm: vmscan: rework move_pages_to_lru() Muchun Song
2022-02-16 11:51 ` [PATCH v3 05/12] mm: thp: introduce folio_split_queue_lock{_irqsave}() Muchun Song
2022-02-16 11:51 ` [PATCH v3 06/12] mm: thp: make split queue lock safe when LRU pages are reparented Muchun Song
2022-02-16 11:51 ` [PATCH v3 07/12] mm: memcontrol: make all the callers of {folio,page}_memcg() safe Muchun Song
2022-02-16 11:51 ` [PATCH v3 08/12] mm: memcontrol: introduce memcg_reparent_ops Muchun Song
2022-02-16 11:51 ` [PATCH v3 09/12] mm: memcontrol: use obj_cgroup APIs to charge the LRU pages Muchun Song
2022-02-22 13:46 ` [mm] edd4aa55af: WARNING:possible_recursive_locking_detected kernel test robot
2022-02-16 11:51 ` [PATCH v3 10/12] mm: memcontrol: rename {un}lock_page_memcg() to {un}lock_page_objcg() Muchun Song
2022-02-16 11:51 ` [PATCH v3 11/12] mm: lru: add VM_BUG_ON_FOLIO to lru maintenance function Muchun Song
2022-02-16 11:51 ` [PATCH v3 12/12] mm: lru: use lruvec lock to serialize memcg changes 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=20220216115132.52602-4-songmuchun@bytedance.com \
--to=songmuchun@bytedance.com \
--cc=akpm@linux-foundation.org \
--cc=alexs@kernel.org \
--cc=bsingharora@gmail.com \
--cc=duanxiongchun@bytedance.com \
--cc=fam.zheng@bytedance.com \
--cc=guro@fb.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=smuchun@gmail.com \
--cc=vdavydov.dev@gmail.com \
--cc=zhengqi.arch@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox