Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx> writes: > When THP migration, if THPs are split and all subpages are migrated successfully > , the migrate_pages() will still return the number of THP that were not migrated. > That will confuse the callers of migrate_pages(), for example, which will make > the longterm pinning failed though all pages are migrated successfully. > > Thus we should return 0 to indicate all pages are migrated in this case. > > Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx> > --- > Changes from v1: > - Fix the return value of migrate_pages() instead of fixing the > callers' validation. > --- > mm/migrate.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 8e5eb6e..1da0dbc 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1582,6 +1582,13 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, > */ > list_splice(&ret_pages, from); > > + /* > + * Return 0 in case all subpages of fail-to-migrate THPs are > + * migrated successfully. > + */ > + if (nr_thp_split && list_empty(from)) > + rc = 0; Why do you need to check nr_thp_split? Wouldn't list_empty(from) == True imply success? And if it doesn't imply success wouldn't it be possible to end up with nr_thp_split && list_empty(from) whilst still having pages that failed to migrate? The list management and return code logic from unmap_and_move() has gotten pretty difficult to follow and could do with some rework IMHO. > count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded); > count_vm_events(PGMIGRATE_FAIL, nr_failed_pages); > count_vm_events(THP_MIGRATION_SUCCESS, nr_thp_succeeded);
