[PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[Muchun Song]

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>
Acked-by: Roman Gushchin <guro@fb.com>
---
 include/linux/memcontrol.h | 18 +++-----------
 mm/compaction.c            | 10 +++++++-
 mm/memcontrol.c            | 62 +++++++++++++++++++++++++++++-----------------
 mm/swap.c                  |  4 +++
 4 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 41a35de93d75..5a8f85bd9bbf 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -769,7 +769,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)
 {
@@ -788,15 +790,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;
@@ -1243,11 +1236,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 3131558a7c31..97d72f6b3dd5 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 7df2176e4f02..7955da38e385 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1146,23 +1146,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 and return lruvec for a given folio.
  * @folio: Pointer to the folio.
@@ -1175,20 +1158,43 @@ 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;
 }
 
 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;
 }
@@ -1196,10 +1202,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 2bca632b73df..9554ff008fe6 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -295,6 +295,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

Re: [PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7182 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210814052519.86679-4-songmuchun@bytedance.com>
References: <20210814052519.86679-4-songmuchun@bytedance.com>
TO: Muchun Song <songmuchun@bytedance.com>
TO: guro(a)fb.com
TO: hannes(a)cmpxchg.org
TO: mhocko(a)kernel.org
TO: akpm(a)linux-foundation.org
TO: shakeelb(a)google.com
TO: vdavydov.dev(a)gmail.com
CC: linux-kernel(a)vger.kernel.org
CC: linux-mm(a)kvack.org
CC: duanxiongchun(a)bytedance.com
CC: fam.zheng(a)bytedance.com

Hi Muchun,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20210813]
[cannot apply to hnaz-linux-mm/master cgroup/for-next linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3 v5.14-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Muchun-Song/Use-obj_cgroup-APIs-to-charge-the-LRU-pages/20210814-132844
base:    4b358aabb93a2c654cd1dcab1a25a589f6e2b153
:::::: branch date: 8 hours ago
:::::: commit date: 8 hours ago
config: h8300-randconfig-s031-20210814 (attached as .config)
compiler: h8300-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-348-gf0e6938b-dirty
        # https://github.com/0day-ci/linux/commit/85e68f3fee63641ee3b35fc190c70deedbf1b913
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Muchun-Song/Use-obj_cgroup-APIs-to-charge-the-LRU-pages/20210814-132844
        git checkout 85e68f3fee63641ee3b35fc190c70deedbf1b913
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=h8300 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   mm/memcontrol.c:4093:21: sparse: sparse: incompatible types in comparison expression (different address spaces):
   mm/memcontrol.c:4093:21: sparse:    struct mem_cgroup_threshold_ary [noderef] __rcu *
   mm/memcontrol.c:4093:21: sparse:    struct mem_cgroup_threshold_ary *
   mm/memcontrol.c:4095:21: sparse: sparse: incompatible types in comparison expression (different address spaces):
   mm/memcontrol.c:4095:21: sparse:    struct mem_cgroup_threshold_ary [noderef] __rcu *
   mm/memcontrol.c:4095:21: sparse:    struct mem_cgroup_threshold_ary *
   mm/memcontrol.c:4251:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   mm/memcontrol.c:4251:9: sparse:    struct mem_cgroup_threshold_ary [noderef] __rcu *
   mm/memcontrol.c:4251:9: sparse:    struct mem_cgroup_threshold_ary *
   mm/memcontrol.c:4345:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   mm/memcontrol.c:4345:9: sparse:    struct mem_cgroup_threshold_ary [noderef] __rcu *
   mm/memcontrol.c:4345:9: sparse:    struct mem_cgroup_threshold_ary *
   mm/memcontrol.c: note: in included file (through include/linux/rculist.h, include/linux/pid.h, include/linux/sched.h, ...):
   include/linux/rcupdate.h:718:9: sparse: sparse: context imbalance in 'folio_lruvec_lock' - wrong count at exit
>> include/linux/rcupdate.h:718:9: sparse: sparse: context imbalance in 'folio_lruvec_lock_irq' - wrong count at exit
>> include/linux/rcupdate.h:718:9: sparse: sparse: context imbalance in 'folio_lruvec_lock_irqsave' - wrong count at exit
   mm/memcontrol.c:1977:6: sparse: sparse: context imbalance in 'folio_memcg_lock' - wrong count at exit
   mm/memcontrol.c:2031:17: sparse: sparse: context imbalance in '__folio_memcg_unlock' - unexpected unlock
   mm/memcontrol.c:5370:6: sparse: sparse: context imbalance in 'mem_cgroup_flush_stats' - wrong count at exit

vim +/folio_lruvec_lock_irq +718 include/linux/rcupdate.h

^1da177e4c3f41 Linus Torvalds   2005-04-16  691  
^1da177e4c3f41 Linus Torvalds   2005-04-16  692  /*
^1da177e4c3f41 Linus Torvalds   2005-04-16  693   * So where is rcu_write_lock()?  It does not exist, as there is no
^1da177e4c3f41 Linus Torvalds   2005-04-16  694   * way for writers to lock out RCU readers.  This is a feature, not
^1da177e4c3f41 Linus Torvalds   2005-04-16  695   * a bug -- this property is what provides RCU's performance benefits.
^1da177e4c3f41 Linus Torvalds   2005-04-16  696   * Of course, writers must coordinate with each other.  The normal
^1da177e4c3f41 Linus Torvalds   2005-04-16  697   * spinlock primitives work well for this, but any other technique may be
^1da177e4c3f41 Linus Torvalds   2005-04-16  698   * used as well.  RCU does not care how the writers keep out of each
^1da177e4c3f41 Linus Torvalds   2005-04-16  699   * others' way, as long as they do so.
^1da177e4c3f41 Linus Torvalds   2005-04-16  700   */
3d76c082907e8f Paul E. McKenney 2009-09-28  701  
3d76c082907e8f Paul E. McKenney 2009-09-28  702  /**
ca5ecddfa8fcbd Paul E. McKenney 2010-04-28  703   * rcu_read_unlock() - marks the end of an RCU read-side critical section.
3d76c082907e8f Paul E. McKenney 2009-09-28  704   *
0223846010750e Paul E. McKenney 2021-04-29  705   * In almost all situations, rcu_read_unlock() is immune from deadlock.
0223846010750e Paul E. McKenney 2021-04-29  706   * In recent kernels that have consolidated synchronize_sched() and
0223846010750e Paul E. McKenney 2021-04-29  707   * synchronize_rcu_bh() into synchronize_rcu(), this deadlock immunity
0223846010750e Paul E. McKenney 2021-04-29  708   * also extends to the scheduler's runqueue and priority-inheritance
0223846010750e Paul E. McKenney 2021-04-29  709   * spinlocks, courtesy of the quiescent-state deferral that is carried
0223846010750e Paul E. McKenney 2021-04-29  710   * out when rcu_read_unlock() is invoked with interrupts disabled.
f27bc4873fa8b7 Paul E. McKenney 2014-05-04  711   *
3d76c082907e8f Paul E. McKenney 2009-09-28  712   * See rcu_read_lock() for more information.
3d76c082907e8f Paul E. McKenney 2009-09-28  713   */
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  714  static inline void rcu_read_unlock(void)
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  715  {
f78f5b90c4ffa5 Paul E. McKenney 2015-06-18  716  	RCU_LOCKDEP_WARN(!rcu_is_watching(),
bde23c6892878e Heiko Carstens   2012-02-01  717  			 "rcu_read_unlock() used illegally while idle");
bc33f24bdca8b6 Paul E. McKenney 2009-08-22 @718  	__release(RCU);
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  719  	__rcu_read_unlock();
d24209bb689e2c Paul E. McKenney 2015-01-21  720  	rcu_lock_release(&rcu_lock_map); /* Keep acq info for rls diags. */
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  721  }
^1da177e4c3f41 Linus Torvalds   2005-04-16  722  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28881 bytes --]

Re: [PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5868 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210814052519.86679-4-songmuchun@bytedance.com>
References: <20210814052519.86679-4-songmuchun@bytedance.com>
TO: Muchun Song <songmuchun@bytedance.com>
TO: guro(a)fb.com
TO: hannes(a)cmpxchg.org
TO: mhocko(a)kernel.org
TO: akpm(a)linux-foundation.org
TO: shakeelb(a)google.com
TO: vdavydov.dev(a)gmail.com
CC: linux-kernel(a)vger.kernel.org
CC: linux-mm(a)kvack.org
CC: duanxiongchun(a)bytedance.com
CC: fam.zheng(a)bytedance.com

Hi Muchun,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20210813]
[cannot apply to hnaz-linux-mm/master cgroup/for-next linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3 v5.14-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Muchun-Song/Use-obj_cgroup-APIs-to-charge-the-LRU-pages/20210814-132844
base:    4b358aabb93a2c654cd1dcab1a25a589f6e2b153
:::::: branch date: 21 hours ago
:::::: commit date: 21 hours ago
config: arm64-randconfig-s031-20210815 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-348-gf0e6938b-dirty
        # https://github.com/0day-ci/linux/commit/85e68f3fee63641ee3b35fc190c70deedbf1b913
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Muchun-Song/Use-obj_cgroup-APIs-to-charge-the-LRU-pages/20210814-132844
        git checkout 85e68f3fee63641ee3b35fc190c70deedbf1b913
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   mm/compaction.c: note: in included file (through include/linux/rculist.h, include/linux/pid.h, include/linux/sched.h, ...):
>> include/linux/rcupdate.h:718:9: sparse: sparse: context imbalance in 'compact_folio_lruvec_lock_irqsave' - wrong count at exit
   mm/compaction.c:562:39: sparse: sparse: context imbalance in 'compact_unlock_should_abort' - unexpected unlock
   mm/compaction.c:682:39: sparse: sparse: context imbalance in 'isolate_freepages_block' - unexpected unlock
   mm/compaction.c:1168:46: sparse: sparse: context imbalance in 'isolate_migratepages_block' - unexpected unlock

vim +/compact_folio_lruvec_lock_irqsave +718 include/linux/rcupdate.h

^1da177e4c3f41 Linus Torvalds   2005-04-16  691  
^1da177e4c3f41 Linus Torvalds   2005-04-16  692  /*
^1da177e4c3f41 Linus Torvalds   2005-04-16  693   * So where is rcu_write_lock()?  It does not exist, as there is no
^1da177e4c3f41 Linus Torvalds   2005-04-16  694   * way for writers to lock out RCU readers.  This is a feature, not
^1da177e4c3f41 Linus Torvalds   2005-04-16  695   * a bug -- this property is what provides RCU's performance benefits.
^1da177e4c3f41 Linus Torvalds   2005-04-16  696   * Of course, writers must coordinate with each other.  The normal
^1da177e4c3f41 Linus Torvalds   2005-04-16  697   * spinlock primitives work well for this, but any other technique may be
^1da177e4c3f41 Linus Torvalds   2005-04-16  698   * used as well.  RCU does not care how the writers keep out of each
^1da177e4c3f41 Linus Torvalds   2005-04-16  699   * others' way, as long as they do so.
^1da177e4c3f41 Linus Torvalds   2005-04-16  700   */
3d76c082907e8f Paul E. McKenney 2009-09-28  701  
3d76c082907e8f Paul E. McKenney 2009-09-28  702  /**
ca5ecddfa8fcbd Paul E. McKenney 2010-04-28  703   * rcu_read_unlock() - marks the end of an RCU read-side critical section.
3d76c082907e8f Paul E. McKenney 2009-09-28  704   *
0223846010750e Paul E. McKenney 2021-04-29  705   * In almost all situations, rcu_read_unlock() is immune from deadlock.
0223846010750e Paul E. McKenney 2021-04-29  706   * In recent kernels that have consolidated synchronize_sched() and
0223846010750e Paul E. McKenney 2021-04-29  707   * synchronize_rcu_bh() into synchronize_rcu(), this deadlock immunity
0223846010750e Paul E. McKenney 2021-04-29  708   * also extends to the scheduler's runqueue and priority-inheritance
0223846010750e Paul E. McKenney 2021-04-29  709   * spinlocks, courtesy of the quiescent-state deferral that is carried
0223846010750e Paul E. McKenney 2021-04-29  710   * out when rcu_read_unlock() is invoked with interrupts disabled.
f27bc4873fa8b7 Paul E. McKenney 2014-05-04  711   *
3d76c082907e8f Paul E. McKenney 2009-09-28  712   * See rcu_read_lock() for more information.
3d76c082907e8f Paul E. McKenney 2009-09-28  713   */
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  714  static inline void rcu_read_unlock(void)
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  715  {
f78f5b90c4ffa5 Paul E. McKenney 2015-06-18  716  	RCU_LOCKDEP_WARN(!rcu_is_watching(),
bde23c6892878e Heiko Carstens   2012-02-01  717  			 "rcu_read_unlock() used illegally while idle");
bc33f24bdca8b6 Paul E. McKenney 2009-08-22 @718  	__release(RCU);
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  719  	__rcu_read_unlock();
d24209bb689e2c Paul E. McKenney 2015-01-21  720  	rcu_lock_release(&rcu_lock_map); /* Keep acq info for rls diags. */
bc33f24bdca8b6 Paul E. McKenney 2009-08-22  721  }
^1da177e4c3f41 Linus Torvalds   2005-04-16  722  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36797 bytes --]

Re: [PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[Roman Gushchin]

On Sat, Aug 14, 2021 at 01:25:10PM +0800, Muchun Song wrote:
> 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>
> Acked-by: Roman Gushchin <guro@fb.com>

Maybe it's mostly s/page/folio, but the patch looks quite differently
in comparison to the version I did ack. In general, please, drop acks
when there are significant changes between versions.

Thanks!

Re: [PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[Muchun Song]

On Wed, Aug 18, 2021 at 11:18 AM Roman Gushchin <guro@fb.com> wrote:
>
> On Sat, Aug 14, 2021 at 01:25:10PM +0800, Muchun Song wrote:
> > 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>
> > Acked-by: Roman Gushchin <guro@fb.com>
>
> Maybe it's mostly s/page/folio, but the patch looks quite differently
> in comparison to the version I did ack. In general, please, drop acks
> when there are significant changes between versions.

Got it. I'll drop all acks in the next version. Thanks.

>
> Thanks!

Re: [PATCH v1 03/12] mm: memcontrol: make lruvec lock safe when LRU pages are reparented

[Roman Gushchin]

On Wed, Aug 18, 2021 at 12:28:02PM +0800, Muchun Song wrote:
> On Wed, Aug 18, 2021 at 11:18 AM Roman Gushchin <guro@fb.com> wrote:
> >
> > On Sat, Aug 14, 2021 at 01:25:10PM +0800, Muchun Song wrote:
> > > 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>
> > > Acked-by: Roman Gushchin <guro@fb.com>
> >
> > Maybe it's mostly s/page/folio, but the patch looks quite differently
> > in comparison to the version I did ack. In general, please, drop acks
> > when there are significant changes between versions.
> 
> Got it. I'll drop all acks in the next version. Thanks.

Thank you!

The code look correct though. But the locking scheme becomes very complicated
(it was already complex). I wonder if there are better ideas. No ideas out
of my head, need to think more. If not, your approach looks ok.