* [PATCH] drm/gpuvm: Let drm_gpuvm_bo_put() report when the vm_bo object is destroyed
@ 2023-12-04 15:14 Boris Brezillon
2023-12-05 1:46 ` Danilo Krummrich
0 siblings, 1 reply; 3+ messages in thread
From: Boris Brezillon @ 2023-12-04 15:14 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: Boris Brezillon, kernel, dri-devel
Some users need to release resources attached to the vm_bo object when
it's destroyed. In Panthor's case, we need to release the pin ref so
BO pages can be returned to the system when all GPU mappings are gone.
This could be done through a custom drm_gpuvm::vm_bo_free() hook, but
this has all sort of locking implications that would force us to expose
a drm_gem_shmem_unpin_locked() helper, not to mention the fact that
having a ::vm_bo_free() implementation without a ::vm_bo_alloc() one
seems odd. So let's keep things simple, and extend drm_gpuvm_bo_put()
to report when the object is destroyed.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/gpu/drm/drm_gpuvm.c | 8 ++++++--
include/drm/drm_gpuvm.h | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index 54f5e8851de5..ae13e2d63637 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -1502,14 +1502,18 @@ drm_gpuvm_bo_destroy(struct kref *kref)
* hold the dma-resv or driver specific GEM gpuva lock.
*
* This function may only be called from non-atomic context.
+ *
+ * Returns: true if vm_bo was destroyed, false otherwise.
*/
-void
+bool
drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo)
{
might_sleep();
if (vm_bo)
- kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
+ return !!kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
+
+ return false;
}
EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put);
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index f94fec9a8517..7cc41a7d86d5 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -738,7 +738,7 @@ drm_gpuvm_bo_get(struct drm_gpuvm_bo *vm_bo)
return vm_bo;
}
-void drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
+bool drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
struct drm_gpuvm_bo *
drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/gpuvm: Let drm_gpuvm_bo_put() report when the vm_bo object is destroyed
2023-12-04 15:14 [PATCH] drm/gpuvm: Let drm_gpuvm_bo_put() report when the vm_bo object is destroyed Boris Brezillon
@ 2023-12-05 1:46 ` Danilo Krummrich
2023-12-05 10:45 ` Boris Brezillon
0 siblings, 1 reply; 3+ messages in thread
From: Danilo Krummrich @ 2023-12-05 1:46 UTC (permalink / raw)
To: Boris Brezillon; +Cc: kernel, dri-devel
On 12/4/23 16:14, Boris Brezillon wrote:
> Some users need to release resources attached to the vm_bo object when
> it's destroyed. In Panthor's case, we need to release the pin ref so
> BO pages can be returned to the system when all GPU mappings are gone.
>
> This could be done through a custom drm_gpuvm::vm_bo_free() hook, but
> this has all sort of locking implications that would force us to expose
> a drm_gem_shmem_unpin_locked() helper, not to mention the fact that
> having a ::vm_bo_free() implementation without a ::vm_bo_alloc() one
> seems odd. So let's keep things simple, and extend drm_gpuvm_bo_put()
> to report when the object is destroyed.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Danilo Krummrich <dakr@redhat.com>
> ---
> drivers/gpu/drm/drm_gpuvm.c | 8 ++++++--
> include/drm/drm_gpuvm.h | 2 +-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> index 54f5e8851de5..ae13e2d63637 100644
> --- a/drivers/gpu/drm/drm_gpuvm.c
> +++ b/drivers/gpu/drm/drm_gpuvm.c
> @@ -1502,14 +1502,18 @@ drm_gpuvm_bo_destroy(struct kref *kref)
> * hold the dma-resv or driver specific GEM gpuva lock.
> *
> * This function may only be called from non-atomic context.
> + *
> + * Returns: true if vm_bo was destroyed, false otherwise.
> */
> -void
> +bool
> drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo)
> {
> might_sleep();
>
> if (vm_bo)
> - kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
> + return !!kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
> +
> + return false;
> }
> EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put);
>
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index f94fec9a8517..7cc41a7d86d5 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -738,7 +738,7 @@ drm_gpuvm_bo_get(struct drm_gpuvm_bo *vm_bo)
> return vm_bo;
> }
>
> -void drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
> +bool drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
>
> struct drm_gpuvm_bo *
> drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm/gpuvm: Let drm_gpuvm_bo_put() report when the vm_bo object is destroyed
2023-12-05 1:46 ` Danilo Krummrich
@ 2023-12-05 10:45 ` Boris Brezillon
0 siblings, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2023-12-05 10:45 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: kernel, dri-devel
On Tue, 5 Dec 2023 02:46:32 +0100
Danilo Krummrich <dakr@redhat.com> wrote:
> On 12/4/23 16:14, Boris Brezillon wrote:
> > Some users need to release resources attached to the vm_bo object when
> > it's destroyed. In Panthor's case, we need to release the pin ref so
> > BO pages can be returned to the system when all GPU mappings are gone.
> >
> > This could be done through a custom drm_gpuvm::vm_bo_free() hook, but
> > this has all sort of locking implications that would force us to expose
> > a drm_gem_shmem_unpin_locked() helper, not to mention the fact that
> > having a ::vm_bo_free() implementation without a ::vm_bo_alloc() one
> > seems odd. So let's keep things simple, and extend drm_gpuvm_bo_put()
> > to report when the object is destroyed.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
>
> Reviewed-by: Danilo Krummrich <dakr@redhat.com>
Queued to drm-misc-next.
>
> > ---
> > drivers/gpu/drm/drm_gpuvm.c | 8 ++++++--
> > include/drm/drm_gpuvm.h | 2 +-
> > 2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> > index 54f5e8851de5..ae13e2d63637 100644
> > --- a/drivers/gpu/drm/drm_gpuvm.c
> > +++ b/drivers/gpu/drm/drm_gpuvm.c
> > @@ -1502,14 +1502,18 @@ drm_gpuvm_bo_destroy(struct kref *kref)
> > * hold the dma-resv or driver specific GEM gpuva lock.
> > *
> > * This function may only be called from non-atomic context.
> > + *
> > + * Returns: true if vm_bo was destroyed, false otherwise.
> > */
> > -void
> > +bool
> > drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo)
> > {
> > might_sleep();
> >
> > if (vm_bo)
> > - kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
> > + return !!kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
> > +
> > + return false;
> > }
> > EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put);
> >
> > diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> > index f94fec9a8517..7cc41a7d86d5 100644
> > --- a/include/drm/drm_gpuvm.h
> > +++ b/include/drm/drm_gpuvm.h
> > @@ -738,7 +738,7 @@ drm_gpuvm_bo_get(struct drm_gpuvm_bo *vm_bo)
> > return vm_bo;
> > }
> >
> > -void drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
> > +bool drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
> >
> > struct drm_gpuvm_bo *
> > drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-05 10:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04 15:14 [PATCH] drm/gpuvm: Let drm_gpuvm_bo_put() report when the vm_bo object is destroyed Boris Brezillon
2023-12-05 1:46 ` Danilo Krummrich
2023-12-05 10:45 ` Boris Brezillon
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.