[PATCH] drm/xe: Make the gem shrinker drm managed

[Thomas Hellström]

On Thu, 2025-05-08 at 12:50 +0100, Matthew Auld wrote:
> On 08/05/2025 12:30, Thomas Hellström wrote:
> > Make the xe drm shrinker drm managed like many other resources
> > created at device creation time.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_device.c   |  9 +++-----
> >   drivers/gpu/drm/xe/xe_shrinker.c | 37 ++++++++++++++++-----------
> > -----
> >   drivers/gpu/drm/xe/xe_shrinker.h |  4 +---
> >   3 files changed, 22 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > b/drivers/gpu/drm/xe/xe_device.c
> > index c02c4c4e9412..399ae5f40321 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -402,9 +402,6 @@ static void xe_device_destroy(struct drm_device
> > *dev, void *dummy)
> >   	if (xe->unordered_wq)
> >   		destroy_workqueue(xe->unordered_wq);
> >   
> > -	if (!IS_ERR_OR_NULL(xe->mem.shrinker))
> > -		xe_shrinker_destroy(xe->mem.shrinker);
> > -
> >   	if (xe->destroy_wq)
> >   		destroy_workqueue(xe->destroy_wq);
> >   
> > @@ -438,9 +435,9 @@ struct xe_device *xe_device_create(struct
> > pci_dev *pdev,
> >   	if (err)
> >   		goto err;
> >   
> > -	xe->mem.shrinker = xe_shrinker_create(xe);
> > -	if (IS_ERR(xe->mem.shrinker))
> > -		return ERR_CAST(xe->mem.shrinker);
> > +	err = xe_shrinker_create(xe);
> > +	if (err)
> > +		goto err;
> >   
> >   	xe->info.devid = pdev->device;
> >   	xe->info.revid = pdev->revision;
> > diff --git a/drivers/gpu/drm/xe/xe_shrinker.c
> > b/drivers/gpu/drm/xe/xe_shrinker.c
> > index 86d47aaf0358..125c836e0ee4 100644
> > --- a/drivers/gpu/drm/xe/xe_shrinker.c
> > +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> > @@ -5,6 +5,7 @@
> >   
> >   #include <linux/shrinker.h>
> >   
> > +#include <drm/drm_managed.h>
> >   #include <drm/ttm/ttm_backup.h>
> >   #include <drm/ttm/ttm_bo.h>
> >   #include <drm/ttm/ttm_tt.h>
> > @@ -213,24 +214,34 @@ static void xe_shrinker_pm(struct work_struct
> > *work)
> >   	xe_pm_runtime_put(shrinker->xe);
> >   }
> >   
> > +static void xe_shrinker_fini(struct drm_device *drm, void *arg)
> > +{
> > +	struct xe_shrinker *shrinker = arg;
> > +
> > +	xe_assert(shrinker->xe, !shrinker->shrinkable_pages);
> > +	xe_assert(shrinker->xe, !shrinker->purgeable_pages);
> > +	shrinker_free(shrinker->shrink);
> > +	flush_work(&shrinker->pm_worker);
> 
> As an aside, would it make sense of us to cut over to async rpm get 
> instead of having to do something similar by hand? IIRC it also does 
> some more fancy stuff under the hood which might be beneficial.

I wasn't aware of that. I'll take a closer look, but yeah, it sounds
like it makes sense.

Thanks for reviewing.
/Thomas


> 
> Reviewed-by: Matthew Auld <matthew.auld at intel.com>
> 
> > +	kfree(shrinker);
> > +}
> > +
> >   /**
> >    * xe_shrinker_create() - Create an xe per-device shrinker
> >    * @xe: Pointer to the xe device.
> >    *
> > - * Returns: A pointer to the created shrinker on success,
> > - * Negative error code on failure.
> > + * Return: %0 on success. Negative error code on failure.
> >    */
> > -struct xe_shrinker *xe_shrinker_create(struct xe_device *xe)
> > +int xe_shrinker_create(struct xe_device *xe)
> >   {
> >   	struct xe_shrinker *shrinker = kzalloc(sizeof(*shrinker),
> > GFP_KERNEL);
> >   
> >   	if (!shrinker)
> > -		return ERR_PTR(-ENOMEM);
> > +		return -ENOMEM;
> >   
> >   	shrinker->shrink = shrinker_alloc(0, "drm-xe_gem:%s", xe-
> > >drm.unique);
> >   	if (!shrinker->shrink) {
> >   		kfree(shrinker);
> > -		return ERR_PTR(-ENOMEM);
> > +		return -ENOMEM;
> >   	}
> >   
> >   	INIT_WORK(&shrinker->pm_worker, xe_shrinker_pm);
> > @@ -240,19 +251,7 @@ struct xe_shrinker *xe_shrinker_create(struct
> > xe_device *xe)
> >   	shrinker->shrink->scan_objects = xe_shrinker_scan;
> >   	shrinker->shrink->private_data = shrinker;
> >   	shrinker_register(shrinker->shrink);
> > +	xe->mem.shrinker = shrinker;
> >   
> > -	return shrinker;
> > -}
> > -
> > -/**
> > - * xe_shrinker_destroy() - Destroy an xe per-device shrinker
> > - * @shrinker: Pointer to the shrinker to destroy.
> > - */
> > -void xe_shrinker_destroy(struct xe_shrinker *shrinker)
> > -{
> > -	xe_assert(shrinker->xe, !shrinker->shrinkable_pages);
> > -	xe_assert(shrinker->xe, !shrinker->purgeable_pages);
> > -	shrinker_free(shrinker->shrink);
> > -	flush_work(&shrinker->pm_worker);
> > -	kfree(shrinker);
> > +	return drmm_add_action_or_reset(&xe->drm,
> > xe_shrinker_fini, shrinker);
> >   }
> > diff --git a/drivers/gpu/drm/xe/xe_shrinker.h
> > b/drivers/gpu/drm/xe/xe_shrinker.h
> > index 28a038f4fcbf..5132ae5192e1 100644
> > --- a/drivers/gpu/drm/xe/xe_shrinker.h
> > +++ b/drivers/gpu/drm/xe/xe_shrinker.h
> > @@ -11,8 +11,6 @@ struct xe_device;
> >   
> >   void xe_shrinker_mod_pages(struct xe_shrinker *shrinker, long
> > shrinkable, long purgeable);
> >   
> > -struct xe_shrinker *xe_shrinker_create(struct xe_device *xe);
> > -
> > -void xe_shrinker_destroy(struct xe_shrinker *shrinker);
> > +int xe_shrinker_create(struct xe_device *xe);
> >   
> >   #endif
>