| Message ID | 20240326150031.569387-1-zi.yan@sent.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v6] mm/migrate: split source folio if it is on deferred split list | expand |
On 2024/3/26 23:00, Zi Yan wrote: > From: Zi Yan <ziy@nvidia.com> > > If the source folio is on deferred split list, it is likely some subpages > are not used. Split it before migration to avoid migrating unused subpages. > > Commit 616b8371539a6 ("mm: thp: enable thp migration in generic path") > did not check if a THP is on deferred split list before migration, thus, > the destination THP is never put on deferred split list even if the source > THP might be. The opportunity of reclaiming free pages in a partially > mapped THP during deferred list scanning is lost, but no other harmful > consequence is present[1]. > > From v5: > 1. Fixed an error in migrate_misplaced_folio() reported by Baolin Wang[3]. > > From v4: > 1. Simplify _deferred_list check without locking and do not count as > migration failures. (per Matthew Wilcox) > > From v3: > 1. Guarded deferred list code behind CONFIG_TRANSPARENT_HUGEPAGE to avoid > compilation error (per SeongJae Park). > > From v2: > 1. Split the source folio instead of migrating it (per Matthew Wilcox)[2]. > > From v1: > 1. Used dst to get correct deferred split list after migration > (per Ryan Roberts). > > [1]: https://lore.kernel.org/linux-mm/03CE3A00-917C-48CC-8E1C-6A98713C817C@nvidia.com/ > [2]: https://lore.kernel.org/linux-mm/Ze_P6xagdTbcu1Kz@casper.infradead.org/ > [3]: https://lore.kernel.org/linux-mm/df9a644c-a007-46ac-98e3-61d4014fcfff@linux.alibaba.com/ > > Fixes: 616b8371539a ("mm: thp: enable thp migration in generic path") > Signed-off-by: Zi Yan <ziy@nvidia.com> LGTM. Feel free to add: Tested-by: Baolin Wang <baolin.wang@linux.alibaba.com> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> > --- > mm/migrate.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 1dbe5bd927de..a31aa75d223d 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1652,6 +1652,29 @@ static int migrate_pages_batch(struct list_head *from, > > cond_resched(); > > + /* > + * The rare folio on the deferred split list should > + * be split now. It should not count as a failure. > + * Only check it without removing it from the list. > + * Since the folio can be on deferred_split_scan() > + * local list and removing it can cause the local list > + * corruption. Folio split process below can handle it > + * with the help of folio_ref_freeze(). > + * > + * nr_pages > 2 is needed to avoid checking order-1 > + * page cache folios. They exist, in contrast to > + * non-existent order-1 anonymous folios, and do not > + * use _deferred_list. > + */ > + if (nr_pages > 2 && > + !list_empty(&folio->_deferred_list)) { > + if (try_split_folio(folio, split_folios) == 0) { > + stats->nr_thp_split += is_thp; > + stats->nr_split++; > + continue; > + } > + } > + > /* > * Large folio migration might be unsupported or > * the allocation might be failed so we should retry
On 26.03.24 16:00, Zi Yan wrote: > From: Zi Yan <ziy@nvidia.com> > > If the source folio is on deferred split list, it is likely some subpages > are not used. Split it before migration to avoid migrating unused subpages. > > Commit 616b8371539a6 ("mm: thp: enable thp migration in generic path") > did not check if a THP is on deferred split list before migration, thus, > the destination THP is never put on deferred split list even if the source > THP might be. The opportunity of reclaiming free pages in a partially > mapped THP during deferred list scanning is lost, but no other harmful > consequence is present[1]. > > From v5: > 1. Fixed an error in migrate_misplaced_folio() reported by Baolin Wang[3]. > > From v4: > 1. Simplify _deferred_list check without locking and do not count as > migration failures. (per Matthew Wilcox) > > From v3: > 1. Guarded deferred list code behind CONFIG_TRANSPARENT_HUGEPAGE to avoid > compilation error (per SeongJae Park). > > From v2: > 1. Split the source folio instead of migrating it (per Matthew Wilcox)[2]. > > From v1: > 1. Used dst to get correct deferred split list after migration > (per Ryan Roberts). > > [1]: https://lore.kernel.org/linux-mm/03CE3A00-917C-48CC-8E1C-6A98713C817C@nvidia.com/ > [2]: https://lore.kernel.org/linux-mm/Ze_P6xagdTbcu1Kz@casper.infradead.org/ > [3]: https://lore.kernel.org/linux-mm/df9a644c-a007-46ac-98e3-61d4014fcfff@linux.alibaba.com/ > > Fixes: 616b8371539a ("mm: thp: enable thp migration in generic path") > Signed-off-by: Zi Yan <ziy@nvidia.com> > --- > mm/migrate.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 1dbe5bd927de..a31aa75d223d 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1652,6 +1652,29 @@ static int migrate_pages_batch(struct list_head *from, > > cond_resched(); > > + /* > + * The rare folio on the deferred split list should > + * be split now. It should not count as a failure. > + * Only check it without removing it from the list. > + * Since the folio can be on deferred_split_scan() > + * local list and removing it can cause the local list > + * corruption. Folio split process below can handle it > + * with the help of folio_ref_freeze(). > + * > + * nr_pages > 2 is needed to avoid checking order-1 > + * page cache folios. They exist, in contrast to > + * non-existent order-1 anonymous folios, and do not > + * use _deferred_list. > + */ > + if (nr_pages > 2 && > + !list_empty(&folio->_deferred_list)) { > + if (try_split_folio(folio, split_folios) == 0) { > + stats->nr_thp_split += is_thp; > + stats->nr_split++; > + continue; > + } > + } > + > /* > * Large folio migration might be unsupported or > * the allocation might be failed so we should retry Acked-by: David Hildenbrand <david@redhat.com>
diff --git a/mm/migrate.c b/mm/migrate.c index 1dbe5bd927de..a31aa75d223d 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1652,6 +1652,29 @@ static int migrate_pages_batch(struct list_head *from, cond_resched(); + /* + * The rare folio on the deferred split list should + * be split now. It should not count as a failure. + * Only check it without removing it from the list. + * Since the folio can be on deferred_split_scan() + * local list and removing it can cause the local list + * corruption. Folio split process below can handle it + * with the help of folio_ref_freeze(). + * + * nr_pages > 2 is needed to avoid checking order-1 + * page cache folios. They exist, in contrast to + * non-existent order-1 anonymous folios, and do not + * use _deferred_list. + */ + if (nr_pages > 2 && + !list_empty(&folio->_deferred_list)) { + if (try_split_folio(folio, split_folios) == 0) { + stats->nr_thp_split += is_thp; + stats->nr_split++; + continue; + } + } + /* * Large folio migration might be unsupported or * the allocation might be failed so we should retry