Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] __shmem_writeback() limits writeback to SWAP_CLUSTER_MAX (32) pages, ignoring the requested size, which causes the shrinker to severely under-reclaim memory while reporting full reclaim success. -- commit bda67bb2ffdd195685542c6f27e5b602b16ff109 Author: Matthias Goergens <[email protected]> drm/i915: unlock shmem folios after skipped writeback This commit fixes an issue where shmem folios are left locked indefinitely when writeback is skipped. It unlocks folios when they are redirtied or when shmem_write_folio returns AOP_WRITEPAGE_ACTIVATE. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index ef94401662955..e7b18f7f9644c 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > @@ -322,10 +322,16 @@ void __shmem_writeback(size_t size, struct > address_space *mapping) > * as normal. > */ > while ((folio = writeback_iter(mapping, &wbc, folio, &error))) { [Severity: High] This isn't a bug introduced by this patch, but does the hardcoded limit on wbc.nr_to_write here cause the shrinker to severely under-reclaim? In drivers/gpu/drm/i915/gem/i915_gem_shmem.c:__shmem_writeback(), the size parameter is completely ignored and the writeback_control is initialized earlier in the function with: struct writeback_control wbc = { .sync_mode = WB_SYNC_NONE, .nr_to_write = SWAP_CLUSTER_MAX, ... }; Because of this, writeback_iter() will terminate after 32 pages (128KB). However, when the system is under memory pressure and i915_gem_shrinker_scan() delegates to i915_gem_shrink(), try_to_writeback() eventually reaches __shmem_writeback(). i915_gem_shrink() assumes the entire object was successfully written back and increments its freed count by the full object size. If the object is large, the mm subsystem is misled into thinking massive amounts of memory were freed when only 128KB actually was. This could degrade memory reclaim under pressure and lead to system stalls or OOMs. Should the writeback limit respect the requested size parameter instead of being hardcoded to SWAP_CLUSTER_MAX? > - if (folio_mapped(folio)) > + if (folio_mapped(folio)) { > folio_redirty_for_writepage(&wbc, folio); > - else > + folio_unlock(folio); > + } else { > error = shmem_write_folio(folio); > + if (error == AOP_WRITEPAGE_ACTIVATE) { > + folio_unlock(folio); > + error = 0; > + } > + } > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
