public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: "David Airlie" <airlied@gmail.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.com>,
	"Daniel Stone" <daniel@fooishbar.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rob Clark" <robdclark@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"Qiang Yu" <yuq825@gmail.com>,
	"Steven Price" <steven.price@arm.com>,
	"Alyssa Rosenzweig" <alyssa.rosenzweig@collabora.com>,
	"Rob Herring" <robh@kernel.org>, "Sean Paul" <sean@poorly.run>,
	"Dmitry Baryshkov" <dmitry.baryshkov@linaro.org>,
	"Abhinav Kumar" <quic_abhinavk@quicinc.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	kernel@collabora.com, virtualization@lists.linux-foundation.org
Subject: [PATCH v10 01/11] drm/msm/gem: Prevent blocking within shrinker loop
Date: Mon,  9 Jan 2023 00:04:35 +0300	[thread overview]
Message-ID: <20230108210445.3948344-2-dmitry.osipenko@collabora.com> (raw)
In-Reply-To: <20230108210445.3948344-1-dmitry.osipenko@collabora.com>

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>
Fixes: b352ba54a820 ("drm/msm/gem: Convert to using drm_gem_lru")
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/gpu/drm/drm_gem.c              | 9 +++++++--
 drivers/gpu/drm/msm/msm_gem_shrinker.c | 8 ++++++--
 include/drm/drm_gem.h                  | 4 +++-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 59a0bb5ebd85..c6bca5ac6e0f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1388,10 +1388,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
  * @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;
@@ -1430,8 +1433,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 051bdbc093cf..b7c1242014ec 100644
--- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
+++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
@@ -116,12 +116,14 @@ 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, &remaining,
+					 stages[i].shrink);
 		nr -= stages[i].freed;
 		freed += stages[i].freed;
 	}
@@ -132,7 +134,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 +184,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 772a4adf5287..f1f00fc2dba6 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -476,7 +476,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__ */
-- 
2.38.1


  reply	other threads:[~2023-01-08 21:05 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-08 21:04 [PATCH v10 00/11] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers Dmitry Osipenko
2023-01-08 21:04 ` Dmitry Osipenko [this message]
2023-02-17 12:02   ` [PATCH v10 01/11] drm/msm/gem: Prevent blocking within shrinker loop Thomas Zimmermann
2023-02-27  4:27     ` Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 02/11] drm/panfrost: Don't sync rpm suspension after mmu flushing Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 03/11] drm/gem: Add evict() callback to drm_gem_object_funcs Dmitry Osipenko
2023-02-17 12:23   ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 04/11] drm/shmem: Put booleans in the end of struct drm_gem_shmem_object Dmitry Osipenko
2023-02-17 12:25   ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 05/11] drm/shmem: Switch to use drm_* debug helpers Dmitry Osipenko
2023-01-26 12:15   ` Gerd Hoffmann
2023-02-17 12:28   ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 06/11] drm/shmem-helper: Don't use vmap_use_count for dma-bufs Dmitry Osipenko
2023-01-26 12:17   ` Gerd Hoffmann
2023-01-26 12:24     ` Dmitry Osipenko
2023-01-27  8:06       ` Gerd Hoffmann
2023-02-17 12:41   ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 07/11] drm/shmem-helper: Switch to reservation lock Dmitry Osipenko
2023-02-17 12:52   ` Thomas Zimmermann
2023-02-17 13:33     ` Dmitry Osipenko
2023-02-17 13:29   ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 08/11] drm/shmem-helper: Add memory shrinker Dmitry Osipenko
2023-02-17 13:19   ` Thomas Zimmermann
2023-02-27  4:34     ` Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 09/11] drm/gem: Add drm_gem_pin_unlocked() Dmitry Osipenko
2023-02-17 13:42   ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 10/11] drm/virtio: Support memory shrinking Dmitry Osipenko
2023-01-27  8:04   ` Gerd Hoffmann
2023-01-08 21:04 ` [PATCH v10 11/11] drm/panfrost: Switch to generic memory shrinker Dmitry Osipenko
2023-01-25 22:55 ` [PATCH v10 00/11] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers Dmitry Osipenko
2023-01-27  8:13   ` Gerd Hoffmann
2023-01-30 12:02     ` Dmitry Osipenko
2023-02-16 12:15       ` Daniel Vetter
2023-02-16 13:08         ` AngeloGioacchino Del Regno
2023-02-16 20:43         ` Dmitry Osipenko
2023-02-16 22:07           ` Daniel Vetter
2023-02-17 13:28 ` Thomas Zimmermann
2023-02-17 13:41   ` Dmitry Osipenko
2023-02-27  4:19     ` Dmitry Osipenko
2023-02-27 10:37       ` Jani Nikula
2023-02-27 11:00         ` Dmitry Osipenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230108210445.3948344-2-dmitry.osipenko@collabora.com \
    --to=dmitry.osipenko@collabora.com \
    --cc=airlied@gmail.com \
    --cc=alyssa.rosenzweig@collabora.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel.almeida@collabora.com \
    --cc=daniel@ffwll.ch \
    --cc=daniel@fooishbar.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gurchetansingh@chromium.org \
    --cc=gustavo.padovan@collabora.com \
    --cc=kernel@collabora.com \
    --cc=kraxel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=olvaffe@gmail.com \
    --cc=quic_abhinavk@quicinc.com \
    --cc=robdclark@gmail.com \
    --cc=robh@kernel.org \
    --cc=sean@poorly.run \
    --cc=steven.price@arm.com \
    --cc=sumit.semwal@linaro.org \
    --cc=tomeu.vizoso@collabora.com \
    --cc=tzimmermann@suse.de \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=yuq825@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox