Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Unconditional PAGE_SIZE stepping over a scatterlist performs non-idempotent folio operations multiple times for large folios. -- --- Patch [1]: [PATCH] drm/i915/userptr: Convert page APIs to folio equivalents in put_pages --- commit a2440a520ca40b971ab3143efb8ecac51e56d073 Author: Hongfu Li <[email protected]> drm/i915/userptr: Convert page APIs to folio equivalents in put_pages This commit replaces page-based APIs with folio equivalents in i915_gem_userptr_put_pages(). It caches the folio at the start of the loop to avoid repeated compound head lookups inside page API wrappers. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > index 043095f93ac60..bd91782f4bbc7 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c [ ... ] > @@ -177,7 +178,9 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object > *obj, > obj->mm.dirty = false; > > for_each_sgt_page(page, sgt_iter, pages) { > - if (obj->mm.dirty && trylock_page(page)) { > + folio = page_folio(page); > + > + if (obj->mm.dirty && folio_trylock(folio)) { [Severity: High] This isn't a bug introduced by this patch, but does this loop process the same folio multiple times for large folios? In i915_gem_userptr_put_pages(), the for_each_sgt_page() macro unconditionally steps by PAGE_SIZE. This causes page_folio() to repeatedly return the same folio for the tail pages of a large folio. For non-idempotent operations, this causes the folio to be processed redundantly. If the scatterlist is backed by a 2MB THP, the same folio is processed 512 times. This means the code unnecessarily locks and unlocks the folio 512 times, and folio_mark_accessed() causes the folio to repeatedly toggle between referenced and active states. Because large folios bypass per-CPU LRU batches, this hammers the global zone LRU spinlock repeatedly per THP, causing severe system-wide lock contention. Could we track the last processed folio to avoid duplicate processing of tail pages in this loop? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
