All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/3] mm: disable LRU pagevec during the migration temporarily
@ 2021-03-19 17:51 Minchan Kim
  2021-03-19 17:51 ` [PATCH v4 2/3] mm: replace migrate_[prep|finish] with lru_cache_[disable|enable] Minchan Kim
  2021-03-19 17:51 ` [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration Minchan Kim
  0 siblings, 2 replies; 8+ messages in thread
From: Minchan Kim @ 2021-03-19 17:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, LKML, joaodias, surenb, cgoldswo, willy, mhocko, david,
	vbabka, linux-fsdevel, oliver.sang, Minchan Kim

LRU pagevec holds refcount of pages until the pagevec are drained.
It could prevent migration since the refcount of the page is greater
than the expection in migration logic. To mitigate the issue,
callers of migrate_pages drains LRU pagevec via migrate_prep or
lru_add_drain_all before migrate_pages call.

However, it's not enough because pages coming into pagevec after the
draining call still could stay at the pagevec so it could keep
preventing page migration. Since some callers of migrate_pages have
retrial logic with LRU draining, the page would migrate at next trail
but it is still fragile in that it doesn't close the fundamental race
between upcoming LRU pages into pagvec and migration so the migration
failure could cause contiguous memory allocation failure in the end.

To close the race, this patch disables lru caches(i.e, pagevec)
during ongoing migration until migrate is done.

Since it's really hard to reproduce, I measured how many times
migrate_pages retried with force mode(it is about a fallback to a
sync migration) with below debug code.

int migrate_pages(struct list_head *from, new_page_t get_new_page,
			..
			..

if (rc && reason == MR_CONTIG_RANGE && pass > 2) {
       printk(KERN_ERR, "pfn 0x%lx reason %d\n", page_to_pfn(page), rc);
       dump_page(page, "fail to migrate");
}

The test was repeating android apps launching with cma allocation
in background every five seconds. Total cma allocation count was
about 500 during the testing. With this patch, the dump_page count
was reduced from 400 to 30.

The new interface is also useful for memory hotplug which currently
drains lru pcp caches after each migration failure. This is rather
suboptimal as it has to disrupt others running during the operation.
With the new interface the operation happens only once. This is also in
line with pcp allocator cache which are disabled for the offlining as
well.

Reviewed-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/migrate.h |  2 ++
 include/linux/swap.h    | 14 +++++++++
 mm/memory_hotplug.c     |  3 +-
 mm/mempolicy.c          |  4 +++
 mm/migrate.c            | 11 ++++---
 mm/page_alloc.c         |  2 ++
 mm/swap.c               | 64 +++++++++++++++++++++++++++++++++++------
 7 files changed, 86 insertions(+), 14 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 3a389633b68f..9e4a2dc8622c 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -46,6 +46,7 @@ extern int isolate_movable_page(struct page *page, isolate_mode_t mode);
 extern void putback_movable_page(struct page *page);
 
 extern void migrate_prep(void);
+extern void migrate_finish(void);
 extern void migrate_prep_local(void);
 extern void migrate_page_states(struct page *newpage, struct page *page);
 extern void migrate_page_copy(struct page *newpage, struct page *page);
@@ -67,6 +68,7 @@ static inline int isolate_movable_page(struct page *page, isolate_mode_t mode)
 	{ return -EBUSY; }
 
 static inline int migrate_prep(void) { return -ENOSYS; }
+static inline int migrate_finish(void) { return -ENOSYS; }
 static inline int migrate_prep_local(void) { return -ENOSYS; }
 
 static inline void migrate_page_states(struct page *newpage, struct page *page)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 32f665b1ee85..85cf022072a0 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -339,6 +339,20 @@ extern void lru_note_cost(struct lruvec *lruvec, bool file,
 extern void lru_note_cost_page(struct page *);
 extern void lru_cache_add(struct page *);
 extern void mark_page_accessed(struct page *);
+
+extern atomic_t lru_disable_count;
+
+static inline bool lru_cache_disabled(void)
+{
+	return atomic_read(&lru_disable_count);
+}
+
+static inline void lru_cache_enable(void)
+{
+	atomic_dec(&lru_disable_count);
+}
+
+extern void lru_cache_disable(void);
 extern void lru_add_drain(void);
 extern void lru_add_drain_cpu(int cpu);
 extern void lru_add_drain_cpu_zone(struct zone *zone);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 5ba51a8bdaeb..959f659ef085 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1611,6 +1611,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 	 * in a way that pages from isolated pageblock are left on pcplists.
 	 */
 	zone_pcp_disable(zone);
+	lru_cache_disable();
 
 	/* set above range as isolated */
 	ret = start_isolate_page_range(start_pfn, end_pfn,
@@ -1642,7 +1643,6 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 			}
 
 			cond_resched();
-			lru_add_drain_all();
 
 			ret = scan_movable_pages(pfn, end_pfn, &pfn);
 			if (!ret) {
@@ -1687,6 +1687,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 	zone->nr_isolate_pageblock -= nr_pages / pageblock_nr_pages;
 	spin_unlock_irqrestore(&zone->lock, flags);
 
+	lru_cache_enable();
 	zone_pcp_enable(zone);
 
 	/* removal success */
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index ab51132547b8..495b43a4b0f8 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1208,6 +1208,8 @@ int do_migrate_pages(struct mm_struct *mm, const nodemask_t *from,
 			break;
 	}
 	mmap_read_unlock(mm);
+
+	migrate_finish();
 	if (err < 0)
 		return err;
 	return busy;
@@ -1371,6 +1373,8 @@ static long do_mbind(unsigned long start, unsigned long len,
 	mmap_write_unlock(mm);
 mpol_out:
 	mpol_put(new);
+	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))
+		migrate_finish();
 	return err;
 }
 
diff --git a/mm/migrate.c b/mm/migrate.c
index 62b81d5257aa..4d6c306d41c6 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -66,11 +66,13 @@ void migrate_prep(void)
 {
 	/*
 	 * Clear the LRU lists so pages can be isolated.
-	 * Note that pages may be moved off the LRU after we have
-	 * drained them. Those pages will fail to migrate like other
-	 * pages that may be busy.
 	 */
-	lru_add_drain_all();
+	lru_cache_disable();
+}
+
+void migrate_finish(void)
+{
+	lru_cache_enable();
 }
 
 /* Do the necessary work of migrate_prep but not if it involves other CPUs */
@@ -1838,6 +1840,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
 	if (err >= 0)
 		err = err1;
 out:
+	migrate_finish();
 	return err;
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2e8348936df8..af5d4eeb2999 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8495,6 +8495,8 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 		ret = migrate_pages(&cc->migratepages, alloc_migration_target,
 				NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE);
 	}
+
+	migrate_finish();
 	if (ret < 0) {
 		putback_movable_pages(&cc->migratepages);
 		return ret;
diff --git a/mm/swap.c b/mm/swap.c
index 31b844d4ed94..c94f55e7b649 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -235,6 +235,18 @@ static void pagevec_move_tail_fn(struct page *page, struct lruvec *lruvec)
 	}
 }
 
+/* return true if pagevec needs to drain */
+static bool pagevec_add_and_need_flush(struct pagevec *pvec, struct page *page)
+{
+	bool ret = false;
+
+	if (!pagevec_add(pvec, page) || PageCompound(page) ||
+			lru_cache_disabled())
+		ret = true;
+
+	return ret;
+}
+
 /*
  * Writeback is about to end against a page which has been marked for immediate
  * reclaim.  If it still appears to be reclaimable, move it to the tail of the
@@ -252,7 +264,7 @@ void rotate_reclaimable_page(struct page *page)
 		get_page(page);
 		local_lock_irqsave(&lru_rotate.lock, flags);
 		pvec = this_cpu_ptr(&lru_rotate.pvec);
-		if (!pagevec_add(pvec, page) || PageCompound(page))
+		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, pagevec_move_tail_fn);
 		local_unlock_irqrestore(&lru_rotate.lock, flags);
 	}
@@ -343,7 +355,7 @@ static void activate_page(struct page *page)
 		local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.activate_page);
 		get_page(page);
-		if (!pagevec_add(pvec, page) || PageCompound(page))
+		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, __activate_page);
 		local_unlock(&lru_pvecs.lock);
 	}
@@ -458,7 +470,7 @@ void lru_cache_add(struct page *page)
 	get_page(page);
 	local_lock(&lru_pvecs.lock);
 	pvec = this_cpu_ptr(&lru_pvecs.lru_add);
-	if (!pagevec_add(pvec, page) || PageCompound(page))
+	if (pagevec_add_and_need_flush(pvec, page))
 		__pagevec_lru_add(pvec);
 	local_unlock(&lru_pvecs.lock);
 }
@@ -654,7 +666,7 @@ void deactivate_file_page(struct page *page)
 		local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.lru_deactivate_file);
 
-		if (!pagevec_add(pvec, page) || PageCompound(page))
+		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, lru_deactivate_file_fn);
 		local_unlock(&lru_pvecs.lock);
 	}
@@ -676,7 +688,7 @@ void deactivate_page(struct page *page)
 		local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.lru_deactivate);
 		get_page(page);
-		if (!pagevec_add(pvec, page) || PageCompound(page))
+		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, lru_deactivate_fn);
 		local_unlock(&lru_pvecs.lock);
 	}
@@ -698,7 +710,7 @@ void mark_page_lazyfree(struct page *page)
 		local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.lru_lazyfree);
 		get_page(page);
-		if (!pagevec_add(pvec, page) || PageCompound(page))
+		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, lru_lazyfree_fn);
 		local_unlock(&lru_pvecs.lock);
 	}
@@ -735,7 +747,7 @@ static void lru_add_drain_per_cpu(struct work_struct *dummy)
  * Calling this function with cpu hotplug locks held can actually lead
  * to obscure indirect dependencies via WQ context.
  */
-void lru_add_drain_all(void)
+inline void __lru_add_drain_all(bool force_all_cpus)
 {
 	/*
 	 * lru_drain_gen - Global pages generation number
@@ -780,7 +792,7 @@ void lru_add_drain_all(void)
 	 * (C) Exit the draining operation if a newer generation, from another
 	 * lru_add_drain_all(), was already scheduled for draining. Check (A).
 	 */
-	if (unlikely(this_gen != lru_drain_gen))
+	if (unlikely(this_gen != lru_drain_gen && !force_all_cpus))
 		goto done;
 
 	/*
@@ -810,7 +822,8 @@ void lru_add_drain_all(void)
 	for_each_online_cpu(cpu) {
 		struct work_struct *work = &per_cpu(lru_add_drain_work, cpu);
 
-		if (pagevec_count(&per_cpu(lru_pvecs.lru_add, cpu)) ||
+		if (force_all_cpus ||
+		    pagevec_count(&per_cpu(lru_pvecs.lru_add, cpu)) ||
 		    data_race(pagevec_count(&per_cpu(lru_rotate.pvec, cpu))) ||
 		    pagevec_count(&per_cpu(lru_pvecs.lru_deactivate_file, cpu)) ||
 		    pagevec_count(&per_cpu(lru_pvecs.lru_deactivate, cpu)) ||
@@ -828,6 +841,11 @@ void lru_add_drain_all(void)
 done:
 	mutex_unlock(&lock);
 }
+
+void lru_add_drain_all(void)
+{
+	__lru_add_drain_all(false);
+}
 #else
 void lru_add_drain_all(void)
 {
@@ -835,6 +853,34 @@ void lru_add_drain_all(void)
 }
 #endif /* CONFIG_SMP */
 
+atomic_t lru_disable_count = ATOMIC_INIT(0);
+
+/*
+ * lru_cache_disable() needs to be called before we start compiling
+ * a list of pages to be migrated using isolate_lru_page().
+ * It drains pages on LRU cache and then disable on all cpus until
+ * lru_cache_enable is called.
+ *
+ * Must be paired with a call to lru_cache_enable().
+ */
+void lru_cache_disable(void)
+{
+	atomic_inc(&lru_disable_count);
+#ifdef CONFIG_SMP
+	/*
+	 * lru_add_drain_all in the force mode will schedule draining on
+	 * all online CPUs so any calls of lru_cache_disabled wrapped by
+	 * local_lock or preemption disabled would be ordered by that.
+	 * The atomic operation doesn't need to have stronger ordering
+	 * requirements because that is enforeced by the scheduling
+	 * guarantees.
+	 */
+	__lru_add_drain_all(true);
+#else
+	lru_add_drain();
+#endif
+}
+
 /**
  * release_pages - batched put_page()
  * @pages: array of pages to release
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] mm: replace migrate_[prep|finish] with lru_cache_[disable|enable]
  2021-03-19 17:51 [PATCH v4 1/3] mm: disable LRU pagevec during the migration temporarily Minchan Kim
@ 2021-03-19 17:51 ` Minchan Kim
  2021-03-19 17:51 ` [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration Minchan Kim
  1 sibling, 0 replies; 8+ messages in thread
From: Minchan Kim @ 2021-03-19 17:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, LKML, joaodias, surenb, cgoldswo, willy, mhocko, david,
	vbabka, linux-fsdevel, oliver.sang, Minchan Kim

Currently, migrate_[prep|finish] is merely a wrapper of
lru_cache_[disable|enable]. There is not much to gain from
having additional abstraction.

Use lru_cache_[disable|enable] instead of migrate_[prep|finish],
which would be more descriptive.

note: migrate_prep_local in compaction.c changed into lru_add_drain
to avoid CPU schedule cost with involving many other CPUs to keep
keep old behavior.

Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/migrate.h |  7 -------
 mm/compaction.c         |  3 ++-
 mm/mempolicy.c          |  8 ++++----
 mm/migrate.c            | 28 ++--------------------------
 mm/page_alloc.c         |  4 ++--
 5 files changed, 10 insertions(+), 40 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 9e4a2dc8622c..6155d97ec76c 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -45,9 +45,6 @@ extern struct page *alloc_migration_target(struct page *page, unsigned long priv
 extern int isolate_movable_page(struct page *page, isolate_mode_t mode);
 extern void putback_movable_page(struct page *page);
 
-extern void migrate_prep(void);
-extern void migrate_finish(void);
-extern void migrate_prep_local(void);
 extern void migrate_page_states(struct page *newpage, struct page *page);
 extern void migrate_page_copy(struct page *newpage, struct page *page);
 extern int migrate_huge_page_move_mapping(struct address_space *mapping,
@@ -67,10 +64,6 @@ static inline struct page *alloc_migration_target(struct page *page,
 static inline int isolate_movable_page(struct page *page, isolate_mode_t mode)
 	{ return -EBUSY; }
 
-static inline int migrate_prep(void) { return -ENOSYS; }
-static inline int migrate_finish(void) { return -ENOSYS; }
-static inline int migrate_prep_local(void) { return -ENOSYS; }
-
 static inline void migrate_page_states(struct page *newpage, struct page *page)
 {
 }
diff --git a/mm/compaction.c b/mm/compaction.c
index e04f4476e68e..3be017ececc0 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2319,7 +2319,8 @@ compact_zone(struct compact_control *cc, struct capture_control *capc)
 	trace_mm_compaction_begin(start_pfn, cc->migrate_pfn,
 				cc->free_pfn, end_pfn, sync);
 
-	migrate_prep_local();
+	/* lru_add_drain_all could be expensive with involving other CPUs */
+	lru_add_drain();
 
 	while ((ret = compact_finished(cc)) == COMPACT_CONTINUE) {
 		int err;
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 495b43a4b0f8..6daf9cc4c843 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1124,7 +1124,7 @@ int do_migrate_pages(struct mm_struct *mm, const nodemask_t *from,
 	int err = 0;
 	nodemask_t tmp;
 
-	migrate_prep();
+	lru_cache_disable();
 
 	mmap_read_lock(mm);
 
@@ -1209,7 +1209,7 @@ int do_migrate_pages(struct mm_struct *mm, const nodemask_t *from,
 	}
 	mmap_read_unlock(mm);
 
-	migrate_finish();
+	lru_cache_enable();
 	if (err < 0)
 		return err;
 	return busy;
@@ -1325,7 +1325,7 @@ static long do_mbind(unsigned long start, unsigned long len,
 
 	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
 
-		migrate_prep();
+		lru_cache_disable();
 	}
 	{
 		NODEMASK_SCRATCH(scratch);
@@ -1374,7 +1374,7 @@ static long do_mbind(unsigned long start, unsigned long len,
 mpol_out:
 	mpol_put(new);
 	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))
-		migrate_finish();
+		lru_cache_enable();
 	return err;
 }
 
diff --git a/mm/migrate.c b/mm/migrate.c
index 4d6c306d41c6..acc9913e4303 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -57,30 +57,6 @@
 
 #include "internal.h"
 
-/*
- * migrate_prep() needs to be called before we start compiling a list of pages
- * to be migrated using isolate_lru_page(). If scheduling work on other CPUs is
- * undesirable, use migrate_prep_local()
- */
-void migrate_prep(void)
-{
-	/*
-	 * Clear the LRU lists so pages can be isolated.
-	 */
-	lru_cache_disable();
-}
-
-void migrate_finish(void)
-{
-	lru_cache_enable();
-}
-
-/* Do the necessary work of migrate_prep but not if it involves other CPUs */
-void migrate_prep_local(void)
-{
-	lru_add_drain();
-}
-
 int isolate_movable_page(struct page *page, isolate_mode_t mode)
 {
 	struct address_space *mapping;
@@ -1771,7 +1747,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
 	int start, i;
 	int err = 0, err1;
 
-	migrate_prep();
+	lru_cache_disable();
 
 	for (i = start = 0; i < nr_pages; i++) {
 		const void __user *p;
@@ -1840,7 +1816,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
 	if (err >= 0)
 		err = err1;
 out:
-	migrate_finish();
+	lru_cache_enable();
 	return err;
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index af5d4eeb2999..bf1606c7965a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8467,7 +8467,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 		.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
 	};
 
-	migrate_prep();
+	lru_cache_disable();
 
 	while (pfn < end || !list_empty(&cc->migratepages)) {
 		if (fatal_signal_pending(current)) {
@@ -8496,7 +8496,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 				NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE);
 	}
 
-	migrate_finish();
+	lru_cache_enable();
 	if (ret < 0) {
 		putback_movable_pages(&cc->migratepages);
 		return ret;
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration
  2021-03-19 17:51 [PATCH v4 1/3] mm: disable LRU pagevec during the migration temporarily Minchan Kim
  2021-03-19 17:51 ` [PATCH v4 2/3] mm: replace migrate_[prep|finish] with lru_cache_[disable|enable] Minchan Kim
@ 2021-03-19 17:51 ` Minchan Kim
  2021-03-20 16:32   ` Andrew Morton
  1 sibling, 1 reply; 8+ messages in thread
From: Minchan Kim @ 2021-03-19 17:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, LKML, joaodias, surenb, cgoldswo, willy, mhocko, david,
	vbabka, linux-fsdevel, oliver.sang, Minchan Kim

Pages containing buffer_heads that are in one of the per-CPU
buffer_head LRU caches will be pinned and thus cannot be migrated.
This can prevent CMA allocations from succeeding, which are often used
on platforms with co-processors (such as a DSP) that can only use
physically contiguous memory. It can also prevent memory
hot-unplugging from succeeding, which involves migrating at least
MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
GiB based on the architecture in use.

Correspondingly, invalidate the BH LRU caches before a migration
starts and stop any buffer_head from being cached in the LRU caches,
until migration has finished.

Tested-by: Oliver Sang <oliver.sang@intel.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 fs/buffer.c                 | 36 ++++++++++++++++++++++++++++++------
 include/linux/buffer_head.h |  4 ++++
 mm/swap.c                   |  5 ++++-
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 0cb7ffd4977c..e9872d0dcbf1 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1264,6 +1264,15 @@ static void bh_lru_install(struct buffer_head *bh)
 	int i;
 
 	check_irqs_on();
+	/*
+	 * the refcount of buffer_head in bh_lru prevents dropping the
+	 * attached page(i.e., try_to_free_buffers) so it could cause
+	 * failing page migration.
+	 * Skip putting upcoming bh into bh_lru until migration is done.
+	 */
+	if (lru_cache_disabled())
+		return;
+
 	bh_lru_lock();
 
 	b = this_cpu_ptr(&bh_lrus);
@@ -1404,6 +1413,15 @@ __bread_gfp(struct block_device *bdev, sector_t block,
 }
 EXPORT_SYMBOL(__bread_gfp);
 
+static void __invalidate_bh_lrus(struct bh_lru *b)
+{
+	int i;
+
+	for (i = 0; i < BH_LRU_SIZE; i++) {
+		brelse(b->bhs[i]);
+		b->bhs[i] = NULL;
+	}
+}
 /*
  * invalidate_bh_lrus() is called rarely - but not only at unmount.
  * This doesn't race because it runs in each cpu either in irq
@@ -1412,16 +1430,12 @@ EXPORT_SYMBOL(__bread_gfp);
 static void invalidate_bh_lru(void *arg)
 {
 	struct bh_lru *b = &get_cpu_var(bh_lrus);
-	int i;
 
-	for (i = 0; i < BH_LRU_SIZE; i++) {
-		brelse(b->bhs[i]);
-		b->bhs[i] = NULL;
-	}
+	__invalidate_bh_lrus(b);
 	put_cpu_var(bh_lrus);
 }
 
-static bool has_bh_in_lru(int cpu, void *dummy)
+bool has_bh_in_lru(int cpu, void *dummy)
 {
 	struct bh_lru *b = per_cpu_ptr(&bh_lrus, cpu);
 	int i;
@@ -1440,6 +1454,16 @@ void invalidate_bh_lrus(void)
 }
 EXPORT_SYMBOL_GPL(invalidate_bh_lrus);
 
+void invalidate_bh_lrus_cpu(int cpu)
+{
+	struct bh_lru *b;
+
+	bh_lru_lock();
+	b = per_cpu_ptr(&bh_lrus, cpu);
+	__invalidate_bh_lrus(b);
+	bh_lru_unlock();
+}
+
 void set_bh_page(struct buffer_head *bh,
 		struct page *page, unsigned long offset)
 {
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 6b47f94378c5..e7e99da31349 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -194,6 +194,8 @@ void __breadahead_gfp(struct block_device *, sector_t block, unsigned int size,
 struct buffer_head *__bread_gfp(struct block_device *,
 				sector_t block, unsigned size, gfp_t gfp);
 void invalidate_bh_lrus(void);
+void invalidate_bh_lrus_cpu(int cpu);
+bool has_bh_in_lru(int cpu, void *dummy);
 struct buffer_head *alloc_buffer_head(gfp_t gfp_flags);
 void free_buffer_head(struct buffer_head * bh);
 void unlock_buffer(struct buffer_head *bh);
@@ -406,6 +408,8 @@ static inline int inode_has_buffers(struct inode *inode) { return 0; }
 static inline void invalidate_inode_buffers(struct inode *inode) {}
 static inline int remove_inode_buffers(struct inode *inode) { return 1; }
 static inline int sync_mapping_buffers(struct address_space *mapping) { return 0; }
+static inline void invalidate_bh_lrus_cpu(int cpu) {}
+static inline bool has_bh_in_lru(int cpu, void *dummy) { return 0; }
 #define buffer_heads_over_limit 0
 
 #endif /* CONFIG_BLOCK */
diff --git a/mm/swap.c b/mm/swap.c
index c94f55e7b649..a75a8265302b 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -36,6 +36,7 @@
 #include <linux/hugetlb.h>
 #include <linux/page_idle.h>
 #include <linux/local_lock.h>
+#include <linux/buffer_head.h>
 
 #include "internal.h"
 
@@ -641,6 +642,7 @@ void lru_add_drain_cpu(int cpu)
 		pagevec_lru_move_fn(pvec, lru_lazyfree_fn);
 
 	activate_page_drain(cpu);
+	invalidate_bh_lrus_cpu(cpu);
 }
 
 /**
@@ -828,7 +830,8 @@ inline void __lru_add_drain_all(bool force_all_cpus)
 		    pagevec_count(&per_cpu(lru_pvecs.lru_deactivate_file, cpu)) ||
 		    pagevec_count(&per_cpu(lru_pvecs.lru_deactivate, cpu)) ||
 		    pagevec_count(&per_cpu(lru_pvecs.lru_lazyfree, cpu)) ||
-		    need_activate_page_drain(cpu)) {
+		    need_activate_page_drain(cpu) ||
+		    has_bh_in_lru(cpu, NULL)) {
 			INIT_WORK(work, lru_add_drain_per_cpu);
 			queue_work_on(cpu, mm_percpu_wq, work);
 			__cpumask_set_cpu(cpu, &has_work);
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration
  2021-03-19 17:51 ` [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration Minchan Kim
@ 2021-03-20 16:32   ` Andrew Morton
  2021-03-20 17:20     ` Minchan Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2021-03-20 16:32 UTC (permalink / raw)
  To: Minchan Kim
  Cc: linux-mm, LKML, joaodias, surenb, cgoldswo, willy, mhocko, david,
	vbabka, linux-fsdevel, oliver.sang

On Fri, 19 Mar 2021 10:51:27 -0700 Minchan Kim <minchan@kernel.org> wrote:

> Pages containing buffer_heads that are in one of the per-CPU
> buffer_head LRU caches will be pinned and thus cannot be migrated.
> This can prevent CMA allocations from succeeding, which are often used
> on platforms with co-processors (such as a DSP) that can only use
> physically contiguous memory. It can also prevent memory
> hot-unplugging from succeeding, which involves migrating at least
> MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
> GiB based on the architecture in use.
> 
> Correspondingly, invalidate the BH LRU caches before a migration
> starts and stop any buffer_head from being cached in the LRU caches,
> until migration has finished.
> 
> Tested-by: Oliver Sang <oliver.sang@intel.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

The signoff chain ordering might mean that Chris was the primary author, but
there is no From:him.  Please clarify?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration
  2021-03-20 16:32   ` Andrew Morton
@ 2021-03-20 17:20     ` Minchan Kim
  2021-03-20 19:54       ` Matthew Wilcox
  0 siblings, 1 reply; 8+ messages in thread
From: Minchan Kim @ 2021-03-20 17:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, LKML, joaodias, surenb, cgoldswo, willy, mhocko, david,
	vbabka, linux-fsdevel, oliver.sang

On Sat, Mar 20, 2021 at 09:32:49AM -0700, Andrew Morton wrote:
> On Fri, 19 Mar 2021 10:51:27 -0700 Minchan Kim <minchan@kernel.org> wrote:
> 
> > Pages containing buffer_heads that are in one of the per-CPU
> > buffer_head LRU caches will be pinned and thus cannot be migrated.
> > This can prevent CMA allocations from succeeding, which are often used
> > on platforms with co-processors (such as a DSP) that can only use
> > physically contiguous memory. It can also prevent memory
> > hot-unplugging from succeeding, which involves migrating at least
> > MIN_MEMORY_BLOCK_SIZE bytes of memory, which ranges from 8 MiB to 1
> > GiB based on the architecture in use.
> > 
> > Correspondingly, invalidate the BH LRU caches before a migration
> > starts and stop any buffer_head from being cached in the LRU caches,
> > until migration has finished.
> > 
> > Tested-by: Oliver Sang <oliver.sang@intel.com>
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> 
> The signoff chain ordering might mean that Chris was the primary author, but
> there is no From:him.  Please clarify?

He tried first version but was diffrent implementation since I
changed a lot. That's why I added his SoB even though current
implementaion is much different. So, maybe I am primary author?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration
  2021-03-20 17:20     ` Minchan Kim
@ 2021-03-20 19:54       ` Matthew Wilcox
  2021-03-20 22:51         ` Chris Goldsworthy
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Wilcox @ 2021-03-20 19:54 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm, LKML, joaodias, surenb, cgoldswo,
	mhocko, david, vbabka, linux-fsdevel, oliver.sang

On Sat, Mar 20, 2021 at 10:20:09AM -0700, Minchan Kim wrote:
> > > Tested-by: Oliver Sang <oliver.sang@intel.com>
> > > Reported-by: kernel test robot <oliver.sang@intel.com>
> > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
> > > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > 
> > The signoff chain ordering might mean that Chris was the primary author, but
> > there is no From:him.  Please clarify?
> 
> He tried first version but was diffrent implementation since I
> changed a lot. That's why I added his SoB even though current
> implementaion is much different. So, maybe I am primary author?

Maybe Chris is Reported-by: ?  And don't forget Laura Abbott as original
author of the patch Chris submitted.  I think she should be Reported-by:
as well, since there is no code from either of them in this version of
the patch.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration
  2021-03-20 19:54       ` Matthew Wilcox
@ 2021-03-20 22:51         ` Chris Goldsworthy
  2021-03-22 16:46           ` Minchan Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Goldsworthy @ 2021-03-20 22:51 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Minchan Kim, Andrew Morton, linux-mm, LKML, joaodias, surenb,
	mhocko, david, vbabka, linux-fsdevel, oliver.sang

On 2021-03-20 12:54, Matthew Wilcox wrote:
> On Sat, Mar 20, 2021 at 10:20:09AM -0700, Minchan Kim wrote:
>> > > Tested-by: Oliver Sang <oliver.sang@intel.com>
>> > > Reported-by: kernel test robot <oliver.sang@intel.com>
>> > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
>> > > Signed-off-by: Minchan Kim <minchan@kernel.org>
>> >
>> > The signoff chain ordering might mean that Chris was the primary author, but
>> > there is no From:him.  Please clarify?
>> 
>> He tried first version but was diffrent implementation since I
>> changed a lot. That's why I added his SoB even though current
>> implementaion is much different. So, maybe I am primary author?

Hey Minchan, let's have you as the primary author.

> Maybe Chris is Reported-by: ?  And don't forget Laura Abbott as 
> original
> author of the patch Chris submitted.  I think she should be 
> Reported-by:
> as well, since there is no code from either of them in this version of
> the patch.

Yes, let's have a Reported-by: from Laura. We can change my 
Signed-off-by to Reported-by: as well.

-- 
The Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration
  2021-03-20 22:51         ` Chris Goldsworthy
@ 2021-03-22 16:46           ` Minchan Kim
  0 siblings, 0 replies; 8+ messages in thread
From: Minchan Kim @ 2021-03-22 16:46 UTC (permalink / raw)
  To: Chris Goldsworthy
  Cc: Matthew Wilcox, Andrew Morton, linux-mm, LKML, joaodias, surenb,
	mhocko, david, vbabka, linux-fsdevel, oliver.sang

On Sat, Mar 20, 2021 at 03:51:40PM -0700, Chris Goldsworthy wrote:
> On 2021-03-20 12:54, Matthew Wilcox wrote:
> > On Sat, Mar 20, 2021 at 10:20:09AM -0700, Minchan Kim wrote:
> > > > > Tested-by: Oliver Sang <oliver.sang@intel.com>
> > > > > Reported-by: kernel test robot <oliver.sang@intel.com>
> > > > > Signed-off-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
> > > > > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > > >
> > > > The signoff chain ordering might mean that Chris was the primary author, but
> > > > there is no From:him.  Please clarify?
> > > 
> > > He tried first version but was diffrent implementation since I
> > > changed a lot. That's why I added his SoB even though current
> > > implementaion is much different. So, maybe I am primary author?
> 
> Hey Minchan, let's have you as the primary author.
> 
> > Maybe Chris is Reported-by: ?  And don't forget Laura Abbott as original
> > author of the patch Chris submitted.  I think she should be Reported-by:
> > as well, since there is no code from either of them in this version of
> > the patch.
> 
> Yes, let's have a Reported-by: from Laura. We can change my Signed-off-by to
> Reported-by: as well.

Thanks Matthew and Chris.

I just wanted to give more credit(something like Co-authored-by if we have
such kinds of tag) to Chris since I felt he had more than. :)

Okay, let's have something like this.

From: Minchan Kim <minchan@kernel.org>

..

Tested-by: Oliver Sang <oliver.sang@intel.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Reported-by: Laura Abbott <lauraa@codeaurora.org>
Reported-by: Chris Goldsworthy <cgoldswo@codeaurora.org>
Signed-off-by: Minchan Kim <minchan@kernel.org>

Andrew, if you want me resend the patch, let me know.

Thank you.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-22 16:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 17:51 [PATCH v4 1/3] mm: disable LRU pagevec during the migration temporarily Minchan Kim
2021-03-19 17:51 ` [PATCH v4 2/3] mm: replace migrate_[prep|finish] with lru_cache_[disable|enable] Minchan Kim
2021-03-19 17:51 ` [PATCH v4 3/3] mm: fs: Invalidate BH LRU during page migration Minchan Kim
2021-03-20 16:32   ` Andrew Morton
2021-03-20 17:20     ` Minchan Kim
2021-03-20 19:54       ` Matthew Wilcox
2021-03-20 22:51         ` Chris Goldsworthy
2021-03-22 16:46           ` Minchan Kim

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.