drm/msm/gem: Prevent blocking within shrinker loop

[ Upstream commit 941f26794ce3e95a7349f2d3eb9570e712d07f38 ]

Consider this scenario:

1. APP1 continuously creates lots of small GEMs
2. APP2 triggers `drop_caches`
3. Shrinker starts to evict APP1 GEMs, while APP1 produces new purgeable
   GEMs
4. msm_gem_shrinker_scan() returns non-zero number of freed pages
   and causes shrinker to try shrink more
5. msm_gem_shrinker_scan() returns non-zero number of freed pages again,
   goto 4
6. The APP2 is blocked in `drop_caches` until APP1 stops producing
   purgeable GEMs

To prevent this blocking scenario, check number of remaining pages
that GPU shrinker couldn't release due to a GEM locking contention
or shrinking rejection. If there are no remaining pages left to shrink,
then there is no need to free up more pages and shrinker may break out
from the loop.

This problem was found during shrinker/madvise IOCTL testing of
virtio-gpu driver. The MSM driver is affected in the same way.

Reviewed-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 04a0d2f95e85 ("drm/msm/gem: Convert to using drm_gem_lru")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Link: https://lore.kernel.org/all/20230108210445.3948344-2-dmitry.osipenko@collabora.com/
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 8b68a3c1e6ab6ea53a712b891c1ba8adbc8d1bbc..b87ed4238fc836622080332166d0262137b1021e 100644 (file)
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1351,10 +1351,13 @@ EXPORT_SYMBOL(drm_gem_lru_move_tail);
  *
  * @lru: The LRU to scan
  * @nr_to_scan: The number of pages to try to reclaim
+ * @remaining: The number of pages left to reclaim, should be initialized by caller
  * @shrink: Callback to try to shrink/reclaim the object.
  */
 unsigned long
-drm_gem_lru_scan(struct drm_gem_lru *lru, unsigned nr_to_scan,
+drm_gem_lru_scan(struct drm_gem_lru *lru,
+                unsigned int nr_to_scan,
+                unsigned long *remaining,
                 bool (*shrink)(struct drm_gem_object *obj))
 {
        struct drm_gem_lru still_in_lru;
@@ -1393,8 +1396,10 @@ drm_gem_lru_scan(struct drm_gem_lru *lru, unsigned nr_to_scan,
                 * hit shrinker in response to trying to get backing pages
                 * for this obj (ie. while it's lock is already held)
                 */
-               if (!dma_resv_trylock(obj->resv))
+               if (!dma_resv_trylock(obj->resv)) {
+                       *remaining += obj->size >> PAGE_SHIFT;
                        goto tail;
+               }
 
                if (shrink(obj)) {
                        freed += obj->size >> PAGE_SHIFT;

diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c
index 1de14e67f96b02012be0c205faf6fbb65ca0f8f1..31f054c903a43d81e97d8bee11b174da46a3c835 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
+++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
@@ -107,6 +107,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
                bool (*shrink)(struct drm_gem_object *obj);
                bool cond;
                unsigned long freed;
+               unsigned long remaining;
        } stages[] = {
                /* Stages of progressively more aggressive/expensive reclaim: */
                { &priv->lru.dontneed, purge,        true },
@@ -116,14 +117,18 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
        };
        long nr = sc->nr_to_scan;
        unsigned long freed = 0;
+       unsigned long remaining = 0;
 
        for (unsigned i = 0; (nr > 0) && (i < ARRAY_SIZE(stages)); i++) {
                if (!stages[i].cond)
                        continue;
                stages[i].freed =
-                       drm_gem_lru_scan(stages[i].lru, nr, stages[i].shrink);
+                       drm_gem_lru_scan(stages[i].lru, nr,
+                                       &stages[i].remaining,
+                                        stages[i].shrink);
                nr -= stages[i].freed;
                freed += stages[i].freed;
+               remaining += stages[i].remaining;
        }
 
        if (freed) {
@@ -132,7 +137,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
                                     stages[3].freed);
        }
 
-       return (freed > 0) ? freed : SHRINK_STOP;
+       return (freed > 0 && remaining > 0) ? freed : SHRINK_STOP;
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -182,10 +187,12 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
                NULL,
        };
        unsigned idx, unmapped = 0;
+       unsigned long remaining = 0;
 
        for (idx = 0; lrus[idx] && unmapped < vmap_shrink_limit; idx++) {
                unmapped += drm_gem_lru_scan(lrus[idx],
                                             vmap_shrink_limit - unmapped,
+                                            &remaining,
                                             vmap_shrink);
        }
 

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index bd42f25e449c2249b85deba5200c17790c32397e..60b2dda8d964b3d298a8ca93164b45db3ae25243 100644 (file)
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -472,7 +472,9 @@ int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
 void drm_gem_lru_init(struct drm_gem_lru *lru, struct mutex *lock);
 void drm_gem_lru_remove(struct drm_gem_object *obj);
 void drm_gem_lru_move_tail(struct drm_gem_lru *lru, struct drm_gem_object *obj);
-unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru, unsigned nr_to_scan,
+unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
+                              unsigned int nr_to_scan,
+                              unsigned long *remaining,
                               bool (*shrink)(struct drm_gem_object *obj));
 
 #endif /* __DRM_GEM_H__ */