mm: migrate: Fix page migration stalls for blkdev pages

PATCH In review Jan Kara 3 participants 21 messages Dec 11, 2018 – Dec 17, 2018

Subsystems (6)

Memory management VFS F2FS file system MM migration +2 more subsystems

lore

Share & tools

lore

Jan Kara

author reviewer

2018-12-11 17:21 UTC #0 lore raw

Hello,

this patch set deals with page migration stalls that were reported by our customer due to block device page that had buffer head that was in bh LRU cache.

The patch set modifies page migration code so that buffer heads are completely handled inside buffer_migrate_page() and then provides new migration helper for pages with buffer heads that is safe to use even for block device pages and that also deals with bh lrus.

Honza

Jan Kara

author reviewer

2018-12-11 17:21 UTC #1 lore raw

PATCH 1/6 mm: migration: Factor out code to compute expected number of page references

Factor out function to compute number of expected page references in migrate_page_move_mapping(). Note that we move hpage_nr_pages() and page_has_private() checks from under xas_lock_irq() however this is safe since we hold page lock.

Signed-off-by Jan Kara <[email protected]>

---
 mm/migrate.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

mm/migrate.c +17 ~~-10~~

index f7e4bfdc13b7..789c7bc90a0c 100644
@@ -428,6 +428,22 @@ static inline bool buffer_migrate_lock_buffers(struct buffer_head *head,
 }
 #endif /* CONFIG_BLOCK */
 
+static int expected_page_refs(struct page *page)
+{
+	int expected_count = 1;
+
+	/*
+	 * Device public or private pages have an extra refcount as they are
+	 * ZONE_DEVICE pages.
+	 */
+	expected_count += is_device_private_page(page);
+	expected_count += is_device_public_page(page);
+	if (page->mapping)
+		expected_count += hpage_nr_pages(page) + page_has_private(page);
+
+	return expected_count;
+}
+
 /*
  * Replace the page in the mapping.
  *

@@ -444,14 +460,7 @@ int migrate_page_move_mapping(struct address_space *mapping,
 	XA_STATE(xas, &mapping->i_pages, page_index(page));
 	struct zone *oldzone, *newzone;
 	int dirty;
-	int expected_count = 1 + extra_count;
-
-	/*
-	 * Device public or private pages have an extra refcount as they are
-	 * ZONE_DEVICE pages.
-	 */
-	expected_count += is_device_private_page(page);
-	expected_count += is_device_public_page(page);
+	int expected_count = expected_page_refs(page) + extra_count;
 
 	if (!mapping) {
 		/* Anonymous page without mapping */

@@ -471,8 +480,6 @@ int migrate_page_move_mapping(struct address_space *mapping,
 	newzone = page_zone(newpage);
 
 	xas_lock_irq(&xas);
-
-	expected_count += hpage_nr_pages(page) + page_has_private(page);
 	if (page_count(page) != expected_count || xas_load(&xas) != page) {
 		xas_unlock_irq(&xas);
 		return -EAGAIN;

Signature · 1 line

2.16.4

Mel Gorman

Acked-by on v1 1/6

2018-12-13 13:05 UTC #2 lore raw

Re: [PATCH 1/6] mm: migration: Factor out code to compute expected number of page references

Quoted 6 lines · Jan Kara

Factor out function to compute number of expected page references in migrate_page_move_mapping(). Note that we move hpage_nr_pages() and page_has_private() checks from under xas_lock_irq() however this is safe since we hold page lock.
Signed-off-by Jan Kara <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Mel Gorman

2018-12-14 15:10 UTC #3 lore raw

Re: [PATCH 1/6] mm: migration: Factor out code to compute expected number of page references

In reply to Jan Kara

Quoted 24 lines · Jan Kara

Factor out function to compute number of expected page references in migrate_page_move_mapping(). Note that we move hpage_nr_pages() and page_has_private() checks from under xas_lock_irq() however this is safe since we hold page lock.
Signed-off-by Jan Kara <[email protected]>
---
 mm/migrate.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)
mm/migrate.c +6 -0
index f7e4bfdc13b7..789c7bc90a0c 100644
@@ -428,6 +428,22 @@ static inline bool buffer_migrate_lock_buffers(struct buffer_head *head,
 }
 #endif /* CONFIG_BLOCK */
 
+static int expected_page_refs(struct page *page)
+{
+	int expected_count = 1;
+
+	/*
+	 * Device public or private pages have an extra refcount as they are

mm/migrate.c static inline bool buffer_migrate_lock_buffers(struct buffer_head *head, · line 446

Quoted 10 lines

mm/migrate.c +10 -0

+	 * ZONE_DEVICE pages.
+	 */
+	expected_count += is_device_private_page(page);
+	expected_count += is_device_public_page(page);
+	if (page->mapping)
+		expected_count += hpage_nr_pages(page) + page_has_private(page);
+
+	return expected_count;
+}
+

I noticed during testing that THP allocation success rates under the mmtests configuration global-dhp__workload_thpscale-madvhugepage-xfs were terrible with massive latencies introduced somewhere in the series. I haven't tried chasing it down as it's relatively late but this block looked odd and I missed it the first time.

This page->mapping test is relevant for the "Anonymous page without mapping" check but I think it's wrong. An anonymous page without mapping doesn't have a NULL mapping, it sets PAGE_MAPPING_ANON and the field can be special in other ways. I think you meant to use page_mapping(page) here, not page->mapping?

Signature · 2 lines

Mel Gorman
SUSE Labs

Jan Kara

author reviewer

2018-12-14 15:53 UTC #4 lore raw

Re: [PATCH 1/6] mm: migration: Factor out code to compute expected number of page references

Quoted 33 lines · Mel Gorman

Quoted 32 lines · Jan Kara

Signed-off-by Jan Kara <[email protected]>

---
 mm/migrate.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

mm/migrate.c +14 -0

index f7e4bfdc13b7..789c7bc90a0c 100644
@@ -428,6 +428,22 @@ static inline bool buffer_migrate_lock_buffers(struct buffer_head *head,
 }
 #endif /* CONFIG_BLOCK */
 
+static int expected_page_refs(struct page *page)
+{
+	int expected_count = 1;
+
+	/*
+	 * Device public or private pages have an extra refcount as they are
+	 * ZONE_DEVICE pages.
+	 */
+	expected_count += is_device_private_page(page);
+	expected_count += is_device_public_page(page);
+	if (page->mapping)
+		expected_count += hpage_nr_pages(page) + page_has_private(page);
+
+	return expected_count;

Quoted 8 lines

mm/migrate.c static inline bool buffer_migrate_lock_buffers(struct buffer_head *head, · line 446
Quoted 2 lines
mm/migrate.c +2 -0
+}
+
I noticed during testing that THP allocation success rates under the mmtests configuration global-dhp__workload_thpscale-madvhugepage-xfs were terrible with massive latencies introduced somewhere in the series. I haven't tried chasing it down as it's relatively late but this block looked odd and I missed it the first time.

Interesting. I've run config-global-dhp__workload_thpscale and that didn't show anything strange. But the numbers were fluctuating a lot both with and without my patches applied. I'll have a look if I can reproduce this sometime next week and look what could be causing the delays.

Quoted 5 lines

This page->mapping test is relevant for the "Anonymous page without mapping" check but I think it's wrong. An anonymous page without mapping doesn't have a NULL mapping, it sets PAGE_MAPPING_ANON and the field can be special in other ways. I think you meant to use page_mapping(page) here, not page->mapping?

Yes, that's a bug. It should have been page_mapping(page). Thanks for catching this.

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Mel Gorman

2018-12-14 16:24 UTC #5 lore raw

Re: [PATCH 1/6] mm: migration: Factor out code to compute expected number of page references

Quoted 11 lines · Jan Kara

Quoted 5 lines
I noticed during testing that THP allocation success rates under the mmtests configuration global-dhp__workload_thpscale-madvhugepage-xfs were terrible with massive latencies introduced somewhere in the series. I haven't tried chasing it down as it's relatively late but this block looked odd and I missed it the first time.
Interesting. I've run config-global-dhp__workload_thpscale and that didn't show anything strange. But the numbers were fluctuating a lot both with and without my patches applied. I'll have a look if I can reproduce this sometime next week and look what could be causing the delays.

Ah, it's the difference between madvise and !madvise. The configuration you used does very little compaction as it neither wakes kswapd of kcompactd. It just falls back to base pages to limit fault latency so you wouldn't have hit the same paths of interest.

Quoted 9 lines

Quoted 5 lines
This page->mapping test is relevant for the "Anonymous page without mapping" check but I think it's wrong. An anonymous page without mapping doesn't have a NULL mapping, it sets PAGE_MAPPING_ANON and the field can be special in other ways. I think you meant to use page_mapping(page) here, not page->mapping?
Yes, that's a bug. It should have been page_mapping(page). Thanks for catching this.

My pleasure, should have spotted it the first time around :/

Signature · 2 lines

Mel Gorman
SUSE Labs

Jan Kara

author reviewer

2018-12-17 13:11 UTC #6 lore raw

Re: [PATCH 1/6] mm: migration: Factor out code to compute expected number of page references

Quoted 7 lines · Mel Gorman

Quoted 6 lines · Jan Kara
Quoted 5 lines
This page->mapping test is relevant for the "Anonymous page without mapping" check but I think it's wrong. An anonymous page without mapping doesn't have a NULL mapping, it sets PAGE_MAPPING_ANON and the field can be special in other ways. I think you meant to use page_mapping(page) here, not page->mapping?

Quoted 5 lines

Quoted 3 lines
Yes, that's a bug. It should have been page_mapping(page). Thanks for catching this.
My pleasure, should have spotted it the first time around :/

And after fixing this, I can see the success rates for global-dhp__workload_thpscale-madvhugepage to go back to the original values. So that was indeed the problem. I'll send the fixup to Andrew.

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Jan Kara

author reviewer

2018-12-11 17:21 UTC #7 lore raw

PATCH 2/6 mm: migrate: Lock buffers before migrate_page_move_mapping()

In reply to Jan Kara

Lock buffers before calling into migrate_page_move_mapping() so that that function doesn't have to know about buffers (which is somewhat unexpected anyway) and all the buffer head logic is in buffer_migrate_page().

Signed-off-by Jan Kara <[email protected]>

---
 mm/migrate.c | 39 +++++++++++++--------------------------
 1 file changed, 13 insertions(+), 26 deletions(-)

mm/migrate.c +13 ~~-26~~

index 789c7bc90a0c..d58a8ecf275e 100644
@@ -490,20 +490,6 @@ int migrate_page_move_mapping(struct address_space *mapping,
 		return -EAGAIN;
 	}
 
-	/*
-	 * In the async migration case of moving a page with buffers, lock the
-	 * buffers using trylock before the mapping is moved. If the mapping
-	 * was moved, we later failed to lock the buffers and could not move
-	 * the mapping back due to an elevated page count, we would have to
-	 * block waiting on other references to be dropped.
-	 */
-	if (mode == MIGRATE_ASYNC && head &&
-			!buffer_migrate_lock_buffers(head, mode)) {
-		page_ref_unfreeze(page, expected_count);
-		xas_unlock_irq(&xas);
-		return -EAGAIN;
-	}
-
 	/*
 	 * Now we know that no one else is looking at the page:
 	 * no turning back from here.

@@ -779,24 +765,23 @@ int buffer_migrate_page(struct address_space *mapping,
 {
 	struct buffer_head *bh, *head;
 	int rc;
+	int expected_count;
 
 	if (!page_has_buffers(page))
 		return migrate_page(mapping, newpage, page, mode);
 
-	head = page_buffers(page);
+	/* Check whether page does not have extra refs before we do more work */
+	expected_count = expected_page_refs(page);
+	if (page_count(page) != expected_count)
+		return -EAGAIN;
 
-	rc = migrate_page_move_mapping(mapping, newpage, page, head, mode, 0);
+	head = page_buffers(page);
+	if (!buffer_migrate_lock_buffers(head, mode))
+		return -EAGAIN;
 
+	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
 	if (rc != MIGRATEPAGE_SUCCESS)
-		return rc;
-
-	/*
-	 * In the async case, migrate_page_move_mapping locked the buffers
-	 * with an IRQ-safe spinlock held. In the sync case, the buffers
-	 * need to be locked now
-	 */
-	if (mode != MIGRATE_ASYNC)
-		BUG_ON(!buffer_migrate_lock_buffers(head, mode));
+		goto unlock_buffers;
 
 	ClearPagePrivate(page);
 	set_page_private(newpage, page_private(page));

@@ -818,6 +803,8 @@ int buffer_migrate_page(struct address_space *mapping,
 	else
 		migrate_page_states(newpage, page);
 
+	rc = MIGRATEPAGE_SUCCESS;
+unlock_buffers:
 	bh = head;
 	do {
 		unlock_buffer(bh);

@@ -826,7 +813,7 @@ int buffer_migrate_page(struct address_space *mapping,
 
 	} while (bh != head);
 
-	return MIGRATEPAGE_SUCCESS;
+	return rc;
 }
 EXPORT_SYMBOL(buffer_migrate_page);
 #endif

Signature · 1 line

2.16.4

Mel Gorman

Acked-by on v1 2/6

2018-12-13 14:19 UTC #8 lore raw

Re: [PATCH 2/6] mm: migrate: Lock buffers before migrate_page_move_mapping()

Quoted 6 lines · Jan Kara

Lock buffers before calling into migrate_page_move_mapping() so that that function doesn't have to know about buffers (which is somewhat unexpected anyway) and all the buffer head logic is in buffer_migrate_page().
Signed-off-by Jan Kara <[email protected]>

This is a much nicer flow.

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Jan Kara

author reviewer

2018-12-11 17:21 UTC #9 lore raw

PATCH 3/6 mm: migrate: Move migrate_page_lock_buffers()

In reply to Jan Kara

buffer_migrate_page() is the only caller of migrate_page_lock_buffers() move it close to it and also drop the now unused stub for !CONFIG_BLOCK.

Signed-off-by Jan Kara <[email protected]>

---
 mm/migrate.c | 92 +++++++++++++++++++++++++++---------------------------------
 1 file changed, 42 insertions(+), 50 deletions(-)

mm/migrate.c +42 ~~-50~~

index d58a8ecf275e..f8df1ad6e7cf 100644
@@ -378,56 +378,6 @@ void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd)
 }
 #endif
 
-#ifdef CONFIG_BLOCK
-/* Returns true if all buffers are successfully locked */
-static bool buffer_migrate_lock_buffers(struct buffer_head *head,
-							enum migrate_mode mode)
-{
-	struct buffer_head *bh = head;
-
-	/* Simple case, sync compaction */
-	if (mode != MIGRATE_ASYNC) {
-		do {
-			get_bh(bh);
-			lock_buffer(bh);
-			bh = bh->b_this_page;
-
-		} while (bh != head);
-
-		return true;
-	}
-
-	/* async case, we cannot block on lock_buffer so use trylock_buffer */
-	do {
-		get_bh(bh);
-		if (!trylock_buffer(bh)) {
-			/*
-			 * We failed to lock the buffer and cannot stall in
-			 * async migration. Release the taken locks
-			 */
-			struct buffer_head *failed_bh = bh;
-			put_bh(failed_bh);
-			bh = head;
-			while (bh != failed_bh) {
-				unlock_buffer(bh);
-				put_bh(bh);
-				bh = bh->b_this_page;
-			}
-			return false;
-		}
-
-		bh = bh->b_this_page;
-	} while (bh != head);
-	return true;
-}
-#else
-static inline bool buffer_migrate_lock_buffers(struct buffer_head *head,
-							enum migrate_mode mode)
-{
-	return true;
-}
-#endif /* CONFIG_BLOCK */
-
 static int expected_page_refs(struct page *page)
 {
 	int expected_count = 1;

@@ -755,6 +705,48 @@ int migrate_page(struct address_space *mapping,
 EXPORT_SYMBOL(migrate_page);
 
 #ifdef CONFIG_BLOCK
+/* Returns true if all buffers are successfully locked */
+static bool buffer_migrate_lock_buffers(struct buffer_head *head,
+							enum migrate_mode mode)
+{
+	struct buffer_head *bh = head;
+
+	/* Simple case, sync compaction */
+	if (mode != MIGRATE_ASYNC) {
+		do {
+			get_bh(bh);
+			lock_buffer(bh);
+			bh = bh->b_this_page;
+
+		} while (bh != head);
+
+		return true;
+	}
+
+	/* async case, we cannot block on lock_buffer so use trylock_buffer */
+	do {
+		get_bh(bh);
+		if (!trylock_buffer(bh)) {
+			/*
+			 * We failed to lock the buffer and cannot stall in
+			 * async migration. Release the taken locks
+			 */
+			struct buffer_head *failed_bh = bh;
+			put_bh(failed_bh);
+			bh = head;
+			while (bh != failed_bh) {
+				unlock_buffer(bh);
+				put_bh(bh);
+				bh = bh->b_this_page;
+			}
+			return false;
+		}
+
+		bh = bh->b_this_page;
+	} while (bh != head);
+	return true;
+}
+
 /*
  * Migration function for pages with buffers. This function can only be used
  * if the underlying filesystem guarantees that no other references to "page"

Signature · 1 line

2.16.4

Mel Gorman

Acked-by on v1 3/6

2018-12-13 14:57 UTC #10 lore raw

Re: [PATCH 3/6] mm: migrate: Move migrate_page_lock_buffers()

Quoted 4 lines · Jan Kara

buffer_migrate_page() is the only caller of migrate_page_lock_buffers() move it close to it and also drop the now unused stub for !CONFIG_BLOCK.
Signed-off-by Jan Kara <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Jan Kara

author reviewer

2018-12-11 17:21 UTC #11 lore raw

PATCH 4/6 mm: migrate: Provide buffer_migrate_page_norefs()

In reply to Jan Kara

Provide a variant of buffer_migrate_page() that also checks whether there are no unexpected references to buffer heads. This function will then be safe to use for block device pages.

Signed-off-by Jan Kara <[email protected]>

---
 include/linux/fs.h |  4 ++++
 mm/migrate.c       | 61 +++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 58 insertions(+), 7 deletions(-)

include/linux/fs.h +4 -0

index c95c0807471f..4bb1a8b65474 100644
@@ -3264,8 +3264,12 @@ extern int generic_check_addressable(unsigned, u64);
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *,
 				enum migrate_mode);
+extern int buffer_migrate_page_norefs(struct address_space *,
+				struct page *, struct page *,
+				enum migrate_mode);
 #else
 #define buffer_migrate_page NULL
+#define buffer_migrate_page_norefs NULL
 #endif
 
 extern int setattr_prepare(struct dentry *, struct iattr *);

mm/migrate.c +54 -7

80 lines not on this page · raw message

Signature · 1 line

2.16.4

Mel Gorman

Acked-by on v1 4/6

2018-12-13 15:35 UTC #12 lore raw

Re: [PATCH 4/6] mm: migrate: Provide buffer_migrate_page_norefs()

Quoted 5 lines · Jan Kara

Provide a variant of buffer_migrate_page() that also checks whether there are no unexpected references to buffer heads. This function will then be safe to use for block device pages.
Signed-off-by Jan Kara <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Andrew Morton

maintainer

2018-12-14 04:53 UTC #13 lore raw

Re: [PATCH 4/6] mm: migrate: Provide buffer_migrate_page_norefs()

In reply to Jan Kara

Quoted 7 lines · On Tue, 11 Dec 2018 18:21:41 +0100 Jan Kara <[email protected]> wrote:

Provide a variant of buffer_migrate_page() that also checks whether there are no unexpected references to buffer heads. This function will then be safe to use for block device pages.
...
+EXPORT_SYMBOL(buffer_migrate_page_norefs);

The export is presently unneeded and I don't think we expect that this will be used by anything other than fs/block_dev.c?

Jan Kara

author reviewer

2018-12-14 09:26 UTC #14 lore raw

Re: [PATCH 4/6] mm: migrate: Provide buffer_migrate_page_norefs()

Quoted 6 lines · Andrew Morton

Quoted 4 lines · On Tue, 11 Dec 2018 18:21:41 +0100 Jan Kara <[email protected]> wrote:
Provide a variant of buffer_migrate_page() that also checks whether there are no unexpected references to buffer heads. This function will then be safe to use for block device pages.

Quoted 6 lines

Quoted 3 lines
...
+EXPORT_SYMBOL(buffer_migrate_page_norefs);
The export is presently unneeded and I don't think we expect that this will be used by anything other than fs/block_dev.c?

Good point. We can always re-add the export if someone needs it. Thanks for removing it!

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Jan Kara

author reviewer

2018-12-11 17:21 UTC #15 lore raw

PATCH 5/6 blkdev: Avoid migration stalls for blkdev pages

In reply to Jan Kara

Currently, block device pages don't provide a ->migratepage callback and thus fallback_migrate_page() is used for them. This handler cannot deal with dirty pages in async mode and also with the case a buffer head is in the LRU buffer head cache (as it has elevated b_count). Thus such page can block memory offlining.

Fix the problem by using buffer_migrate_page_norefs() for migrating block device pages. That function takes care of dropping bh LRU in case migration would fail due to elevated buffer refcount to avoid stalls and can also migrate dirty pages without writing them.

Signed-off-by Jan Kara <[email protected]>

---
 fs/block_dev.c | 1 +
 1 file changed, 1 insertion(+)

fs/block_dev.c +1 -0

index a80b4f0ee7c4..de2135178e62 100644
@@ -1966,6 +1966,7 @@ static const struct address_space_operations def_blk_aops = {
 	.writepages	= blkdev_writepages,
 	.releasepage	= blkdev_releasepage,
 	.direct_IO	= blkdev_direct_IO,
+	.migratepage	= buffer_migrate_page_norefs,
 	.is_dirty_writeback = buffer_check_dirty_writeback,
 };

Signature · 1 line

2.16.4

Mel Gorman

Acked-by on v1 5/6

2018-12-13 15:35 UTC #16 lore raw

Re: [PATCH 5/6] blkdev: Avoid migration stalls for blkdev pages

Quoted 6 lines · Jan Kara

Currently, block device pages don't provide a ->migratepage callback and thus fallback_migrate_page() is used for them. This handler cannot deal with dirty pages in async mode and also with the case a buffer head is in the LRU buffer head cache (as it has elevated b_count). Thus such page can block memory offlining.

Quoted 6 lines

Fix the problem by using buffer_migrate_page_norefs() for migrating block device pages. That function takes care of dropping bh LRU in case migration would fail due to elevated buffer refcount to avoid stalls and can also migrate dirty pages without writing them.
Signed-off-by Jan Kara <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Jan Kara

author reviewer

2018-12-11 17:21 UTC #17 lore raw

PATCH 6/6 mm: migrate: Drop unused argument of migrate_page_move_mapping()

In reply to Jan Kara

All callers of migrate_page_move_mapping() now pass NULL for 'head' argument. Drop it.

Signed-off-by Jan Kara <[email protected]>

---
 fs/aio.c                | 2 +-
 fs/f2fs/data.c          | 2 +-
 fs/iomap.c              | 2 +-
 fs/ubifs/file.c         | 2 +-
 include/linux/migrate.h | 3 +--
 mm/migrate.c            | 7 +++----
 6 files changed, 8 insertions(+), 10 deletions(-)

fs/aio.c +1 -1

index 97f983592925..4f4878ebca9a 100644
@@ -408,7 +408,7 @@ static int aio_migratepage(struct address_space *mapping, struct page *new,
 	BUG_ON(PageWriteback(old));
 	get_page(new);
 
-	rc = migrate_page_move_mapping(mapping, new, old, NULL, mode, 1);
+	rc = migrate_page_move_mapping(mapping, new, old, mode, 1);
 	if (rc != MIGRATEPAGE_SUCCESS) {
 		put_page(new);
 		goto out_unlock;

fs/f2fs/data.c +1 -1

index b293cb3e27a2..008b74eff00d 100644
@@ -2738,7 +2738,7 @@ int f2fs_migrate_page(struct address_space *mapping,
 	 */
 	extra_count = (atomic_written ? 1 : 0) - page_has_private(page);
 	rc = migrate_page_move_mapping(mapping, newpage,
-				page, NULL, mode, extra_count);
+				page, mode, extra_count);
 	if (rc != MIGRATEPAGE_SUCCESS) {
 		if (atomic_written)
 			mutex_unlock(&fi->inmem_lock);

fs/iomap.c +1 -1

index 3ffb776fbebe..8df6a75d2d11 100644
@@ -550,7 +550,7 @@ iomap_migrate_page(struct address_space *mapping, struct page *newpage,
 {
 	int ret;
 
-	ret = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	ret = migrate_page_move_mapping(mapping, newpage, page, mode, 0);
 	if (ret != MIGRATEPAGE_SUCCESS)
 		return ret;

fs/ubifs/file.c +1 -1

index 1b78f2e09218..5d2ffb1a45fc 100644
@@ -1481,7 +1481,7 @@ static int ubifs_migrate_page(struct address_space *mapping,
 {
 	int rc;
 
-	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	rc = migrate_page_move_mapping(mapping, newpage, page, mode, 0);
 	if (rc != MIGRATEPAGE_SUCCESS)
 		return rc;

include/linux/migrate.h +1 -2

index f2b4abbca55e..8eeeaf946f95 100644
@@ -77,8 +77,7 @@ extern void migrate_page_copy(struct page *newpage, struct page *page);
 extern int migrate_huge_page_move_mapping(struct address_space *mapping,
 				  struct page *newpage, struct page *page);
 extern int migrate_page_move_mapping(struct address_space *mapping,
-		struct page *newpage, struct page *page,
-		struct buffer_head *head, enum migrate_mode mode,
+		struct page *newpage, struct page *page, enum migrate_mode mode,
 		int extra_count);
 #else

mm/migrate.c +3 -4

index c4075d5ec073..1e47ea88a5b3 100644
@@ -403,8 +403,7 @@ static int expected_page_refs(struct page *page)
  * 3 for pages with a mapping and PagePrivate/PagePrivate2 set.
  */
 int migrate_page_move_mapping(struct address_space *mapping,
-		struct page *newpage, struct page *page,
-		struct buffer_head *head, enum migrate_mode mode,
+		struct page *newpage, struct page *page, enum migrate_mode mode,
 		int extra_count)
 {
 	XA_STATE(xas, &mapping->i_pages, page_index(page));

@@ -691,7 +690,7 @@ int migrate_page(struct address_space *mapping,
 
 	BUG_ON(PageWriteback(page));	/* Writeback must be complete */
 
-	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	rc = migrate_page_move_mapping(mapping, newpage, page, mode, 0);
 
 	if (rc != MIGRATEPAGE_SUCCESS)
 		return rc;

@@ -794,7 +793,7 @@ static int __buffer_migrate_page(struct address_space *mapping,
 		}
 	}
 
-	rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0);
+	rc = migrate_page_move_mapping(mapping, newpage, page, mode, 0);
 	if (rc != MIGRATEPAGE_SUCCESS)
 		goto unlock_buffers;

Signature · 1 line

2.16.4

Mel Gorman

Acked-by on v1 6/6

2018-12-13 15:35 UTC #18 lore raw

Re: [PATCH 6/6] mm: migrate: Drop unused argument of migrate_page_move_mapping()

Quoted 4 lines · Jan Kara

All callers of migrate_page_move_mapping() now pass NULL for 'head' argument. Drop it.
Signed-off-by Jan Kara <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Jan Kara

author reviewer

2018-12-13 16:17 UTC #19 lore raw

Re: [PATCH 6/6] mm: migrate: Drop unused argument of migrate_page_move_mapping()

Quoted 7 lines · Mel Gorman

Quoted 4 lines · Jan Kara
All callers of migrate_page_move_mapping() now pass NULL for 'head' argument. Drop it.
Signed-off-by Jan Kara <[email protected]>
Acked-by Mel Gorman <[email protected]>

Thanks for review Mel! Andrew, can you please pick up the series? Thanks!

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Jan Kara

author reviewer

2018-12-17 13:17 UTC #20 lore raw

Re: [PATCH 6/6] mm: migrate: Drop unused argument of migrate_page_move_mapping()

Quoted 10 lines · Jan Kara

Quoted 7 lines · Mel Gorman
Quoted 4 lines · Jan Kara
All callers of migrate_page_move_mapping() now pass NULL for 'head' argument. Drop it.
Signed-off-by Jan Kara <[email protected]>
Acked-by Mel Gorman <[email protected]>
Thanks for review Mel! Andrew, can you please pick up the series? Thanks!

Andrew, Mel has spotted a bug in patch 1/6 or this series. Can you please add the attached fixup to the patch 1? Thanks!

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

mm/migrate.c +1 -1

index 789c7bc90a0c..816ce10e908f 100644
@@ -438,7 +438,7 @@ static int expected_page_refs(struct page *page)
 	 */
 	expected_count += is_device_private_page(page);
 	expected_count += is_device_public_page(page);
-	if (page->mapping)
+	if (page_mapping(page))
 		expected_count += hpage_nr_pages(page) + page_has_private(page);
 
 	return expected_count;