linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio
@ 2024-01-29  7:09 Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio() Kefeng Wang
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

The folio migration is widely used in kernel, memory compaction, memory
hotplug, soft offline page, numa balance, memory demote/promotion, etc,
but once access a poisoned source folio when migrating, the kerenl will
panic.

There is a mechanism in the kernel to recover from uncorrectable memory
errors, ARCH_HAS_COPY_MC(Machine Check Safe Memory Copy), which is already
used in NVDIMM or core-mm paths(eg, CoW, khugepaged, coredump, ksm copy),
see copy_mc_to_{user,kernel}, copy_mc_{user_}highpage callers.

This series of patches provide the recovery mechanism from folio copy for
the widely used folio migration. Please note, because folio migration is
no guarantee of success, so we could chose to make folio migration tolerant
of memory failures, adding folio_mc_copy() which is a #MC versions of
folio_copy(), once accessing a poisoned source folio, we could return error
and make the folio migration fail, and this could avoid the similar panic
shown below.

  CPU: 1 PID: 88343 Comm: test_softofflin Kdump: loaded Not tainted 6.6.0
  pc : copy_page+0x10/0xc0
  lr : copy_highpage+0x38/0x50
  ...
  Call trace:
   copy_page+0x10/0xc0
   folio_copy+0x78/0x90
   migrate_folio_extra+0x54/0xa0
   move_to_new_folio+0xd8/0x1f0
   migrate_folio_move+0xb8/0x300
   migrate_pages_batch+0x528/0x788
   migrate_pages_sync+0x8c/0x258
   migrate_pages+0x440/0x528
   soft_offline_in_use_page+0x2ec/0x3c0
   soft_offline_page+0x238/0x310
   soft_offline_page_store+0x6c/0xc0
   dev_attr_store+0x20/0x40
   sysfs_kf_write+0x4c/0x68
   kernfs_fop_write_iter+0x130/0x1c8
   new_sync_write+0xa4/0x138
   vfs_write+0x238/0x2d8
   ksys_write+0x74/0x110

Kefeng Wang (9):
  mm: migrate: simplify __buffer_migrate_folio()
  mm: migrate_device: use more folio in __migrate_device_pages()
  mm: migrate: remove migrate_folio_extra()
  mm: remove MIGRATE_SYNC_NO_COPY mode
  mm: add folio_mc_copy()
  mm: migrate: support poisoned folio copy recover from migrate folio
  fs: hugetlbfs: support poison recover from hugetlbfs_migrate_folio()
  mm: migrate: remove folio_migrate_copy()
  fs: aio: add explicit check for large folio in aio_migrate_folio()

 fs/aio.c                     | 15 +++------
 fs/hugetlbfs/inode.c         | 13 +++++---
 include/linux/migrate.h      |  4 +--
 include/linux/migrate_mode.h |  5 ---
 include/linux/mm.h           |  1 +
 mm/balloon_compaction.c      |  8 -----
 mm/migrate.c                 | 64 +++++++++++-------------------------
 mm/migrate_device.c          | 26 +++++++--------
 mm/util.c                    | 20 +++++++++++
 mm/zsmalloc.c                |  8 -----
 10 files changed, 69 insertions(+), 95 deletions(-)

-- 
2.27.0



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

* [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-02-01 19:12   ` Matthew Wilcox
  2024-01-29  7:09 ` [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages() Kefeng Wang
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

Use filemap_migrate_folio() helper to simplify __buffer_migrate_folio().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/migrate.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index cc9f2bcd73b4..cdae25b7105f 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -774,24 +774,16 @@ static int __buffer_migrate_folio(struct address_space *mapping,
 		}
 	}
 
-	rc = folio_migrate_mapping(mapping, dst, src, 0);
+	rc = filemap_migrate_folio(mapping, dst, src, mode);
 	if (rc != MIGRATEPAGE_SUCCESS)
 		goto unlock_buffers;
 
-	folio_attach_private(dst, folio_detach_private(src));
-
 	bh = head;
 	do {
 		folio_set_bh(bh, dst, bh_offset(bh));
 		bh = bh->b_this_page;
 	} while (bh != head);
 
-	if (mode != MIGRATE_SYNC_NO_COPY)
-		folio_migrate_copy(dst, src);
-	else
-		folio_migrate_flags(dst, src);
-
-	rc = MIGRATEPAGE_SUCCESS;
 unlock_buffers:
 	if (check_refs)
 		spin_unlock(&mapping->i_private_lock);
-- 
2.27.0



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

* [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio() Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-02-01 19:27   ` Matthew Wilcox
  2024-01-29  7:09 ` [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra() Kefeng Wang
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

Use newfolio/folio for migrate_folio_extra()/migrate_folio() to
save three compound_head() calls.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/migrate_device.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index b6c27c76e1a0..d49a48d87d72 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -694,6 +694,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 		struct page *newpage = migrate_pfn_to_page(dst_pfns[i]);
 		struct page *page = migrate_pfn_to_page(src_pfns[i]);
 		struct address_space *mapping;
+		struct folio *newfolio, *folio;
 		int r;
 
 		if (!newpage) {
@@ -729,13 +730,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 		}
 
 		mapping = page_mapping(page);
+		folio = page_folio(page);
+		newfolio = page_folio(newpage);
 
-		if (is_device_private_page(newpage) ||
-		    is_device_coherent_page(newpage)) {
+		if (folio_is_device_private(newfolio) ||
+		    folio_is_device_coherent(newfolio)) {
 			if (mapping) {
-				struct folio *folio;
-
-				folio = page_folio(page);
 
 				/*
 				 * For now only support anonymous memory migrating to
@@ -749,7 +749,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 					continue;
 				}
 			}
-		} else if (is_zone_device_page(newpage)) {
+		} else if (folio_is_zone_device(newfolio)) {
 			/*
 			 * Other types of ZONE_DEVICE page are not supported.
 			 */
@@ -758,12 +758,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 		}
 
 		if (migrate && migrate->fault_page == page)
-			r = migrate_folio_extra(mapping, page_folio(newpage),
-						page_folio(page),
+			r = migrate_folio_extra(mapping, newfolio, folio,
 						MIGRATE_SYNC_NO_COPY, 1);
 		else
-			r = migrate_folio(mapping, page_folio(newpage),
-					page_folio(page), MIGRATE_SYNC_NO_COPY);
+			r = migrate_folio(mapping, newfolio, folio,
+					  MIGRATE_SYNC_NO_COPY);
 		if (r != MIGRATEPAGE_SUCCESS)
 			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
 	}
-- 
2.27.0



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

* [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio() Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages() Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-02-01 20:09   ` Matthew Wilcox
  2024-01-29  7:09 ` [PATCH rfc 4/9] mm: remove MIGRATE_SYNC_NO_COPY mode Kefeng Wang
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

Convert migrate_folio_extra() to __migrate_folio() which will be used
by migrate_folio() and filemap_migrate_folio(), also directly call
folio_migrate_mapping() in __migrate_device_pages() to simplify code.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/linux/migrate.h |  2 --
 mm/migrate.c            | 32 +++++++++++---------------------
 mm/migrate_device.c     | 13 +++++++------
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 2ce13e8a309b..517f70b70620 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -63,8 +63,6 @@ extern const char *migrate_reason_names[MR_TYPES];
 #ifdef CONFIG_MIGRATION
 
 void putback_movable_pages(struct list_head *l);
-int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
-		struct folio *src, enum migrate_mode mode, int extra_count);
 int migrate_folio(struct address_space *mapping, struct folio *dst,
 		struct folio *src, enum migrate_mode mode);
 int migrate_pages(struct list_head *l, new_folio_t new, free_folio_t free,
diff --git a/mm/migrate.c b/mm/migrate.c
index cdae25b7105f..a51ceebbe3b1 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -655,22 +655,24 @@ EXPORT_SYMBOL(folio_migrate_copy);
  *                    Migration functions
  ***********************************************************/
 
-int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
-		struct folio *src, enum migrate_mode mode, int extra_count)
+static int __migrate_folio(struct address_space *mapping, struct folio *dst,
+			   struct folio *src, enum migrate_mode mode,
+			   void *src_private)
 {
 	int rc;
 
-	BUG_ON(folio_test_writeback(src));	/* Writeback must be complete */
-
-	rc = folio_migrate_mapping(mapping, dst, src, extra_count);
-
+	rc = folio_migrate_mapping(mapping, dst, src, 0);
 	if (rc != MIGRATEPAGE_SUCCESS)
 		return rc;
 
+	if (src_private)
+		folio_attach_private(dst, folio_detach_private(src));
+
 	if (mode != MIGRATE_SYNC_NO_COPY)
 		folio_migrate_copy(dst, src);
 	else
 		folio_migrate_flags(dst, src);
+
 	return MIGRATEPAGE_SUCCESS;
 }
 
@@ -689,7 +691,8 @@ int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
 int migrate_folio(struct address_space *mapping, struct folio *dst,
 		struct folio *src, enum migrate_mode mode)
 {
-	return migrate_folio_extra(mapping, dst, src, mode, 0);
+	BUG_ON(folio_test_writeback(src));	/* Writeback must be complete */
+	return __migrate_folio(mapping, dst, src, mode, NULL);
 }
 EXPORT_SYMBOL(migrate_folio);
 
@@ -843,20 +846,7 @@ EXPORT_SYMBOL_GPL(buffer_migrate_folio_norefs);
 int filemap_migrate_folio(struct address_space *mapping,
 		struct folio *dst, struct folio *src, enum migrate_mode mode)
 {
-	int ret;
-
-	ret = folio_migrate_mapping(mapping, dst, src, 0);
-	if (ret != MIGRATEPAGE_SUCCESS)
-		return ret;
-
-	if (folio_get_private(src))
-		folio_attach_private(dst, folio_detach_private(src));
-
-	if (mode != MIGRATE_SYNC_NO_COPY)
-		folio_migrate_copy(dst, src);
-	else
-		folio_migrate_flags(dst, src);
-	return MIGRATEPAGE_SUCCESS;
+	return __migrate_folio(mapping, dst, src, mode, folio_get_private(src));
 }
 EXPORT_SYMBOL_GPL(filemap_migrate_folio);
 
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index d49a48d87d72..bea71d69295a 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -695,7 +695,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 		struct page *page = migrate_pfn_to_page(src_pfns[i]);
 		struct address_space *mapping;
 		struct folio *newfolio, *folio;
-		int r;
+		int r, extra_cnt = 0;
 
 		if (!newpage) {
 			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
@@ -757,14 +757,15 @@ static void __migrate_device_pages(unsigned long *src_pfns,
 			continue;
 		}
 
+		BUG_ON(folio_test_writeback(folio));
+
 		if (migrate && migrate->fault_page == page)
-			r = migrate_folio_extra(mapping, newfolio, folio,
-						MIGRATE_SYNC_NO_COPY, 1);
-		else
-			r = migrate_folio(mapping, newfolio, folio,
-					  MIGRATE_SYNC_NO_COPY);
+			extra_cnt = 1;
+		r = folio_migrate_mapping(mapping, newfolio, folio, extra_cnt);
 		if (r != MIGRATEPAGE_SUCCESS)
 			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
+		else
+			folio_migrate_flags(newfolio, folio);
 	}
 
 	if (notified)
-- 
2.27.0



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

* [PATCH rfc 4/9] mm: remove MIGRATE_SYNC_NO_COPY mode
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
                   ` (2 preceding siblings ...)
  2024-01-29  7:09 ` [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra() Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-02-01 20:23   ` Matthew Wilcox
  2024-01-29  7:09 ` [PATCH rfc 5/9] mm: add folio_mc_copy() Kefeng Wang
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

Commit 2916ecc0f9d4 ("mm/migrate: new migrate mode MIGRATE_SYNC_NO_COPY")
introduce a new MIGRATE_SYNC_NO_COPY mode to allow to offload the copy to
a device DMA engine, which is only used __migrate_device_pages() to decide
whether or not copy the old page, and the MIGRATE_SYNC_NO_COPY mode only
set in hmm, as the MIGRATE_SYNC_NO_COPY set is removed by previous cleanup,
it seems that we could remove the unnecessary MIGRATE_SYNC_NO_COPY.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 fs/aio.c                     | 12 +-----------
 fs/hugetlbfs/inode.c         |  5 +----
 include/linux/migrate_mode.h |  5 -----
 mm/balloon_compaction.c      |  8 --------
 mm/migrate.c                 |  8 +-------
 mm/zsmalloc.c                |  8 --------
 6 files changed, 3 insertions(+), 43 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index bb2ff48991f3..1d0ca2a2776d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -409,17 +409,7 @@ static int aio_migrate_folio(struct address_space *mapping, struct folio *dst,
 	struct kioctx *ctx;
 	unsigned long flags;
 	pgoff_t idx;
-	int rc;
-
-	/*
-	 * We cannot support the _NO_COPY case here, because copy needs to
-	 * happen under the ctx->completion_lock. That does not work with the
-	 * migration workflow of MIGRATE_SYNC_NO_COPY.
-	 */
-	if (mode == MIGRATE_SYNC_NO_COPY)
-		return -EINVAL;
-
-	rc = 0;
+	int rc = 0;
 
 	/* mapping->i_private_lock here protects against the kioctx teardown.  */
 	spin_lock(&mapping->i_private_lock);
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index ee13c2ca8ad2..52839ffdd9a1 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1129,10 +1129,7 @@ static int hugetlbfs_migrate_folio(struct address_space *mapping,
 		hugetlb_set_folio_subpool(src, NULL);
 	}
 
-	if (mode != MIGRATE_SYNC_NO_COPY)
-		folio_migrate_copy(dst, src);
-	else
-		folio_migrate_flags(dst, src);
+	folio_migrate_copy(dst, src);
 
 	return MIGRATEPAGE_SUCCESS;
 }
diff --git a/include/linux/migrate_mode.h b/include/linux/migrate_mode.h
index f37cc03f9369..9fb482bb7323 100644
--- a/include/linux/migrate_mode.h
+++ b/include/linux/migrate_mode.h
@@ -7,16 +7,11 @@
  *	on most operations but not ->writepage as the potential stall time
  *	is too significant
  * MIGRATE_SYNC will block when migrating pages
- * MIGRATE_SYNC_NO_COPY will block when migrating pages but will not copy pages
- *	with the CPU. Instead, page copy happens outside the migratepage()
- *	callback and is likely using a DMA engine. See migrate_vma() and HMM
- *	(mm/hmm.c) for users of this mode.
  */
 enum migrate_mode {
 	MIGRATE_ASYNC,
 	MIGRATE_SYNC_LIGHT,
 	MIGRATE_SYNC,
-	MIGRATE_SYNC_NO_COPY,
 };
 
 enum migrate_reason {
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index 22c96fed70b5..6597ebea8ae2 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -234,14 +234,6 @@ static int balloon_page_migrate(struct page *newpage, struct page *page,
 {
 	struct balloon_dev_info *balloon = balloon_page_device(page);
 
-	/*
-	 * We can not easily support the no copy case here so ignore it as it
-	 * is unlikely to be used with balloon pages. See include/linux/hmm.h
-	 * for a user of the MIGRATE_SYNC_NO_COPY mode.
-	 */
-	if (mode == MIGRATE_SYNC_NO_COPY)
-		return -EINVAL;
-
 	VM_BUG_ON_PAGE(!PageLocked(page), page);
 	VM_BUG_ON_PAGE(!PageLocked(newpage), newpage);
 
diff --git a/mm/migrate.c b/mm/migrate.c
index a51ceebbe3b1..107965bbc852 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -668,10 +668,7 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst,
 	if (src_private)
 		folio_attach_private(dst, folio_detach_private(src));
 
-	if (mode != MIGRATE_SYNC_NO_COPY)
-		folio_migrate_copy(dst, src);
-	else
-		folio_migrate_flags(dst, src);
+	folio_migrate_copy(dst, src);
 
 	return MIGRATEPAGE_SUCCESS;
 }
@@ -901,7 +898,6 @@ static int fallback_migrate_folio(struct address_space *mapping,
 		/* Only writeback folios in full synchronous migration */
 		switch (mode) {
 		case MIGRATE_SYNC:
-		case MIGRATE_SYNC_NO_COPY:
 			break;
 		default:
 			return -EBUSY;
@@ -1159,7 +1155,6 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
 		 */
 		switch (mode) {
 		case MIGRATE_SYNC:
-		case MIGRATE_SYNC_NO_COPY:
 			break;
 		default:
 			rc = -EBUSY;
@@ -1370,7 +1365,6 @@ static int unmap_and_move_huge_page(new_folio_t get_new_folio,
 			goto out;
 		switch (mode) {
 		case MIGRATE_SYNC:
-		case MIGRATE_SYNC_NO_COPY:
 			break;
 		default:
 			goto out;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c937635e0ad1..b9ffe1a041ca 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1811,14 +1811,6 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
 	unsigned long old_obj, new_obj;
 	unsigned int obj_idx;
 
-	/*
-	 * We cannot support the _NO_COPY case here, because copy needs to
-	 * happen under the zs lock, which does not work with
-	 * MIGRATE_SYNC_NO_COPY workflow.
-	 */
-	if (mode == MIGRATE_SYNC_NO_COPY)
-		return -EINVAL;
-
 	VM_BUG_ON_PAGE(!PageIsolated(page), page);
 
 	/* The page is locked, so this pointer must remain valid */
-- 
2.27.0



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

* [PATCH rfc 5/9] mm: add folio_mc_copy()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
                   ` (3 preceding siblings ...)
  2024-01-29  7:09 ` [PATCH rfc 4/9] mm: remove MIGRATE_SYNC_NO_COPY mode Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio Kefeng Wang
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

Add a variant of folio_copy() which use copy_mc_highpage() to support
machine check safe copy when folio copy.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 include/linux/mm.h |  1 +
 mm/util.c          | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ac6b71cbdffb..fffa6025d8f9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1300,6 +1300,7 @@ void put_pages_list(struct list_head *pages);
 
 void split_page(struct page *page, unsigned int order);
 void folio_copy(struct folio *dst, struct folio *src);
+int folio_mc_copy(struct folio *dst, struct folio *src);
 
 unsigned long nr_free_buffer_pages(void);
 
diff --git a/mm/util.c b/mm/util.c
index 5faf3adc6f43..63e6e644eb5a 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -828,6 +828,26 @@ void folio_copy(struct folio *dst, struct folio *src)
 }
 EXPORT_SYMBOL(folio_copy);
 
+int folio_mc_copy(struct folio *dst, struct folio *src)
+{
+	long nr = folio_nr_pages(src);
+	long i = 0;
+	int ret = 0;
+
+	for (;;) {
+		if (copy_mc_highpage(folio_page(dst, i), folio_page(src, i))) {
+			ret = -EFAULT;
+			break;
+		}
+		if (++i == nr)
+			break;
+		cond_resched();
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(folio_mc_copy);
+
 int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
 int sysctl_overcommit_ratio __read_mostly = 50;
 unsigned long sysctl_overcommit_kbytes __read_mostly;
-- 
2.27.0



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

* [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
                   ` (4 preceding siblings ...)
  2024-01-29  7:09 ` [PATCH rfc 5/9] mm: add folio_mc_copy() Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-02-01 20:34   ` Matthew Wilcox
  2024-01-29  7:09 ` [PATCH rfc 7/9] fs: hugetlbfs: support poison recover from hugetlbfs_migrate_folio() Kefeng Wang
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

In order to support poisoned folio copy recover from migrate folio,
let's use folio_mc_copy() and move it in the begin of the function
of __migrate_folio(), which could simply error handling since there
is no turning back if folio_migrate_mapping() return success, the
downside is the folio copied even though folio_migrate_mapping()
return fail, a small optimization is to check whether folio does
not have extra refs before we do more work ahead in __migrate_folio(),
which could help us avoid unnecessary folio copy.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/migrate.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 107965bbc852..99286394b5e5 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -661,6 +661,14 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst,
 {
 	int rc;
 
+	/* Check whether src does not have extra refs before we do more work */
+	if (folio_ref_count(src) != folio_expected_refs(mapping, src))
+		return -EAGAIN;
+
+	rc = folio_mc_copy(dst, src);
+	if (rc)
+		return rc;
+
 	rc = folio_migrate_mapping(mapping, dst, src, 0);
 	if (rc != MIGRATEPAGE_SUCCESS)
 		return rc;
@@ -668,7 +676,7 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst,
 	if (src_private)
 		folio_attach_private(dst, folio_detach_private(src));
 
-	folio_migrate_copy(dst, src);
+	folio_migrate_flags(dst, src);
 
 	return MIGRATEPAGE_SUCCESS;
 }
-- 
2.27.0



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

* [PATCH rfc 7/9] fs: hugetlbfs: support poison recover from hugetlbfs_migrate_folio()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
                   ` (5 preceding siblings ...)
  2024-01-29  7:09 ` [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 8/9] mm: migrate: remove folio_migrate_copy() Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 9/9] fs: aio: add explicit check for large folio in aio_migrate_folio() Kefeng Wang
  8 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

This is similar to __migrate_folio(), use folio_mc_copy() to avoid
panic when copy poisoned folio.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 fs/hugetlbfs/inode.c    | 10 +++++++++-
 include/linux/migrate.h |  1 +
 mm/migrate.c            |  3 +--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 52839ffdd9a1..3871968d1780 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1119,6 +1119,14 @@ static int hugetlbfs_migrate_folio(struct address_space *mapping,
 {
 	int rc;
 
+	/* Check whether src does not have extra refs before we do more work */
+	if (folio_ref_count(src) != folio_expected_refs(mapping, src))
+		return -EAGAIN;
+
+	rc = folio_mc_copy(dst, src);
+	if (rc)
+		return rc;
+
 	rc = migrate_huge_page_move_mapping(mapping, dst, src);
 	if (rc != MIGRATEPAGE_SUCCESS)
 		return rc;
@@ -1129,7 +1137,7 @@ static int hugetlbfs_migrate_folio(struct address_space *mapping,
 		hugetlb_set_folio_subpool(src, NULL);
 	}
 
-	folio_migrate_copy(dst, src);
+	folio_migrate_flags(dst, src);
 
 	return MIGRATEPAGE_SUCCESS;
 }
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 517f70b70620..ab387ea66365 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -79,6 +79,7 @@ void folio_migrate_flags(struct folio *newfolio, struct folio *folio);
 void folio_migrate_copy(struct folio *newfolio, struct folio *folio);
 int folio_migrate_mapping(struct address_space *mapping,
 		struct folio *newfolio, struct folio *folio, int extra_count);
+int folio_expected_refs(struct address_space *mapping, struct folio *folio);
 
 #else
 
diff --git a/mm/migrate.c b/mm/migrate.c
index 99286394b5e5..097d67c82f8b 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -375,8 +375,7 @@ void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd)
 }
 #endif
 
-static int folio_expected_refs(struct address_space *mapping,
-		struct folio *folio)
+int folio_expected_refs(struct address_space *mapping, struct folio *folio)
 {
 	int refs = 1;
 	if (!mapping)
-- 
2.27.0



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

* [PATCH rfc 8/9] mm: migrate: remove folio_migrate_copy()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
                   ` (6 preceding siblings ...)
  2024-01-29  7:09 ` [PATCH rfc 7/9] fs: hugetlbfs: support poison recover from hugetlbfs_migrate_folio() Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  2024-01-29  7:09 ` [PATCH rfc 9/9] fs: aio: add explicit check for large folio in aio_migrate_folio() Kefeng Wang
  8 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

The folio_migrate_copy() is just a wrapper of folio_copy() and
folio_migrate_flags(), it is simple and only aio use it for now,
unfold it and remove folio_migrate_copy().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 fs/aio.c                | 3 ++-
 include/linux/migrate.h | 1 -
 mm/migrate.c            | 7 -------
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 1d0ca2a2776d..631e83eee5a1 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -454,7 +454,8 @@ static int aio_migrate_folio(struct address_space *mapping, struct folio *dst,
 	 * events from being lost.
 	 */
 	spin_lock_irqsave(&ctx->completion_lock, flags);
-	folio_migrate_copy(dst, src);
+	folio_copy(dst, src);
+	folio_migrate_flags(dst, src);
 	BUG_ON(ctx->ring_pages[idx] != &src->page);
 	ctx->ring_pages[idx] = &dst->page;
 	spin_unlock_irqrestore(&ctx->completion_lock, flags);
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index ab387ea66365..13fff8f7832b 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -76,7 +76,6 @@ int migrate_huge_page_move_mapping(struct address_space *mapping,
 void migration_entry_wait_on_locked(swp_entry_t entry, spinlock_t *ptl)
 		__releases(ptl);
 void folio_migrate_flags(struct folio *newfolio, struct folio *folio);
-void folio_migrate_copy(struct folio *newfolio, struct folio *folio);
 int folio_migrate_mapping(struct address_space *mapping,
 		struct folio *newfolio, struct folio *folio, int extra_count);
 int folio_expected_refs(struct address_space *mapping, struct folio *folio);
diff --git a/mm/migrate.c b/mm/migrate.c
index 097d67c82f8b..d5c1b1542335 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -643,13 +643,6 @@ void folio_migrate_flags(struct folio *newfolio, struct folio *folio)
 }
 EXPORT_SYMBOL(folio_migrate_flags);
 
-void folio_migrate_copy(struct folio *newfolio, struct folio *folio)
-{
-	folio_copy(newfolio, folio);
-	folio_migrate_flags(newfolio, folio);
-}
-EXPORT_SYMBOL(folio_migrate_copy);
-
 /************************************************************
  *                    Migration functions
  ***********************************************************/
-- 
2.27.0



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

* [PATCH rfc 9/9] fs: aio: add explicit check for large folio in aio_migrate_folio()
  2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
                   ` (7 preceding siblings ...)
  2024-01-29  7:09 ` [PATCH rfc 8/9] mm: migrate: remove folio_migrate_copy() Kefeng Wang
@ 2024-01-29  7:09 ` Kefeng Wang
  8 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Andrew Morton, linux-mm
  Cc: Tony Luck, Naoya Horiguchi, Miaohe Lin, Matthew Wilcox,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel, Kefeng Wang

Since large folio copy could spend lots of time and it is involved with
a cond_resched(), the aio couldn't support migrate large folio as it takes
a spin lock when folio copy, add explicit check for large folio and return
err directly.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 fs/aio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/aio.c b/fs/aio.c
index 631e83eee5a1..372f22b85b11 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -411,6 +411,10 @@ static int aio_migrate_folio(struct address_space *mapping, struct folio *dst,
 	pgoff_t idx;
 	int rc = 0;
 
+	/* Large folio aren't supported */
+	if (folio_test_large(src))
+		return -EINVAL;
+
 	/* mapping->i_private_lock here protects against the kioctx teardown.  */
 	spin_lock(&mapping->i_private_lock);
 	ctx = mapping->i_private_data;
-- 
2.27.0



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

* Re: [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio()
  2024-01-29  7:09 ` [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio() Kefeng Wang
@ 2024-02-01 19:12   ` Matthew Wilcox
  0 siblings, 0 replies; 19+ messages in thread
From: Matthew Wilcox @ 2024-02-01 19:12 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel

On Mon, Jan 29, 2024 at 03:09:26PM +0800, Kefeng Wang wrote:
> @@ -774,24 +774,16 @@ static int __buffer_migrate_folio(struct address_space *mapping,
>  		}
>  	}
>  
> -	rc = folio_migrate_mapping(mapping, dst, src, 0);
> +	rc = filemap_migrate_folio(mapping, dst, src, mode);
>  	if (rc != MIGRATEPAGE_SUCCESS)
>  		goto unlock_buffers;
>  
> -	folio_attach_private(dst, folio_detach_private(src));
> -
>  	bh = head;
>  	do {
>  		folio_set_bh(bh, dst, bh_offset(bh));
>  		bh = bh->b_this_page;
>  	} while (bh != head);
>  
> -	if (mode != MIGRATE_SYNC_NO_COPY)
> -		folio_migrate_copy(dst, src);
> -	else
> -		folio_migrate_flags(dst, src);
> -
> -	rc = MIGRATEPAGE_SUCCESS;

I wondered if maybe there was an ordering requirement; that we had to
set up the BHs before copying the data over.  But I don't think there
is; the page should be frozen, the buffer heads are locked, and I don't
think the ordering matters.  So ...

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>


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

* Re: [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages()
  2024-01-29  7:09 ` [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages() Kefeng Wang
@ 2024-02-01 19:27   ` Matthew Wilcox
  2024-02-02  2:44     ` Kefeng Wang
  0 siblings, 1 reply; 19+ messages in thread
From: Matthew Wilcox @ 2024-02-01 19:27 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel

On Mon, Jan 29, 2024 at 03:09:27PM +0800, Kefeng Wang wrote:
> Use newfolio/folio for migrate_folio_extra()/migrate_folio() to
> save three compound_head() calls.

I've looked at this function before and I get stuck on the question of
whether a device page truly is a folio or not.  I think for the moment,
we've decided that it is, so ..

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Although ...

> @@ -729,13 +730,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  		}
>  
>  		mapping = page_mapping(page);
> +		folio = page_folio(page);
> +		newfolio = page_folio(newpage);

you should save one more call to compound_head() by changing the
page_mapping() to folio_mapping().



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

* Re: [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra()
  2024-01-29  7:09 ` [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra() Kefeng Wang
@ 2024-02-01 20:09   ` Matthew Wilcox
  2024-02-02  2:46     ` Kefeng Wang
  0 siblings, 1 reply; 19+ messages in thread
From: Matthew Wilcox @ 2024-02-01 20:09 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel

On Mon, Jan 29, 2024 at 03:09:28PM +0800, Kefeng Wang wrote:
> Convert migrate_folio_extra() to __migrate_folio() which will be used
> by migrate_folio() and filemap_migrate_folio(), also directly call
> folio_migrate_mapping() in __migrate_device_pages() to simplify code.

This feels like two patches?  First convert __migrate_device_pages()
then do the other thing?

> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  include/linux/migrate.h |  2 --
>  mm/migrate.c            | 32 +++++++++++---------------------
>  mm/migrate_device.c     | 13 +++++++------
>  3 files changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index 2ce13e8a309b..517f70b70620 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -63,8 +63,6 @@ extern const char *migrate_reason_names[MR_TYPES];
>  #ifdef CONFIG_MIGRATION
>  
>  void putback_movable_pages(struct list_head *l);
> -int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
> -		struct folio *src, enum migrate_mode mode, int extra_count);
>  int migrate_folio(struct address_space *mapping, struct folio *dst,
>  		struct folio *src, enum migrate_mode mode);
>  int migrate_pages(struct list_head *l, new_folio_t new, free_folio_t free,
> diff --git a/mm/migrate.c b/mm/migrate.c
> index cdae25b7105f..a51ceebbe3b1 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -655,22 +655,24 @@ EXPORT_SYMBOL(folio_migrate_copy);
>   *                    Migration functions
>   ***********************************************************/
>  
> -int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
> -		struct folio *src, enum migrate_mode mode, int extra_count)
> +static int __migrate_folio(struct address_space *mapping, struct folio *dst,
> +			   struct folio *src, enum migrate_mode mode,
> +			   void *src_private)
>  {
>  	int rc;
>  
> -	BUG_ON(folio_test_writeback(src));	/* Writeback must be complete */
> -
> -	rc = folio_migrate_mapping(mapping, dst, src, extra_count);
> -
> +	rc = folio_migrate_mapping(mapping, dst, src, 0);
>  	if (rc != MIGRATEPAGE_SUCCESS)
>  		return rc;
>  
> +	if (src_private)
> +		folio_attach_private(dst, folio_detach_private(src));
> +
>  	if (mode != MIGRATE_SYNC_NO_COPY)
>  		folio_migrate_copy(dst, src);
>  	else
>  		folio_migrate_flags(dst, src);
> +
>  	return MIGRATEPAGE_SUCCESS;
>  }
>  
> @@ -689,7 +691,8 @@ int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
>  int migrate_folio(struct address_space *mapping, struct folio *dst,
>  		struct folio *src, enum migrate_mode mode)
>  {
> -	return migrate_folio_extra(mapping, dst, src, mode, 0);
> +	BUG_ON(folio_test_writeback(src));	/* Writeback must be complete */
> +	return __migrate_folio(mapping, dst, src, mode, NULL);
>  }
>  EXPORT_SYMBOL(migrate_folio);
>  
> @@ -843,20 +846,7 @@ EXPORT_SYMBOL_GPL(buffer_migrate_folio_norefs);
>  int filemap_migrate_folio(struct address_space *mapping,
>  		struct folio *dst, struct folio *src, enum migrate_mode mode)
>  {
> -	int ret;
> -
> -	ret = folio_migrate_mapping(mapping, dst, src, 0);
> -	if (ret != MIGRATEPAGE_SUCCESS)
> -		return ret;
> -
> -	if (folio_get_private(src))
> -		folio_attach_private(dst, folio_detach_private(src));
> -
> -	if (mode != MIGRATE_SYNC_NO_COPY)
> -		folio_migrate_copy(dst, src);
> -	else
> -		folio_migrate_flags(dst, src);
> -	return MIGRATEPAGE_SUCCESS;
> +	return __migrate_folio(mapping, dst, src, mode, folio_get_private(src));
>  }
>  EXPORT_SYMBOL_GPL(filemap_migrate_folio);
>  
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index d49a48d87d72..bea71d69295a 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -695,7 +695,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  		struct page *page = migrate_pfn_to_page(src_pfns[i]);
>  		struct address_space *mapping;
>  		struct folio *newfolio, *folio;
> -		int r;
> +		int r, extra_cnt = 0;
>  
>  		if (!newpage) {
>  			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> @@ -757,14 +757,15 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  			continue;
>  		}
>  
> +		BUG_ON(folio_test_writeback(folio));
> +
>  		if (migrate && migrate->fault_page == page)
> -			r = migrate_folio_extra(mapping, newfolio, folio,
> -						MIGRATE_SYNC_NO_COPY, 1);
> -		else
> -			r = migrate_folio(mapping, newfolio, folio,
> -					  MIGRATE_SYNC_NO_COPY);
> +			extra_cnt = 1;
> +		r = folio_migrate_mapping(mapping, newfolio, folio, extra_cnt);
>  		if (r != MIGRATEPAGE_SUCCESS)
>  			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> +		else
> +			folio_migrate_flags(newfolio, folio);
>  	}
>  
>  	if (notified)
> -- 
> 2.27.0
> 


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

* Re: [PATCH rfc 4/9] mm: remove MIGRATE_SYNC_NO_COPY mode
  2024-01-29  7:09 ` [PATCH rfc 4/9] mm: remove MIGRATE_SYNC_NO_COPY mode Kefeng Wang
@ 2024-02-01 20:23   ` Matthew Wilcox
  0 siblings, 0 replies; 19+ messages in thread
From: Matthew Wilcox @ 2024-02-01 20:23 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel

On Mon, Jan 29, 2024 at 03:09:29PM +0800, Kefeng Wang wrote:
> Commit 2916ecc0f9d4 ("mm/migrate: new migrate mode MIGRATE_SYNC_NO_COPY")
> introduce a new MIGRATE_SYNC_NO_COPY mode to allow to offload the copy to
> a device DMA engine, which is only used __migrate_device_pages() to decide
> whether or not copy the old page, and the MIGRATE_SYNC_NO_COPY mode only
> set in hmm, as the MIGRATE_SYNC_NO_COPY set is removed by previous cleanup,
> it seems that we could remove the unnecessary MIGRATE_SYNC_NO_COPY.

Ah!  I didn't understand the point of what you were doing in the
previous patch.  Now it makes a lot more sense.  This is a big
improvement, thanks!


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

* Re: [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio
  2024-01-29  7:09 ` [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio Kefeng Wang
@ 2024-02-01 20:34   ` Matthew Wilcox
  2024-02-02  3:04     ` Kefeng Wang
  0 siblings, 1 reply; 19+ messages in thread
From: Matthew Wilcox @ 2024-02-01 20:34 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel

On Mon, Jan 29, 2024 at 03:09:31PM +0800, Kefeng Wang wrote:
> In order to support poisoned folio copy recover from migrate folio,
> let's use folio_mc_copy() and move it in the begin of the function
> of __migrate_folio(), which could simply error handling since there
> is no turning back if folio_migrate_mapping() return success, the
> downside is the folio copied even though folio_migrate_mapping()
> return fail, a small optimization is to check whether folio does
> not have extra refs before we do more work ahead in __migrate_folio(),
> which could help us avoid unnecessary folio copy.

OK, I see why you've done it this way.

Would it make more sense if we pulled the folio refcount freezing
out of folio_migrate_mapping() into its callers?  That way
folio_migrate_mapping() could never fail.


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

* Re: [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages()
  2024-02-01 19:27   ` Matthew Wilcox
@ 2024-02-02  2:44     ` Kefeng Wang
  0 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-02-02  2:44 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel



On 2024/2/2 3:27, Matthew Wilcox wrote:
> On Mon, Jan 29, 2024 at 03:09:27PM +0800, Kefeng Wang wrote:
>> Use newfolio/folio for migrate_folio_extra()/migrate_folio() to
>> save three compound_head() calls.
> 
> I've looked at this function before and I get stuck on the question of
> whether a device page truly is a folio or not.  I think for the moment,
> we've decided that it is, so ..
> 
> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> Although ...
> 
>> @@ -729,13 +730,12 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>>   		}
>>   
>>   		mapping = page_mapping(page);
>> +		folio = page_folio(page);
>> +		newfolio = page_folio(newpage);
> 
> you should save one more call to compound_head() by changing the
> page_mapping() to folio_mapping().

Missing this one, will update, thanks.
> 


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

* Re: [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra()
  2024-02-01 20:09   ` Matthew Wilcox
@ 2024-02-02  2:46     ` Kefeng Wang
  0 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-02-02  2:46 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel



On 2024/2/2 4:09, Matthew Wilcox wrote:
> On Mon, Jan 29, 2024 at 03:09:28PM +0800, Kefeng Wang wrote:
>> Convert migrate_folio_extra() to __migrate_folio() which will be used
>> by migrate_folio() and filemap_migrate_folio(), also directly call
>> folio_migrate_mapping() in __migrate_device_pages() to simplify code.
> 
> This feels like two patches?  First convert __migrate_device_pages()
> then do the other thing?

That will be better, will split it, thanks

> 
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   include/linux/migrate.h |  2 --
>>   mm/migrate.c            | 32 +++++++++++---------------------
>>   mm/migrate_device.c     | 13 +++++++------
>>   3 files changed, 18 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
>> index 2ce13e8a309b..517f70b70620 100644
>> --- a/include/linux/migrate.h
>> +++ b/include/linux/migrate.h
>> @@ -63,8 +63,6 @@ extern const char *migrate_reason_names[MR_TYPES];
>>   #ifdef CONFIG_MIGRATION
>>   
>>   void putback_movable_pages(struct list_head *l);
>> -int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
>> -		struct folio *src, enum migrate_mode mode, int extra_count);
>>   int migrate_folio(struct address_space *mapping, struct folio *dst,
>>   		struct folio *src, enum migrate_mode mode);
>>   int migrate_pages(struct list_head *l, new_folio_t new, free_folio_t free,
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index cdae25b7105f..a51ceebbe3b1 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -655,22 +655,24 @@ EXPORT_SYMBOL(folio_migrate_copy);
>>    *                    Migration functions
>>    ***********************************************************/
>>   
>> -int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
>> -		struct folio *src, enum migrate_mode mode, int extra_count)
>> +static int __migrate_folio(struct address_space *mapping, struct folio *dst,
>> +			   struct folio *src, enum migrate_mode mode,
>> +			   void *src_private)
>>   {
>>   	int rc;
>>   
>> -	BUG_ON(folio_test_writeback(src));	/* Writeback must be complete */
>> -
>> -	rc = folio_migrate_mapping(mapping, dst, src, extra_count);
>> -
>> +	rc = folio_migrate_mapping(mapping, dst, src, 0);
>>   	if (rc != MIGRATEPAGE_SUCCESS)
>>   		return rc;
>>   
>> +	if (src_private)
>> +		folio_attach_private(dst, folio_detach_private(src));
>> +
>>   	if (mode != MIGRATE_SYNC_NO_COPY)
>>   		folio_migrate_copy(dst, src);
>>   	else
>>   		folio_migrate_flags(dst, src);
>> +
>>   	return MIGRATEPAGE_SUCCESS;
>>   }
>>   
>> @@ -689,7 +691,8 @@ int migrate_folio_extra(struct address_space *mapping, struct folio *dst,
>>   int migrate_folio(struct address_space *mapping, struct folio *dst,
>>   		struct folio *src, enum migrate_mode mode)
>>   {
>> -	return migrate_folio_extra(mapping, dst, src, mode, 0);
>> +	BUG_ON(folio_test_writeback(src));	/* Writeback must be complete */
>> +	return __migrate_folio(mapping, dst, src, mode, NULL);
>>   }
>>   EXPORT_SYMBOL(migrate_folio);
>>   
>> @@ -843,20 +846,7 @@ EXPORT_SYMBOL_GPL(buffer_migrate_folio_norefs);
>>   int filemap_migrate_folio(struct address_space *mapping,
>>   		struct folio *dst, struct folio *src, enum migrate_mode mode)
>>   {
>> -	int ret;
>> -
>> -	ret = folio_migrate_mapping(mapping, dst, src, 0);
>> -	if (ret != MIGRATEPAGE_SUCCESS)
>> -		return ret;
>> -
>> -	if (folio_get_private(src))
>> -		folio_attach_private(dst, folio_detach_private(src));
>> -
>> -	if (mode != MIGRATE_SYNC_NO_COPY)
>> -		folio_migrate_copy(dst, src);
>> -	else
>> -		folio_migrate_flags(dst, src);
>> -	return MIGRATEPAGE_SUCCESS;
>> +	return __migrate_folio(mapping, dst, src, mode, folio_get_private(src));
>>   }
>>   EXPORT_SYMBOL_GPL(filemap_migrate_folio);
>>   
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index d49a48d87d72..bea71d69295a 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -695,7 +695,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>>   		struct page *page = migrate_pfn_to_page(src_pfns[i]);
>>   		struct address_space *mapping;
>>   		struct folio *newfolio, *folio;
>> -		int r;
>> +		int r, extra_cnt = 0;
>>   
>>   		if (!newpage) {
>>   			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
>> @@ -757,14 +757,15 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>>   			continue;
>>   		}
>>   
>> +		BUG_ON(folio_test_writeback(folio));
>> +
>>   		if (migrate && migrate->fault_page == page)
>> -			r = migrate_folio_extra(mapping, newfolio, folio,
>> -						MIGRATE_SYNC_NO_COPY, 1);
>> -		else
>> -			r = migrate_folio(mapping, newfolio, folio,
>> -					  MIGRATE_SYNC_NO_COPY);
>> +			extra_cnt = 1;
>> +		r = folio_migrate_mapping(mapping, newfolio, folio, extra_cnt);
>>   		if (r != MIGRATEPAGE_SUCCESS)
>>   			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
>> +		else
>> +			folio_migrate_flags(newfolio, folio);
>>   	}
>>   
>>   	if (notified)
>> -- 
>> 2.27.0
>>


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

* Re: [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio
  2024-02-01 20:34   ` Matthew Wilcox
@ 2024-02-02  3:04     ` Kefeng Wang
  2024-02-02  9:06       ` Kefeng Wang
  0 siblings, 1 reply; 19+ messages in thread
From: Kefeng Wang @ 2024-02-02  3:04 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel



On 2024/2/2 4:34, Matthew Wilcox wrote:
> On Mon, Jan 29, 2024 at 03:09:31PM +0800, Kefeng Wang wrote:
>> In order to support poisoned folio copy recover from migrate folio,
>> let's use folio_mc_copy() and move it in the begin of the function
>> of __migrate_folio(), which could simply error handling since there
>> is no turning back if folio_migrate_mapping() return success, the
>> downside is the folio copied even though folio_migrate_mapping()
>> return fail, a small optimization is to check whether folio does
>> not have extra refs before we do more work ahead in __migrate_folio(),
>> which could help us avoid unnecessary folio copy.
> 
> OK, I see why you've done it this way.
> 
> Would it make more sense if we pulled the folio refcount freezing
> out of folio_migrate_mapping() into its callers?  That way
> folio_migrate_mapping() could never fail.

Will try this way, thank.


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

* Re: [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio
  2024-02-02  3:04     ` Kefeng Wang
@ 2024-02-02  9:06       ` Kefeng Wang
  0 siblings, 0 replies; 19+ messages in thread
From: Kefeng Wang @ 2024-02-02  9:06 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andrew Morton, linux-mm, Tony Luck, Naoya Horiguchi, Miaohe Lin,
	David Hildenbrand, Muchun Song, Benjamin LaHaise, jglisse,
	linux-aio, linux-fsdevel



On 2024/2/2 11:04, Kefeng Wang wrote:
> 
> 
> On 2024/2/2 4:34, Matthew Wilcox wrote:
>> On Mon, Jan 29, 2024 at 03:09:31PM +0800, Kefeng Wang wrote:
>>> In order to support poisoned folio copy recover from migrate folio,
>>> let's use folio_mc_copy() and move it in the begin of the function
>>> of __migrate_folio(), which could simply error handling since there
>>> is no turning back if folio_migrate_mapping() return success, the
>>> downside is the folio copied even though folio_migrate_mapping()
>>> return fail, a small optimization is to check whether folio does
>>> not have extra refs before we do more work ahead in __migrate_folio(),
>>> which could help us avoid unnecessary folio copy.
>>
>> OK, I see why you've done it this way.
>>
>> Would it make more sense if we pulled the folio refcount freezing
>> out of folio_migrate_mapping() into its callers?  That way
>> folio_migrate_mapping() could never fail.
> 
Question, the folio ref freezing is under the xas_lock_irq(), it can't
be moved out of lock, and if with xas lock irq, we couldn't call
folio_mc_copy(), so the above way is not feasible, or maybe I missing
something?




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

end of thread, other threads:[~2024-02-02  9:06 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-29  7:09 [PATCH rfc 0/9] mm: migrate: support poison recover from migrate folio Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 1/9] mm: migrate: simplify __buffer_migrate_folio() Kefeng Wang
2024-02-01 19:12   ` Matthew Wilcox
2024-01-29  7:09 ` [PATCH rfc 2/9] mm: migrate_device: use more folio in __migrate_device_pages() Kefeng Wang
2024-02-01 19:27   ` Matthew Wilcox
2024-02-02  2:44     ` Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 3/9] mm: migrate: remove migrate_folio_extra() Kefeng Wang
2024-02-01 20:09   ` Matthew Wilcox
2024-02-02  2:46     ` Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 4/9] mm: remove MIGRATE_SYNC_NO_COPY mode Kefeng Wang
2024-02-01 20:23   ` Matthew Wilcox
2024-01-29  7:09 ` [PATCH rfc 5/9] mm: add folio_mc_copy() Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 6/9] mm: migrate: support poisoned recover from migrate folio Kefeng Wang
2024-02-01 20:34   ` Matthew Wilcox
2024-02-02  3:04     ` Kefeng Wang
2024-02-02  9:06       ` Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 7/9] fs: hugetlbfs: support poison recover from hugetlbfs_migrate_folio() Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 8/9] mm: migrate: remove folio_migrate_copy() Kefeng Wang
2024-01-29  7:09 ` [PATCH rfc 9/9] fs: aio: add explicit check for large folio in aio_migrate_folio() Kefeng Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).