Re: [RFC 04/11] mm/migrate_device: THP migration of zone device pages

[Balbir Singh <balbirs@nvidia.com>]

On 3/6/25 20:24, Mika Penttilä wrote:
> Hi,
> 
> On 3/6/25 06:42, Balbir Singh wrote:
> ...
> 
>>  
>>  			/*
>>  			 * The only time there is no vma is when called from
>> @@ -728,15 +1000,47 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>>  					migrate->pgmap_owner);
>>  				mmu_notifier_invalidate_range_start(&range);
>>  			}
>> -			migrate_vma_insert_page(migrate, addr, newpage,
>> +
>> +			if ((src_pfns[i] & MIGRATE_PFN_COMPOUND) &&
>> +				(!(dst_pfns[i] & MIGRATE_PFN_COMPOUND))) {
>> +				nr = HPAGE_PMD_NR;
>> +				src_pfns[i] &= ~MIGRATE_PFN_COMPOUND;
>> +				src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
>> +				goto next;
>> +			}
>> +
>> +			migrate_vma_insert_page(migrate, addr, &dst_pfns[i],
>>  						&src_pfns[i]);
>> -			continue;
>> +			goto next;
>>  		}
>>  
>>  		newfolio = page_folio(newpage);
>>  		folio = page_folio(page);
>>  		mapping = folio_mapping(folio);
>>  
>> +		/*
>> +		 * If THP migration is enabled, check if both src and dst
>> +		 * can migrate large pages
>> +		 */
>> +		if (thp_migration_supported()) {
>> +			if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
>> +				(src_pfns[i] & MIGRATE_PFN_COMPOUND) &&
>> +				!(dst_pfns[i] & MIGRATE_PFN_COMPOUND)) {
>> +
>> +				if (!migrate) {
>> +					src_pfns[i] &= ~(MIGRATE_PFN_MIGRATE |
>> +							 MIGRATE_PFN_COMPOUND);
>> +					goto next;
>> +				}
>> +				src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> 
> This looks strange as is but patch 08 changes this to split and then
> migrate.

Yes, in the series at patch 4/11 split migration is not supported. That support
needs additional changes to be fully supported and comes in later.

> 
> 
>> +			} else if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
>> +				(dst_pfns[i] & MIGRATE_PFN_COMPOUND) &&
>> +				!(src_pfns[i] & MIGRATE_PFN_COMPOUND)) {
>> +				src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> 
> Should there be goto next; or similar here also, we are not migrating
> this src?
> 

Yes, will do, but generally it just falls through, but the additional checks below
are not needed.

Thanks for the review!
Balbir