linux-kernel - Re: [v1 resend 08/12] mm/thp: add split during migration support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c08156c8-bb0d-4d8f-b411-92c82e1dcfba@nvidia.com>
Date: Tue, 8 Jul 2025 17:37:25 +1000
From: Balbir Singh <balbirs@...dia.com>
To: Mika Penttilä <mpenttil@...hat.com>, linux-mm@...ck.org
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
 Karol Herbst <kherbst@...hat.com>, Lyude Paul <lyude@...hat.com>,
 Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Jérôme Glisse
 <jglisse@...hat.com>, Shuah Khan <shuah@...nel.org>,
 David Hildenbrand <david@...hat.com>, Barry Song <baohua@...nel.org>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Ryan Roberts <ryan.roberts@....com>, Matthew Wilcox <willy@...radead.org>,
 Peter Xu <peterx@...hat.com>, Zi Yan <ziy@...dia.com>,
 Kefeng Wang <wangkefeng.wang@...wei.com>, Jane Chu <jane.chu@...cle.com>,
 Alistair Popple <apopple@...dia.com>, Donet Tom <donettom@...ux.ibm.com>
Subject: Re: [v1 resend 08/12] mm/thp: add split during migration support

On 7/7/25 13:29, Mika Penttilä wrote:
> On 7/7/25 05:35, Balbir Singh wrote:
>> On 7/5/25 13:17, Mika Penttilä wrote:
>>>>>>> +static void migrate_vma_split_pages(struct migrate_vma *migrate,
>>>>>>> +					unsigned long idx, unsigned long addr,
>>>>>>> +					struct folio *folio)
>>>>>>> +{
>>>>>>> +	unsigned long i;
>>>>>>> +	unsigned long pfn;
>>>>>>> +	unsigned long flags;
>>>>>>> +
>>>>>>> +	folio_get(folio);
>>>>>>> +	split_huge_pmd_address(migrate->vma, addr, true);
>>>>>>> +	__split_huge_page_to_list_to_order(folio_page(folio, 0), NULL, 0, true);
>>>>>> We already have reference to folio, why is folio_get() needed ?
>>>>>>
>>>>>> Splitting the page splits pmd for anon folios, why is there split_huge_pmd_address() ?
>>>>> Oh I see 
>>>>> +	if (!isolated)
>>>>> +		unmap_folio(folio);
>>>>>
>>>>> which explains the explicit split_huge_pmd_address(migrate->vma, addr, true);
>>>>>
>>>>> Still, why the folio_get(folio);?
>>>>>  
>>>>>
>>>> That is for split_huge_pmd_address, when called with freeze=true, it drops the
>>>> ref count on the page
>>>>
>>>> 	if (freeze)
>>>> 		put_page(page);
>>>>
>>>> Balbir
>>>>
>>> yeah I guess you could have used the pmd_migration path in __split_huge_pmd_locked, and not use freeze because you have installed the migration pmd entry already.
>>> Which brings to a bigger concern, that you do need the freeze semantics, like clear PageAnonExclusive (which may fail). I think you did not get this part
>>> right in the 3/12 patch. And in this patch, you can't assume the split succeeds, which would mean you can't migrate the range at all.
>>> Doing the split this late is quite problematic all in all.
>>>
>> Clearing PageAnonExclusive will *not* fail for device private pages from what I can see in __folio_try_share_anon_rmap().
>> Doing the split late is a requirement due to the nature of the three stage migration operation, the other side
>> might fail to allocate THP sized pages and so the code needs to deal with it
>>
>> Balbir Singh
> 
> Yes seems clearing PageAnonExclusive doesn't fail for device private pages in the end, 
> but the 3/12 patch doesn't even try to clear PageAnonExclusive with your changes afaics,
> which is a separate issue.
> 
> And __split_huge_page_to_list_to_order() (return value is not checked) can fail for out of memory.
> So think you can not just assume split just works. If late split is a requirement (which I can understand is),
> you should be prepared to rollback somehow the operation.
> 

I'll add a check, rolling back is just setting up the entries to not be migrated

Thanks,
Balbir Singh