Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Unnecessary double execution of the memory shrinker on the allocation failure path. --
--- Patch [4]: [PATCH v3 4/5] drm/i915/gem: Read and shrink memory in a separate function --- commit 323006b149f5bd15e61d31522a7aa9cdbf860e09 Author: Krzysztof Karas <[email protected]> drm/i915/gem: Read and shrink memory in a separate function This commit extracts the memory reading and shrinking logic from shmem_sg_alloc_table into a new shmem_shrink_get_folio helper. It also replaces the previous array-based iteration with a do-while loop. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c [ ... ] > @@ -78,6 +78,55 @@ static int validate_size(size_t size, unsigned int > page_count, > return 0; > } > > +static struct folio *shmem_shrink_get_folio(struct address_space *mapping, > + unsigned long folio_index, > + gfp_t gfp, unsigned int page_count, > + struct drm_i915_private *i915) > +{ > + struct folio *folio = NULL; > + unsigned int retries = 2; > + > + do { > + cond_resched(); > + folio = shmem_read_folio_gfp(mapping, folio_index, gfp); > + if (IS_ERR(folio)) { > + i915_gem_shrink(NULL, i915, 2 * page_count, NULL, > + I915_SHRINK_BOUND | > I915_SHRINK_UNBOUND); [Severity: Medium] Will this result in an unnecessary double execution of the memory shrinker when allocations fail? In the original code, the array-based state machine checked the shrink state and jumped to the error path before calling i915_gem_shrink again on the final failure. With the refactored do-while loop, if the second shmem_read_folio_gfp call fails, the loop unconditionally executes the shrinker a second time before decrementing the retries counter to 0 and exiting. Could this waste CPU cycles and increase lock contention on i915->mm.obj_lock immediately before returning -ENOMEM to the caller, considering the memory freed by this second pass is completely ignored? > + > + /* > + * We've tried hard to allocate the memory by reaping [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
