linux-kernel - Re: [v5 05/15] mm/migrate_device: handle partially mapped folios during collection

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 for Android: free password hash cracker in your pocket

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

Message-ID: <21e6367f-7082-4b9e-8562-3d498abb629f@meta.com>
Date: Fri, 19 Sep 2025 07:33:06 -0400
From: Chris Mason <clm@...a.com>
To: Balbir Singh <balbirs@...dia.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        dri-devel@...ts.freedesktop.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
        Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
        Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
        Ying Huang <ying.huang@...ux.alibaba.com>,
        Alistair Popple <apopple@...dia.com>,
        Oscar Salvador <osalvador@...e.de>,
        Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Nico Pache <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
        Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
        Lyude Paul <lyude@...hat.com>, Danilo Krummrich <dakr@...nel.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Ralph Campbell <rcampbell@...dia.com>,
        Mika Penttilä <mpenttil@...hat.com>,
        Matthew Brost <matthew.brost@...el.com>,
        Francois Dugast <francois.dugast@...el.com>
Subject: Re: [v5 05/15] mm/migrate_device: handle partially mapped folios
 during collection

On 9/19/25 4:36 AM, Balbir Singh wrote:
> On 9/19/25 02:42, Chris Mason wrote:
>> On Mon,  8 Sep 2025 10:04:38 +1000 Balbir Singh <balbirs@...dia.com> wrote:
>>
>>> Extend migrate_vma_collect_pmd() to handle partially mapped large
>>> folios that require splitting before migration can proceed.
>>>
>>> During PTE walk in the collection phase, if a large folio is only
>>> partially mapped in the migration range, it must be split to ensure
>>> the folio is correctly migrated.
>>>
>>> Signed-off-by: Balbir Singh <balbirs@...dia.com>
>>> ---
>>>  mm/migrate_device.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 94 insertions(+)
>>>
>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>> index abd9f6850db6..f45ef182287d 100644
>>> --- a/mm/migrate_device.c
>>> +++ b/mm/migrate_device.c
>>> @@ -54,6 +54,53 @@ static int migrate_vma_collect_hole(unsigned long start,
>>>  	return 0;
>>>  }
>>>  
>>> +/**
>>> + * migrate_vma_split_folio() - Helper function to split a THP folio
>>> + * @folio: the folio to split
>>> + * @fault_page: struct page associated with the fault if any
>>> + *
>>> + * Returns 0 on success
>>> + */
>>> +static int migrate_vma_split_folio(struct folio *folio,
>>> +				   struct page *fault_page)
>>> +{
>>> +	int ret;
>>> +	struct folio *fault_folio = fault_page ? page_folio(fault_page) : NULL;
>>> +	struct folio *new_fault_folio = NULL;
>>> +
>>> +	if (folio != fault_folio) {
>>> +		folio_get(folio);
>>> +		folio_lock(folio);
>>> +	}
>>
>> Can fault_folio ever be non-null and different from folio? Apologies for
>> not knowing the lock ordering rules but this jumps out.
>>
> 
> Yes, migration can occur in fault context or be driver driven
> 
>>> +
>>> +	ret = split_folio(folio);
>>> +	if (ret) {
>>> +		if (folio != fault_folio) {
>>> +			folio_unlock(folio);
>>> +			folio_put(folio);
>>> +		}
>>> +		return ret;
>>> +	}
>>> +
>>> +	new_fault_folio = fault_page ? page_folio(fault_page) : NULL;
>>> +
>>> +	/*
>>> +	 * Ensure the lock is held on the correct
>>> +	 * folio after the split
>>> +	 */
>>> +	if (!new_fault_folio) {
>>> +		folio_unlock(folio);
>>> +		folio_put(folio);
>>> +	} else if (folio != new_fault_folio) {
>>> +		folio_get(new_fault_folio);
>>> +		folio_lock(new_fault_folio);
>>> +		folio_unlock(folio);
>>> +		folio_put(folio);
>>> +	}
>>
>> Same question here, do we need trylocks?
>>
> 
> Since we had the folio lock before, the assumption is that we can
> still grab the lock after split and it's OK to wait, since this
> is not a hot-path.

I think the lock ordering rules either let us take two folios without
trylock or they don't...holding the lock in the past shouldn't change
things?  The same holds true above, two locks + no ordering, one or both
of these locking sites should deadlock.

But obviously I'm a tourist here, and I need to refresh the review
queue, so I'll move on ;)  Thanks for taking a look at it.

-chris