Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Pinned folios are permanently leaked in the error path if an allocation fails before the scatterlist pointer `sg` is advanced past the first entry. -- --- Patch [5]: [PATCH v3 5/5] drm/i915/gem: Remove iterator and use while loop --- commit f3eba03d9fc5ce905a11cce1578d72ea21a144f1 Author: Krzysztof Karas <[email protected]> drm/i915/gem: Remove iterator and use while loop This commit changes the main "for" loop into a "while" loop to get rid of the obscure iterator "i" and use a more descriptive name to indicate how many pages were already covered. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index 7c8de8fe0a225..66d0f8f6ffccb 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c [ ... ] > @@ -212,7 +211,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, > struct sg_table *st, > } > > next_pfn = folio_pfn(folio) + folio_page_index + nr_pages; > - i += nr_pages - 1; > + pages_done += nr_pages; > > /* Check that the i965g/gm workaround works. */ > GEM_BUG_ON(gfp & __GFP_DMA32 && next_pfn >= 0x00100000UL); [Severity: High] This isn't a bug introduced by this patch, but looking at the error handling in shmem_sg_alloc_table() in drivers/gpu/drm/i915/gem/i915_gem_shmem.c (around line 228), is it possible to leak pinned folios? If an allocation fails after successfully allocating the first folio(s) into the first scatterlist entry, st->nents will be 1 but sg will still equal st->sgl. The error path jumps to err_sg: err_sg: sg_mark_end(sg); if (sg != st->sgl) { shmem_sg_free_table(st, mapping, false, false); } else { mapping_clear_unevictable(mapping); sg_free_table(st); } Since sg == st->sgl, this takes the else branch and bypasses shmem_sg_free_table(), leaving the folios in the first scatterlist entry pinned. Would checking if (st->nents > 0) be more appropriate here? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
