Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] xe_shrinker_count() reports shrinkable UMA BOs regardless of __GFP_FS, but xe_shrinker_scan() bails out without scanning them when __GFP_FS is missing. - [High] Bypassing ttm_backup_bytes_avail() for UMA shrinkable BOs falsely assumes they can be freed without swap space, causing useless CPU overhead yielding zero net memory reclamation. --
commit eb93ae097c20ae5f357e41557ba0c3d785cb142c Author: aquiles <[email protected]> drm/xe: count UMA shrinkable BOs as reclaimable without swap This commit modifies the xe shrinker count logic to unconditionally report UMA shrinkable buffer objects as reclaimable, even without swap space. It subtracts UMA shrinkable pages from the ttm backup limit check and adds them back unconditionally. > diff --git a/drivers/gpu/drm/xe/xe_shrinker.c > b/drivers/gpu/drm/xe/xe_shrinker.c > index 83374cd576608..3ca2de342dcd6 100644 > --- a/drivers/gpu/drm/xe/xe_shrinker.c > +++ b/drivers/gpu/drm/xe/xe_shrinker.c [ ... ] > @@ -140,16 +140,22 @@ xe_shrinker_count(struct shrinker *shrink, struct > shrink_control *sc) > { > struct xe_shrinker *shrinker = to_xe_shrinker(shrink); > unsigned long num_pages; > + long uma_reclaimable_pages; > bool can_backup = !!(sc->gfp_mask & __GFP_FS); > + bool is_uma = !IS_DGFX(shrinker->xe); > > num_pages = ttm_backup_bytes_avail() >> PAGE_SHIFT; > read_lock(&shrinker->lock); > > + uma_reclaimable_pages = is_uma ? shrinker->shrinkable_pages : 0; > + > if (can_backup) > - num_pages = min_t(unsigned long, num_pages, > shrinker->shrinkable_pages); > + num_pages = min_t(unsigned long, num_pages, > + shrinker->shrinkable_pages - > uma_reclaimable_pages); [Severity: High] Does bypassing the ttm_backup_bytes_avail() limit for UMA shrinkable BOs result in zero net memory reclamation without swap? On UMA systems without swap, the shrinker uses ttm_bo_shrink() which backs up the BO by copying its pages to a shmem file. Because no swap space is available, the newly allocated shmem backup pages cannot be swapped to disk and remain permanently pinned in physical memory. This deceives the core MM into trying to reclaim these pages, burning CPU cycles on copying memory that yields zero net free physical memory. > else > num_pages = 0; > > + num_pages += uma_reclaimable_pages; [Severity: High] Can unconditionally returning uma_reclaimable_pages cause an infinite reclaim retry loop when __GFP_FS is missing? When the shrinker is invoked with __GFP_FS missing (e.g., direct reclaim in the filesystem or block layer), can_backup is false, but this code adds uma_reclaimable_pages to the returned count anyway. Looking at xe_shrinker_scan() in the same file, the scan loop explicitly bails out if !can_backup before it ever attempts to scan non-purgeable (shrinkable) pages: if (nr_scanned >= nr_to_scan || !can_backup) goto out; /* ... */ shrink_flags.purge = false; The shrinker API strictly requires the count to accurately reflect only the objects that the scan function will attempt to free under the given gfp_mask. Returning a falsely high count for objects that the scan function explicitly skips breaks the VM reclaim feedback loop, causing infinite reclaim loops. > num_pages += shrinker->purgeable_pages; > read_unlock(&shrinker->lock); > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
