[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6bb5b0ae-6de3-4df8-a8c1-07d4c6f8c275@redhat.com>
Date: Sat, 5 Jul 2025 06:17:00 +0300
From: Mika Penttilä <mpenttil@...hat.com>
To: Balbir Singh <balbirs@...dia.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
>>>> +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.
--Mika
Powered by blists - more mailing lists