Re: [External] Re: [PATCH] arm64: mm: hugetlb: add support for free vmemmap pages of HugeTLB

[Mike Kravetz]

On 5/19/21 9:21 AM, Muchun Song wrote:
> On Wed, May 19, 2021 at 11:22 PM Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
>>
>> On Wed, May 19, 2021 at 10:43 PM Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
>>>
>>> On Wed, May 19, 2021 at 8:35 PM Anshuman Khandual
>>> <anshuman.khandual@xxxxxxx> wrote:
>>>> How does this interact with HugeTLB migration as such which might iterate
>>>> over individual constituent struct pages (overriding the same struct page
>>>> for all tail pages when this feature is enabled). A simple test involving
>>>> madvise(ptr, size, MADV_SOFT_OFFLINE) fails on various HugeTLB page sizes,
>>>> with this patch applied. Although I have not debugged this any further.
>>>
>>> It is weird. Actually, I didn't change the behaviour of the page migration.
>>> This feature is default off. If you want to enable this feature, you can pass
>>> "hugetlb_free_vmemmap=on" to the boot cmdline. Do you mean that the
>>> success rate of page migration will decrease when you enable this feature?
>>> The rate will increase if disbale. Right?
>>
>> I have done the test and found the issue. Because unmap_and_move_huge_page
>> always returns -EBUSY. I will look into this issue in depth. Thanks for your
>> report.
>>
>> The return point is as below:
>>
>> if (page_private(hpage) && !page_mapping(hpage)) {
>>         rc = -EBUSY;
>>         goto out_unlock;
>> }
> 
> I know the issue. It was caused by commit d6995da31122 ("hugetlb:
> use page.private for hugetlb specific page flags"). The below patch
> can fix this issue.
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index e7a173da74ec..43419c4bb097 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1290,7 +1290,7 @@ static int unmap_and_move_huge_page(new_page_t
> get_new_page,
>          * page_mapping() set, hugetlbfs specific move page routine will not
>          * be called and we could leak usage counts for subpools.
>          */
> -       if (page_private(hpage) && !page_mapping(hpage)) {
> +       if (hugetlb_page_subpool(hpage) && !page_mapping(hpage)) {
>                 rc = -EBUSY;
>                 goto out_unlock;
>         }
> 

Thank you Muchun!  That was my bad commit.
-- 
Mike Kravetz