All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: "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>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Christian König" <christian.koenig@amd.com>,
	"Qiang Yu" <yuq825@gmail.com>,
	"Steven Price" <steven.price@arm.com>,
	"Emma Anholt" <emma@anholt.net>, "Melissa Wen" <mwen@igalia.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	kernel@collabora.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH v18 19/26] drm/shmem-helper: Add common memory shrinker
Date: Fri, 10 Nov 2023 15:58:58 +0100	[thread overview]
Message-ID: <20231110155858.4c92f05d@collabora.com> (raw)
In-Reply-To: <20231029230205.93277-20-dmitry.osipenko@collabora.com>

On Mon, 30 Oct 2023 02:01:58 +0300
Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote:

> @@ -238,6 +308,20 @@ void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
>  	if (refcount_dec_not_one(&shmem->pages_use_count))
>  		return;
>  
> +	/*
> +	 * Destroying the object is a special case because acquiring
> +	 * the obj lock can cause a locking order inversion between
> +	 * reservation_ww_class_mutex and fs_reclaim.
> +	 *
> +	 * This deadlock is not actually possible, because no one should
> +	 * be already holding the lock when GEM is released.  Unfortunately
> +	 * lockdep is not aware of this detail.  So when the refcount drops
> +	 * to zero, we pretend it is already locked.
> +	 */
> +	if (!kref_read(&shmem->base.refcount) &&
> +	    refcount_dec_and_test(&shmem->pages_use_count))
> +		return drm_gem_shmem_free_pages(shmem);

Uh, with get/put_pages() being moved to the create/free_gem()
hooks, we're back to a situation where pages_use_count > 0 when we
reach gem->refcount == 0, which is not nice. We really need to patch
drivers so they dissociate GEM creation from the backing storage
allocation/reservation + mapping of the BO in GPU VM space.


WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: kernel@collabora.com, "Thomas Zimmermann" <tzimmermann@suse.de>,
	"Emma Anholt" <emma@anholt.net>,
	"Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	"Maxime Ripard" <mripard@kernel.org>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Melissa Wen" <mwen@igalia.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Steven Price" <steven.price@arm.com>,
	virtualization@lists.linux-foundation.org,
	"Qiang Yu" <yuq825@gmail.com>
Subject: Re: [PATCH v18 19/26] drm/shmem-helper: Add common memory shrinker
Date: Fri, 10 Nov 2023 15:58:58 +0100	[thread overview]
Message-ID: <20231110155858.4c92f05d@collabora.com> (raw)
In-Reply-To: <20231029230205.93277-20-dmitry.osipenko@collabora.com>

On Mon, 30 Oct 2023 02:01:58 +0300
Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote:

> @@ -238,6 +308,20 @@ void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
>  	if (refcount_dec_not_one(&shmem->pages_use_count))
>  		return;
>  
> +	/*
> +	 * Destroying the object is a special case because acquiring
> +	 * the obj lock can cause a locking order inversion between
> +	 * reservation_ww_class_mutex and fs_reclaim.
> +	 *
> +	 * This deadlock is not actually possible, because no one should
> +	 * be already holding the lock when GEM is released.  Unfortunately
> +	 * lockdep is not aware of this detail.  So when the refcount drops
> +	 * to zero, we pretend it is already locked.
> +	 */
> +	if (!kref_read(&shmem->base.refcount) &&
> +	    refcount_dec_and_test(&shmem->pages_use_count))
> +		return drm_gem_shmem_free_pages(shmem);

Uh, with get/put_pages() being moved to the create/free_gem()
hooks, we're back to a situation where pages_use_count > 0 when we
reach gem->refcount == 0, which is not nice. We really need to patch
drivers so they dissociate GEM creation from the backing storage
allocation/reservation + mapping of the BO in GPU VM space.


  reply	other threads:[~2023-11-10 14:59 UTC|newest]

Thread overview: 168+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-29 23:01 [PATCH v18 00/26] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers Dmitry Osipenko
2023-10-29 23:01 ` Dmitry Osipenko
2023-10-29 23:01 ` [PATCH v18 01/26] drm/gem: Change locked/unlocked postfix of drm_gem_v/unmap() function names Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:34   ` Maxime Ripard
2023-11-24 10:34     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 02/26] drm/gem: Add _locked postfix to functions that have unlocked counterpart Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:35   ` Maxime Ripard
2023-11-24 10:35     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 03/26] drm/shmem-helper: Make all exported symbols GPL Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:36   ` Maxime Ripard
2023-11-24 10:36     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked functions Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:40   ` Maxime Ripard
2023-11-24 10:40     ` Maxime Ripard
2023-11-24 10:44     ` Boris Brezillon
2023-11-24 10:44       ` Boris Brezillon
2023-11-24 10:59     ` Boris Brezillon
2023-11-24 10:59       ` Boris Brezillon
2023-11-28 11:14       ` Maxime Ripard
2023-11-28 11:14         ` Maxime Ripard
2023-11-28 12:37         ` Boris Brezillon
2023-11-28 12:37           ` Boris Brezillon
2023-11-28 22:05           ` Dmitry Osipenko
2023-11-28 22:05             ` Dmitry Osipenko
2023-11-29  7:53             ` Boris Brezillon
2023-11-29  7:53               ` Boris Brezillon
2023-11-29 10:47               ` Dmitry Osipenko
2023-11-29 10:47                 ` Dmitry Osipenko
2023-11-29 10:57                 ` Boris Brezillon
2023-11-29 10:57                   ` Boris Brezillon
2023-11-29 13:09               ` Maxime Ripard
2023-11-29 13:09                 ` Maxime Ripard
2023-11-29 13:46                 ` Boris Brezillon
2023-11-29 13:46                   ` Boris Brezillon
2023-11-29 15:15                   ` Maxime Ripard
2023-11-29 15:15                     ` Maxime Ripard
2023-11-29 15:47                     ` Boris Brezillon
2023-11-29 15:47                       ` Boris Brezillon
2023-12-04 12:55                       ` Maxime Ripard
2023-12-04 12:55                         ` Maxime Ripard
2023-12-05 11:43                         ` Dmitry Osipenko
2023-12-05 11:43                           ` Dmitry Osipenko
2023-12-14 18:16                           ` Maxime Ripard
2023-12-14 18:16                             ` Maxime Ripard
2023-12-15  0:42                             ` Dmitry Osipenko
2023-12-15  0:42                               ` Dmitry Osipenko
2023-10-29 23:01 ` [PATCH v18 05/26] drm/shmem-helper: Remove obsoleted is_iomem test Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 10:08   ` Boris Brezillon
2023-11-10 10:08     ` Boris Brezillon
2023-11-24 10:40   ` Maxime Ripard
2023-11-24 10:40     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 06/26] drm/shmem-helper: Add and use pages_pin_count Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:41   ` Maxime Ripard
2023-11-24 10:41     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 07/26] drm/shmem-helper: Use refcount_t for pages_use_count Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:41   ` Maxime Ripard
2023-11-24 10:41     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 08/26] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages() Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:47   ` Maxime Ripard
2023-11-24 10:47     ` Maxime Ripard
2023-11-24 11:20     ` Boris Brezillon
2023-11-24 11:20       ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 09/26] drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:48   ` Maxime Ripard
2023-11-24 10:48     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 10/26] drm/shmem-helper: Use refcount_t for vmap_use_count Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-24 10:48   ` Maxime Ripard
2023-11-24 10:48     ` Maxime Ripard
2023-10-29 23:01 ` [PATCH v18 11/26] drm/shmem-helper: Prepare drm_gem_shmem_free() to shrinker addition Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 10:16   ` Boris Brezillon
2023-11-10 10:16     ` Boris Brezillon
2023-11-20 11:02     ` Dmitry Osipenko
2023-11-20 11:02       ` Dmitry Osipenko
2023-11-20 11:19       ` Boris Brezillon
2023-11-20 11:19         ` Boris Brezillon
2023-11-20 11:38         ` Dmitry Osipenko
2023-11-20 11:38           ` Dmitry Osipenko
2023-10-29 23:01 ` [PATCH v18 12/26] drm/shmem-helper: Make drm_gem_shmem_get_pages() public Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 10:17   ` Boris Brezillon
2023-11-10 10:17     ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 13/26] drm/shmem-helper: Add drm_gem_shmem_put_pages() Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-10-29 23:01 ` [PATCH v18 14/26] drm/lima: Explicitly get and put drm-shmem pages Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 10:30   ` Boris Brezillon
2023-11-10 10:30     ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 15/26] drm/panfrost: " Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 10:53   ` Boris Brezillon
2023-11-10 10:53     ` Boris Brezillon
2023-11-22 22:04     ` Dmitry Osipenko
2023-11-22 22:04       ` Dmitry Osipenko
2023-11-23  9:05       ` Boris Brezillon
2023-11-23  9:05         ` Boris Brezillon
2023-11-23 12:24         ` Dmitry Osipenko
2023-11-23 12:24           ` Dmitry Osipenko
2023-11-23 14:33           ` Boris Brezillon
2023-11-23 14:33             ` Boris Brezillon
2023-11-23 14:48   ` Boris Brezillon
2023-11-23 14:48     ` Boris Brezillon
2023-11-24  9:40     ` Boris Brezillon
2023-11-24  9:40       ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 16/26] drm/virtio: " Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 10:59   ` Boris Brezillon
2023-11-10 10:59     ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 17/26] drm/v3d: " Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 11:01   ` Boris Brezillon
2023-11-10 11:01     ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 18/26] drm/shmem-helper: Change sgt allocation policy Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 11:15   ` Boris Brezillon
2023-11-10 11:15     ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 19/26] drm/shmem-helper: Add common memory shrinker Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-11-10 14:58   ` Boris Brezillon [this message]
2023-11-10 14:58     ` Boris Brezillon
2023-11-13  9:35     ` Boris Brezillon
2023-11-13  9:35       ` Boris Brezillon
2023-10-29 23:01 ` [PATCH v18 20/26] drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked() Dmitry Osipenko
2023-10-29 23:01   ` Dmitry Osipenko
2023-10-29 23:02 ` [PATCH v18 21/26] drm/shmem-helper: Optimize unlocked get_pages_sgt() Dmitry Osipenko
2023-10-29 23:02   ` Dmitry Osipenko
2023-11-13  9:49   ` Boris Brezillon
2023-11-13  9:49     ` Boris Brezillon
2023-10-29 23:02 ` [PATCH v18 22/26] drm/shmem-helper: Don't free refcounted GEM Dmitry Osipenko
2023-10-29 23:02   ` Dmitry Osipenko
2023-11-13  9:54   ` Boris Brezillon
2023-11-13  9:54     ` Boris Brezillon
2023-11-22 22:30     ` Dmitry Osipenko
2023-11-22 22:30       ` Dmitry Osipenko
2023-11-23  9:08       ` Boris Brezillon
2023-11-23  9:08         ` Boris Brezillon
2023-11-23 12:36         ` Dmitry Osipenko
2023-11-23 12:36           ` Dmitry Osipenko
2023-10-29 23:02 ` [PATCH v18 23/26] drm/virtio: Pin display framebuffer BO Dmitry Osipenko
2023-10-29 23:02   ` Dmitry Osipenko
2023-10-29 23:02 ` [PATCH v18 24/26] drm/virtio: Attach shmem BOs dynamically Dmitry Osipenko
2023-10-29 23:02   ` Dmitry Osipenko
2023-11-13  9:57   ` Boris Brezillon
2023-11-13  9:57     ` Boris Brezillon
2023-11-22 22:37     ` Dmitry Osipenko
2023-11-22 22:37       ` Dmitry Osipenko
2023-11-22 22:41       ` Dmitry Osipenko
2023-11-22 22:41         ` Dmitry Osipenko
2023-10-29 23:02 ` [PATCH v18 25/26] drm/virtio: Support shmem shrinking Dmitry Osipenko
2023-10-29 23:02   ` Dmitry Osipenko
2023-11-03 22:55   ` Gurchetan Singh
2023-11-03 22:55     ` Gurchetan Singh
2023-11-06  2:02     ` Dmitry Osipenko
2023-11-06  2:02       ` Dmitry Osipenko
2023-10-29 23:02 ` [PATCH v18 26/26] drm/panfrost: Switch to generic memory shrinker Dmitry Osipenko
2023-10-29 23:02   ` Dmitry Osipenko
2023-11-24 10:04   ` Boris Brezillon
2023-11-24 10:04     ` Boris Brezillon

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=20231110155858.4c92f05d@collabora.com \
    --to=boris.brezillon@collabora.com \
    --cc=airlied@gmail.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.osipenko@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emma@anholt.net \
    --cc=gurchetansingh@chromium.org \
    --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=mwen@igalia.com \
    --cc=olvaffe@gmail.com \
    --cc=steven.price@arm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.