Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] drm_pagemap_src_pfn_nr_pages relies on folio_order for valid pages, causing severe overcounting and skipped entries for PTE-mapped THPs, which masks migration failures. -- commit bb0a5adb569e2e78382923dc4aeb9bb445b55021 Author: Junhua Shen <[email protected]> drm/pagemap: fix page undercount for compound src entries This commit factors out the page count decoding into a new helper function, drm_pagemap_src_pfn_nr_pages(), to consistently handle compound source entries. It ensures that entries with only MIGRATE_PFN_COMPOUND are correctly counted as HPAGE_PMD_NR base pages in both accounting loops of drm_pagemap_migrate_to_devmem(). This prevents a potential spurious -EBUSY in the final migration race check due to undercounting. > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index 892b325fa99b8..a31cc4f0af68c 100644 > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c [ ... ] > @@ -554,6 +554,37 @@ static int drm_pagemap_migrate_range(struct > drm_pagemap_devmem *devmem, > +static unsigned long drm_pagemap_src_pfn_nr_pages(unsigned long src_pfn, > + struct page **src_page) > +{ > + struct page *page = NULL; > + unsigned long nr_pages = 1; > + > + if (src_pfn & MIGRATE_PFN_VALID) { > + page = migrate_pfn_to_page(src_pfn); > + if (page) > + nr_pages = NR_PAGES(folio_order(page_folio(page))); [Severity: High] This is a pre-existing issue, but by factoring out this logic, the helper perpetuates a flaw for PTE-mapped large folios (THPs). For a valid page that is part of a PTE-mapped THP, this derives the step size from the folio order rather than the migration entry flags. Since migrate_vma tracks PTE-mapped THPs as independent order-0 entries (each with MIGRATE_PFN_VALID but not MIGRATE_PFN_COMPOUND), will returning the full large folio size here cause severe overcounting for these entries? > + } else if (src_pfn & MIGRATE_PFN_COMPOUND) { > + nr_pages = HPAGE_PMD_NR; > + } > + > + if (src_page) > + *src_page = page; > + > + return nr_pages; > +} [ ... ] > @@ -818,10 +844,10 @@ int drm_pagemap_migrate_to_devmem(struct > drm_pagemap_devmem *devmem_allocation, > migrate_vma_pages(&migrate); > > for (i = 0; !err && i < npages;) { > - struct page *page = migrate_pfn_to_page(migrate.src[i]); > - unsigned long nr_pages = page ? > NR_PAGES(folio_order(page_folio(page))) : 1; > + unsigned long src_pfn = migrate.src[i]; > + unsigned long nr_pages = drm_pagemap_src_pfn_nr_pages(src_pfn, > NULL); > > - if (migrate.src[i] & MIGRATE_PFN_MIGRATE) > + if (src_pfn & MIGRATE_PFN_MIGRATE) > migrated_pages += nr_pages; > > i += nr_pages; [Severity: High] This isn't a bug introduced by this patch, but because drm_pagemap_src_pfn_nr_pages() returns the full folio order for PTE-mapped THPs, won't this final verification loop in drm_pagemap_migrate_to_devmem() step by 512 instead of 1? If it steps by 512 for a PTE-mapped THP, it skips checking the MIGRATE_PFN_MIGRATE flag for the remaining independent tail pages. If any tail page failed to migrate, does this silently mask the migration failure and falsely report complete success, risking data corruption and permanently desynchronized CPU/GPU page tables? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
