On 3/3/26 06:52, Jordan Niethe wrote: > Hi > > On 2/3/26 20:22, David Hildenbrand (Arm) wrote: >> On 3/2/26 00:38, Jordan Niethe wrote: >>> Hi, >>> >> >> I'll go through he remainder of the patchset this week. > > Much appreciated. > >> >> While skimming over patch #2, I was wondering whether >> "page_to_migration_pfn()" would better fit "migration_pfn_to_page". > > I guess you were thinking about migration_pfn_/from/_page() rather than > "migration_pfn_to_page()"? Renaming it to page_to_migration_pfn() would > be fine.
Yes. :) page_to_migration_pfn() and migration_pfn_to_page() But really, we should only be dealing with folio migration longterm ... > >> >> ... and I was wondering why that code deals with pages instead of folios. > >> >> E.g., >> >> page = folio_page(folio, 0); >> mpfn[i] = migrate_pfn_from_page(page); >> >> Should just be >> >> mpfn[i] = folio_to_migration_pfn(folio); >> >> Right? > > This patch is quite limited, essentially just converts usages of > migrate_pfn(page_to_pfn()) to migration_pfn_from_page(). However, I > agree, there could be scope for moving some of those usages to folios. > > Was that example from drm_pagemap_migrate_populate_ram_pfn()? There > 'page' goes on to be used elsewhere in the function so we'd need some > further refactoring to fully benefit. > > I see migrate_vma_collect_huge_pmd() as a candidate for a > folio_to_migration_pfn() function too. All that code should be converted to folios (unless I am missing something important). We only support migration of folios, and all ZONE_DEVICE pages correspond to folios. E.g., in migrate_vma_setup() we're implicitly converting from page to folio multiple times, which is rather wasteful. Now, I don't think the conversion is your responsibility :) But you'd provide the folio helpers and use them where we can already. I'm fine with leaving the code the still operates purely on page to be converted later. -- Cheers, David
