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

From: Mike Kravetz <mike.kravetz@oracle.com>
To: Muchun Song <songmuchun@bytedance.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	"Bodeddula, Balasubramaniam" <bodeddub@amazon.com>,
	Oscar Salvador <osalvador@suse.de>,
	David Rientjes <rientjes@google.com>,
	linux-arm-kernel@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Xiongchun duan <duanxiongchun@bytedance.com>,
	fam.zheng@bytedance.com, zhengqi.arch@bytedance.com
Subject: Re: [External] Re: [PATCH] arm64: mm: hugetlb: add support for free vmemmap pages of HugeTLB
Date: Wed, 19 May 2021 15:44:50 -0700	[thread overview]
Message-ID: <7c39926f-3e8c-afda-7a68-81d2a65c580c@oracle.com> (raw)
In-Reply-To: <CAMZfGtUOmFQCXYKq3V=bJs1Ve6kgnT_o_LhsY0gitOY+D7T10A@mail.gmail.com>

On 5/19/21 9:21 AM, Muchun Song wrote:
> On Wed, May 19, 2021 at 11:22 PM Muchun Song <songmuchun@bytedance.com> wrote:
>>
>> On Wed, May 19, 2021 at 10:43 PM Muchun Song <songmuchun@bytedance.com> wrote:
>>>
>>> On Wed, May 19, 2021 at 8:35 PM Anshuman Khandual
>>> <anshuman.khandual@arm.com> 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