All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Steven Price <steven.price@arm.com>
Cc: "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>,
	"Akash Goel" <akash.goel@arm.com>,
	"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
Subject: Re: [PATCH v2 7/8] drm/panthor: Track the number of mmap on a BO
Date: Mon, 2 Feb 2026 17:52:36 +0100	[thread overview]
Message-ID: <20260202175236.0e26e671@fedora> (raw)
In-Reply-To: <ffa32c4e-845a-467f-a1ff-064cc9f30b70@arm.com>

On Mon, 2 Feb 2026 16:48:41 +0000
Steven Price <steven.price@arm.com> wrote:

> On 02/02/2026 11:36, Boris Brezillon wrote:
> > This will be used to order things by reclaimability.
> > 
> > v2:
> > - Fix refcounting
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  drivers/gpu/drm/panthor/panthor_gem.c | 44 +++++++++++++++++++++++++--
> >  drivers/gpu/drm/panthor/panthor_gem.h |  3 ++
> >  2 files changed, 45 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> > index 7e966fbe500f..26fe4be10a86 100644
> > --- a/drivers/gpu/drm/panthor/panthor_gem.c
> > +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> > @@ -491,6 +491,7 @@ static void panthor_gem_print_info(struct drm_printer *p, unsigned int indent,
> >  	drm_printf_indent(p, indent, "vmap_use_count=%u\n",
> >  			  refcount_read(&bo->cmap.vaddr_use_count));
> >  	drm_printf_indent(p, indent, "vaddr=%p\n", bo->cmap.vaddr);
> > +	drm_printf_indent(p, indent, "mmap_count=%u\n", refcount_read(&bo->cmap.mmap_count));
> >  }
> >  
> >  static int panthor_gem_pin_locked(struct drm_gem_object *obj)
> > @@ -603,6 +604,12 @@ static int panthor_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *v
> >  	if (is_cow_mapping(vma->vm_flags))
> >  		return -EINVAL;
> >  
> > +	if (!refcount_inc_not_zero(&bo->cmap.mmap_count)) {
> > +		dma_resv_lock(obj->resv, NULL);
> > +		refcount_set(&bo->cmap.mmap_count, 1);  
> 
> I think you still need to recheck the refcount with the lock held.
> Otherwise two threads could race:
> 
>  Thread 1			| Thread 2
>  -------------------------------+--------------------------
>  if (!refcount_inc_not_zero())  |
> 	<pre-empted>		|
> 				| if (!refcount_inc_not_zero())
> 				| dma_resv_lock()
> 				| refcount_set(..., 1)
> 				| dma_resv_unlock()
>  dma_resv_lock()		|
>  refcount_set(..., 1)		|
>  dma_resv_unlock()		|
> 
> Which leaves a refcount missing.

Uh, right. I fixed that pattern in multiple places but somehow
overlooked this one. Will fix in v3.

Thanks!

  reply	other threads:[~2026-02-02 16:52 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 [this message]
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
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=20260202175236.0e26e671@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=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.