Compaction, NUMA page movement, THP collapse/split, and memory failure
do isolate unevictable pages from their "LRU", losing the record of
mlock_count in doing so (isolators are likely to use page->lru for their
own private lists, so mlock_count has to be presumed lost).
That's unfortunate, and we should put in some work to correct that: one
can imagine a function to build up the mlock_count again - but it would
require i_mmap_rwsem for read, so be careful where it's called. Or
page_referenced_one() and try_to_unmap_one() might do that extra work.
But one place that can very easily be improved is page migration's
__unmap_and_move(): a small adjustment to where the successful new page
is put back on LRU, and its mlock_count (if any) is built back up by
remove_migration_ptes().
Signed-off-by: Hugh Dickins <[email protected]>
---
mm/migrate.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index 7c4223ce2500..f4bcf1541b62 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1032,6 +1032,21 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
if (!page_mapped(page))
rc = move_to_new_page(newpage, page, mode);
+ /*
+ * When successful, push newpage to LRU immediately: so that if it
+ * turns out to be an mlocked page, remove_migration_ptes() will
+ * automatically build up the correct newpage->mlock_count for it.
+ *
+ * We would like to do something similar for the old page, when
+ * unsuccessful, and other cases when a page has been temporarily
+ * isolated from the unevictable LRU: but this case is the easiest.
+ */
+ if (rc == MIGRATEPAGE_SUCCESS) {
+ lru_cache_add(newpage);
+ if (page_was_mapped)
+ lru_add_drain();
+ }
+
if (page_was_mapped)
remove_migration_ptes(page,
rc == MIGRATEPAGE_SUCCESS ? newpage : page, false);
@@ -1045,20 +1060,12 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
unlock_page(page);
out:
/*
- * If migration is successful, decrease refcount of the newpage
+ * If migration is successful, decrease refcount of the newpage,
* which will not free the page because new page owner increased
- * refcounter. As well, if it is LRU page, add the page to LRU
- * list in here. Use the old state of the isolated source page to
- * determine if we migrated a LRU page. newpage was already unlocked
- * and possibly modified by its owner - don't rely on the page
- * state.
+ * refcounter.
*/
- if (rc == MIGRATEPAGE_SUCCESS) {
- if (unlikely(!is_lru))
- put_page(newpage);
- else
- putback_lru_page(newpage);
- }
+ if (rc == MIGRATEPAGE_SUCCESS)
+ put_page(newpage);
return rc;
}
--
2.34.1
On 2/6/22 22:43, Hugh Dickins wrote:
> Compaction, NUMA page movement, THP collapse/split, and memory failure
> do isolate unevictable pages from their "LRU", losing the record of
> mlock_count in doing so (isolators are likely to use page->lru for their
> own private lists, so mlock_count has to be presumed lost).
>
> That's unfortunate, and we should put in some work to correct that: one
> can imagine a function to build up the mlock_count again - but it would
> require i_mmap_rwsem for read, so be careful where it's called. Or
> page_referenced_one() and try_to_unmap_one() might do that extra work.
>
> But one place that can very easily be improved is page migration's
> __unmap_and_move(): a small adjustment to where the successful new page
> is put back on LRU, and its mlock_count (if any) is built back up by
> remove_migration_ptes().
>
> Signed-off-by: Hugh Dickins <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
> ---
> mm/migrate.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 7c4223ce2500..f4bcf1541b62 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1032,6 +1032,21 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
> if (!page_mapped(page))
> rc = move_to_new_page(newpage, page, mode);
>
> + /*
> + * When successful, push newpage to LRU immediately: so that if it
> + * turns out to be an mlocked page, remove_migration_ptes() will
> + * automatically build up the correct newpage->mlock_count for it.
> + *
> + * We would like to do something similar for the old page, when
> + * unsuccessful, and other cases when a page has been temporarily
> + * isolated from the unevictable LRU: but this case is the easiest.
> + */
> + if (rc == MIGRATEPAGE_SUCCESS) {
> + lru_cache_add(newpage);
> + if (page_was_mapped)
> + lru_add_drain();
> + }
> +
> if (page_was_mapped)
> remove_migration_ptes(page,
> rc == MIGRATEPAGE_SUCCESS ? newpage : page, false);
> @@ -1045,20 +1060,12 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
> unlock_page(page);
> out:
> /*
> - * If migration is successful, decrease refcount of the newpage
> + * If migration is successful, decrease refcount of the newpage,
> * which will not free the page because new page owner increased
> - * refcounter. As well, if it is LRU page, add the page to LRU
> - * list in here. Use the old state of the isolated source page to
> - * determine if we migrated a LRU page. newpage was already unlocked
> - * and possibly modified by its owner - don't rely on the page
> - * state.
> + * refcounter.
> */
> - if (rc == MIGRATEPAGE_SUCCESS) {
> - if (unlikely(!is_lru))
> - put_page(newpage);
> - else
> - putback_lru_page(newpage);
> - }
> + if (rc == MIGRATEPAGE_SUCCESS)
> + put_page(newpage);
>
> return rc;
> }