Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incorrect PFN array iteration skips unrelated folios when encountering tail pages. --
--- Patch [1]: [PATCH v3] drm/pagemap: Prevent double migration of device pages --- commit 8b99d73a2bacb652ddc2b438316d6f18c0ebfb5e Author: Arvind Yadav <[email protected]> drm/pagemap: Prevent double migration of device pages A device-private folio migrated to system memory by a CPU fault can remain reachable through the raw-PFN eviction path until migration finalization drops the source reference. > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c [ ... ] > +static void drm_pagemap_retire_migrated_pages(unsigned long *src_pfns, > + unsigned long npages) > +{ > + unsigned long i = 0; > + > + while (i < npages) { > + struct page *page = migrate_pfn_to_page(src_pfns[i]); > + unsigned long nr = 1; > + > + if (!page) { > + i++; > + continue; > + } > + > + nr = folio_nr_pages(page_folio(page)); > + > + if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) && > + is_device_private_page(page)) > + drm_pagemap_page_set_flags(page, > + > DRM_PAGEMAP_ZDD_FLAG_MIGRATED); > + > + i += nr; [Severity: High] Does this iteration correctly handle tail pages? If src_pfns[i] points to a tail page of a large folio (for instance, when a migration batch starts in the middle of a large device-private folio due to unaligned VMA boundaries or partial unmaps), folio_nr_pages() returns the total number of pages in the entire large folio. Advancing the array index by i += nr will overshoot the remaining pages in the array, skipping subsequent unrelated folios. This could prevent the DRM_PAGEMAP_ZDD_FLAG_MIGRATED flag from being set on the skipped folios, leaving them vulnerable to the exact double-migration soft lockups and RCU stalls this patch aims to fix. > + } > +} > + > +static void drm_pagemap_skip_retired_pages(unsigned long *src_pfns, > + unsigned long npages) > +{ > + unsigned long i = 0; > + > + while (i < npages) { > + struct page *page = migrate_pfn_to_page(src_pfns[i]); > + unsigned long nr = 1; > + > + if (!page) { > + i++; > + continue; > + } > + > + nr = folio_nr_pages(page_folio(page)); > + > + if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) && > + is_device_private_page(page) && > + (drm_pagemap_page_get_flags(page) & > + DRM_PAGEMAP_ZDD_FLAG_MIGRATED)) > + src_pfns[i] &= ~MIGRATE_PFN_MIGRATE; > + > + i += nr; [Severity: High] Can this loop also skip unrelated folios if the current page is a tail page? Using folio_nr_pages() to advance the array index from a tail page has the same overshoot issue as above, potentially failing to clear the MIGRATE_PFN_MIGRATE bit for migrated folios and leading to a double-migration attempt. > + } > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
