On 1 Nov 2021, at 2:54, Baolin Wang wrote:
On 2021/10/29 23:43, Zi Yan wrote:
On 29 Oct 2021, at 3:42, Baolin Wang wrote:
Now hugetlb migration is also available for some scenarios, such as
soft offling or memory compaction. So we should correct the migration
hugetlb migration is available at the time if (PageHuge(page)) branch
is added. I am not sure what is new here.
No new things actually, sorry for confusing and will update the commit message in next version.
stats for hugetlb with using compound_nr() instead of thp_nr_pages()
to get the number of pages.
nr_failed records the number of pages, not subpages. It is returned to
I also think nr_failed should record the number of pages, not the number of hugetlb, if I understand you correctly.
user space when move_pages() syscall is used. After your change,
if users try to migrate a list of pages including THPs and/or hugetlb
pages and some of THPs and/or hugetlb fail to migrate, move_pages()
will return a number larger than the number of pages the users tried
OK, thanks for pointing out the issue.
But before my patch, we've already returned the number of pages successed or failed for THP migration, instead of the number of THP. That means if we just move only 1 page by
Ah, you are right.
move_pages() and if this page is 2M THP, so move_pages() will return 512 if failed to migrate, which is larger than the page count specified from user.
if (err > 0)
err += nr_pages - i - 1;
I am not sure this is right for user-space.
On the other hand, the stats of PGMIGRATE_SUCCESS/PGMIGRATE_FAIL should stand for the number of pages, instead of the number of hugetlb. Also for hugetlb migration when memory compaction, we've already counted the number of pages for a hugetlb into cc->nr_migratepages, if the hugetlb migration failed, the trace stat of compaction will be confusing if we return the number of hugetlb.
trace_mm_compaction_migratepages(cc->nr_migratepages, err, &cc->migratepages);
So I think the stats of hugetlb migration should be consistent with THP.
It makes sense to me.
to migrate. I am not sure this is the change we want. Or at least,
the comment of migrate_pages() and the manpage of move_pages() need
to be changed and linux-api mailing list should be cc’d.
I don't think we should update the comments of migrate_pages(), "Returns the number of pages that were not migrated" makes sense to me if I understand correctly.
For the manpage of move_pages(), as you said, the the returned non-migrate page numbers can be larger than the numbers specified from user if failed to migrate a THP or a hugetlb. I am not sure if we should change the manpage, since the THP already did, but I can send a patch to update the manpage if you think this is still necessary. Thanks.
I am not sure changing manpage would help the users of move_pages() after
think about it again, since users might not know all the THP and/or hugetlb
information when they call move_pages() and they just pass a list of N pages. >
I just wonder if we could fix the rc value of migrate_pages to return
the number of {base page, THP, hugetlb} instead, so that move_pages()
can get its return value right.
