Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Memory leak on error path in `shmem_sg_alloc_table` when failing to allocate after populating exactly one scatterlist entry. -- --- Patch [4]: [PATCH v4 4/5] drm/i915/gem: Read and shrink memory in a separate function --- commit 5dd680cf1299c10f0034b5b2b67f54aa39eb638d Author: Krzysztof Karas <[email protected]> drm/i915/gem: Read and shrink memory in a separate function This commit extracts the page reading and memory shrinking logic from shmem_sg_alloc_table() into a new helper function shmem_shrink_get_folio(). It also removes a loop, noting that multiple shrink calls do not provide additional benefits. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index f35b7efa1fc23..ac8022997553b 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c [ ... ] > @@ -119,57 +169,16 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, > struct sg_table *st, > for (i = 0; i < page_count; i++) { > unsigned long folio_page_index = 0; > unsigned long nr_pages; > - const unsigned int shrink[] = { > - I915_SHRINK_BOUND | I915_SHRINK_UNBOUND, > - 0, > - }, *s = shrink; > gfp_t gfp = noreclaim; > > /* Grab the next folio if we exhausted the current one. */ > if (!i || i > folio_end) { > - do { [ ... ] > + folio = shmem_shrink_get_folio(mapping, i, gfp, > + page_count, i915); > + if (IS_ERR(folio)) { > + ret = PTR_ERR(folio); > + goto err_sg; > + } [Severity: High] This is a pre-existing issue, but does this error path leak memory if the allocation fails after exactly one scatterlist entry is populated? Looking at the error handling for shmem_sg_alloc_table in drivers/gpu/drm/i915/gem/i915_gem_shmem.c: 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); } If a folio is successfully allocated and added to the first scatterlist entry, but the loop fails on a subsequent iteration before sg is advanced with sg_next(), sg will still equal st->sgl. When we jump to err_sg under this condition, the code skips calling shmem_sg_free_table and only frees the scatterlist structure via sg_free_table. Could this bypass the reference count decrements for the folios already added to the first scatterlist entry, causing a permanent leak during memory exhaustion? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
