Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] msm_gem_shrinker_scan loses freed count by returning SHRINK_STOP when remaining is 0. - [High] drm_gem_lru_scan fails to account for unscanned objects in the remaining counter, causing catastrophic early aborts. -- commit 9ce73904f0ad62a66be5eff7808ac4a6af7edc46 Author: Boris Brezillon <[email protected]> drm/gem: Consider GEM object reclaimable if shrinking fails If the object wasn't moved to a different LRU after the shrink callback returns, it means the buffer is still reclaimable. Update the remaining counter to reflect that. Link: https://patch.msgid.link/[email protected] > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index f884d155a832a..6e778939acae4 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1574,6 +1574,16 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, > */ > WARN_ON(obj->lru == &still_in_lru); > WARN_ON(obj->lru == lru); > + } else if (obj->lru == &still_in_lru) { > + /* > + * If the object wasn't moved and wasn't shrunk either, > + * it's still remaining as reclaimable. Note that > + * obj->lru is supposed to be checked with the LRU lock > + * held for an accurate result, but we don't care about > + * accuracy here. Worst thing that could happen is an > + * extra scan. > + */ > + *remaining += obj->size >> PAGE_SHIFT; [Severity: High] This isn't a bug introduced by this patch, but does this manual accumulation fully fix the remaining counter? In drivers/gpu/drm/drm_gem.c:drm_gem_lru_scan(), when the shrinker successfully frees enough objects to satisfy nr_to_scan, the loop terminates early: while (freed < nr_to_scan) { ... } The objects still left in lru->list are never scanned, and their sizes are not added to *remaining. If no objects failed to shrink before the loop terminated, *remaining stays 0. This missing count triggers a severe pre-existing issue in drivers/gpu/drm/msm/msm_gem_shrinker.c:msm_gem_shrinker_scan(): return (freed > 0 && remaining > 0) ? freed : SHRINK_STOP; When remaining is incorrectly 0, this logic swallows the freed count by returning SHRINK_STOP. The core VM shrinker then drops the count of freed pages, causing under-reporting of reclaimed memory which can mislead kswapd and trigger premature OOM. Should lru->count + still_in_lru.count be added to *remaining before returning from drm_gem_lru_scan(), rather than manually accumulating sizes inside the loop? > } > > dma_resv_unlock(obj->resv); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
