On 5/12/26 14:43, David Hildenbrand (Arm) wrote:
>> +
>> +/**
>> + * migrate_hmm_range_setup() - prepare to migrate a range of memory
>> + * @range: contains pointer to struct migrate_vma to be set up.
>> + *
>> + * When collecting has been done with hmm_range_fault(), this
>> + * should be called next, and completes range->migrate by
>> + * populating migrate->src[] and migrate->dst[]
>> + * using range->hmm_pfns[].
>> + * Also, migrate->cpages and migrate->npages get initialized.
>> + * After migrate_hmm_range_setup(), range->migrate is good
>> + * for the rest of the migrate_vma_* flow.
>> + */
>> +void migrate_hmm_range_setup(struct hmm_range *range)
>> +{
>> + struct migrate_vma *migrate = range->migrate;
>> +
>> + if (!migrate)
>> + return;
>> +
>> + migrate->npages = (migrate->end - migrate->start) >> PAGE_SHIFT;
>> + migrate->cpages = 0;
>> +
>> + for (unsigned long i = 0; i < migrate->npages; i++) {
>> + unsigned long pfn = range->hmm_pfns[i];
>> +
>> + /*
>> + * We are only interested in entries to be
>> + * migrated.
>> + */
>> + if (!(pfn & HMM_PFN_MIGRATE)) {
>> + migrate->src[i] = 0;
>> + migrate->dst[i] = 0;
>> + continue;
>> + }
>> +
>> + migrate->cpages++;
>> +
>> + /* HMM_PFN_MIGRATE without HMM_PFN_VALID denotes the special
>> zero page */
>> + if (pfn & (HMM_PFN_VALID))
> No need for ()
ack
>
>> + migrate->src[i] =
>> migrate_pfn(page_to_pfn(hmm_pfn_to_page(pfn)))
> Is there no easier way to avoid going through a page?
>
> I guess you really just want "migrate_pfn(pfn & ~HMM_PFN_FLAGS)".
>
> Maybe add a hmm_pfn_to_pfn() helper that does the "pfn & ~HMM_PFN_FLAGS" ?
Basically yes. I noticed this was helpful for instance with the "separate
address space for device_private"
patchset not to depend on the coding.
>
>> + | MIGRATE_PFN_MIGRATE;
> We (MM folks) usually put the | onto the previous line to then properly indent
> the second line
Okay I will change.
>
> migrate->src[i] = migrate_pfn(page_to_pfn(hmm_pfn_to_page(pfn))) |
> MIGRATE_PFN_MIGRATE;
>
>> + else
>> + migrate->src[i] = MIGRATE_PFN_MIGRATE;
> It might be cleaner to just set migrate->src[i] = 0; here and to
> unconditionally
>
> migrate->src[i] |= MIGRATE_PFN_MIGRATE;
>
> So you don't have the MIGRATE_PFN_MIGRATE on two paths.
Good point.
>
>> +
>> + migrate->src[i] |= (pfn & HMM_PFN_WRITE) ? MIGRATE_PFN_WRITE :
>> 0;
>> + migrate->src[i] |= (pfn & HMM_PFN_COMPOUND) ?
>> MIGRATE_PFN_COMPOUND : 0;
>> + migrate->dst[i] = 0;
>> + }
>> +
>> + if (migrate->cpages)
>> + migrate_vma_unmap(migrate);
> Can you remind me why we do this here, in the setup() phase? The function doc
> does not really describe that.
It is to be in par what migrate_vma_setup() does today after the "collect"
phase.
Thanks,
Mika
>
