Re: [PATCH 5/5] mm: remove MIGRATE_SYNC_NO_COPY mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/23/2024 10:28 PM, 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
nit:  s/only used/only used in/
whether or not copy the old page, and the MIGRATE_SYNC_NO_COPY mode only
s/copy/to copy/,  s/mode only set/mode is only set/
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@xxxxxxxxxx>
---
  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 57c9f7c077e6..07ff8bbdcd2a 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -410,17 +410,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 412f295acebe..6df794ed4066 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1128,10 +1128,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 1d1cb832fdb4..e04b451c4289 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -671,10 +671,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;
  }
@@ -903,7 +900,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;
@@ -1161,7 +1157,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;
@@ -1372,7 +1367,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 b42d3545ca85..6e7967853477 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1752,14 +1752,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 */

Except the nits above, patch looks good to me.

Reviewed-by: Jane Chu <jane.chu@xxxxxxxxxx>

-jane






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux