From: Boris Brezillon <boris.brezillon@collabora.com>
To: Akash Goel <akash.goel@arm.com>
Cc: "Steven Price" <steven.price@arm.com>,
"Liviu Dudau" <liviu.dudau@arm.com>,
"Adrián Larumbe" <adrian.larumbe@collabora.com>,
dri-devel@lists.freedesktop.org,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Rob Clark" <robin.clark@oss.qualcomm.com>,
"Sean Paul" <sean@poorly.run>,
"Konrad Dybcio" <konradybcio@kernel.org>,
"Akhil P Oommen" <akhilpo@oss.qualcomm.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
"Chris Diamand" <chris.diamand@arm.com>,
"Danilo Krummrich" <dakr@kernel.org>,
"Matthew Brost" <matthew.brost@intel.com>,
"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
"Alice Ryhl" <aliceryhl@google.com>,
kernel@collabora.com, nd@arm.com
Subject: Re: [PATCH v2 8/8] drm/panthor: Add a GEM shrinker
Date: Tue, 3 Feb 2026 09:09:12 +0100 [thread overview]
Message-ID: <20260203090912.10b4e6bc@fedora> (raw)
In-Reply-To: <e44be9f6-b7d8-4586-a72f-21e704b8579e@arm.com>
On Mon, 2 Feb 2026 20:08:30 +0000
Akash Goel <akash.goel@arm.com> wrote:
> >> +bool panthor_gem_try_evict(struct drm_gem_object *obj,
> >> + struct ww_acquire_ctx *ticket)
> >
> > I think this could be static - I don't see any reference outside this
> > file (other than the header).
Will do.
> >
> >> +{
> >> + /*
> >> + * Track last locked entry for unwinding locks in error and
> >> + * success paths
> >> + */
> >> + struct panthor_gem_object *bo = to_panthor_bo(obj);
> >> + struct drm_gpuvm_bo *vm_bo, *last_locked = NULL;
> >> + enum panthor_gem_reclaim_state old_state;
> >> + int ret = 0;
> >> +
> >> + /* To avoid potential lock ordering issue between bo_gpuva and
> >> + * mapping->i_mmap_rwsem, unmap the pages from CPU side before
> >> + * acquring the bo_gpuva lock. As the bo_resv lock is held, CPU
> >> + * page fault handler won't be able to map in the pages whilst
> >> + * eviction is in progress.
> >> + */
> >> + drm_vma_node_unmap(&bo->base.vma_node, bo->base.dev->anon_inode->i_mapping);
> >
> > There might be an issue here - drm_gem_lru_scan() will have taken the
> > resv lock. drm_vma_node_unmap() could cause a callback to
> > panthor_vm_close(). If that ends up being the last reference to
> > bo->cmap.mmap_count then we'll deadlock attempting to aquire the resv
> > lock again.
>
> Actually drm_vma_node_unmap() would just invalidate the CPU PTEs.
> The CPU mapping won't be removed and so panthor_vm_close() won't get called.
Yep, that's also my understanding of drm_vma_node_unmap(): it kills the
relevant PTEs in the user VM, but leave the VMA active, so next time
there's an access, the fault handler will be called.
>
> >
> > I not 100% on that, and sadly it seems my test setup has died so I can't
> > test that out today.
> >
>
> We have tests that tries to trigger an evicition for a CPU mapped BO and
> so far we didn't see a deadlock problem.
Actually, that's one of the very few tests I have in my igt branch [1],
and it was passing fine.
[1]https://gitlab.freedesktop.org/bbrezillon/igt-gpu-tools/-/commit/fc76934a5579767d2aabe787d40e38a17c3f4ea4#67d3c5d7df01192b03c20b43ad33249c663a95f5_80_97
next prev parent reply other threads:[~2026-02-03 8:09 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-02 11:35 [PATCH v2 0/8] drm/panthor: Add a GEM shrinker Boris Brezillon
2026-02-02 11:36 ` [PATCH v2 1/8] drm/gem: Consider GEM object reclaimable if shrinking fails Boris Brezillon
2026-02-02 16:05 ` Steven Price
2026-02-02 11:36 ` [PATCH v2 2/8] drm/panthor: Move panthor_gems_debugfs_init() to panthor_gem.c Boris Brezillon
2026-02-02 11:36 ` [PATCH v2 3/8] drm/panthor: Group panthor_kernel_bo_xxx() helpers Boris Brezillon
2026-02-02 11:36 ` [PATCH v2 4/8] drm/panthor: Part ways with drm_gem_shmem_object Boris Brezillon
2026-02-02 16:35 ` Steven Price
2026-02-02 16:51 ` Boris Brezillon
2026-02-02 11:36 ` [PATCH v2 5/8] drm/panthor: Lazily allocate pages on mmap() Boris Brezillon
2026-02-02 16:40 ` Steven Price
2026-02-04 13:29 ` Liviu Dudau
2026-02-02 11:36 ` [PATCH v2 6/8] drm/panthor: Split panthor_vm_prepare_map_op_ctx() to prepare for reclaim Boris Brezillon
2026-02-02 11:36 ` [PATCH v2 7/8] drm/panthor: Track the number of mmap on a BO Boris Brezillon
2026-02-02 16:48 ` Steven Price
2026-02-02 16:52 ` Boris Brezillon
2026-02-02 11:36 ` [PATCH v2 8/8] drm/panthor: Add a GEM shrinker Boris Brezillon
2026-02-02 17:09 ` Steven Price
2026-02-02 20:08 ` Akash Goel
2026-02-03 8:09 ` Boris Brezillon [this message]
2026-02-04 10:24 ` Steven Price
2026-02-04 13:32 ` Liviu Dudau
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=20260203090912.10b4e6bc@fedora \
--to=boris.brezillon@collabora.com \
--cc=adrian.larumbe@collabora.com \
--cc=airlied@gmail.com \
--cc=akash.goel@arm.com \
--cc=akhilpo@oss.qualcomm.com \
--cc=aliceryhl@google.com \
--cc=chris.diamand@arm.com \
--cc=dakr@kernel.org \
--cc=dmitry.osipenko@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=kernel@collabora.com \
--cc=konradybcio@kernel.org \
--cc=liviu.dudau@arm.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthew.brost@intel.com \
--cc=mripard@kernel.org \
--cc=nd@arm.com \
--cc=robin.clark@oss.qualcomm.com \
--cc=sean@poorly.run \
--cc=simona@ffwll.ch \
--cc=steven.price@arm.com \
--cc=thomas.hellstrom@linux.intel.com \
--cc=tzimmermann@suse.de \
/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.