[Intel-xe] [PATCH] drm/xe: destroy clients engine and vm xarrays on close

[Lucas De Marchi]

On Thu, Apr 27, 2023 at 04:45:56PM +0300, Mika Kuoppala wrote:
>Lucas De Marchi <lucas.demarchi at intel.com> writes:
>
>> On Wed, Apr 12, 2023 at 02:09:23PM +0300, Mika Kuoppala wrote:
>>>xe_file_close cleanups the xarrays but forgets
>>>to destroy them causing a memleak in xarray internals.
>>>Found with kmemleak.
>>>
>>>Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>>>---
>>> drivers/gpu/drm/xe/xe_device.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>>index 45d6e5ff47fd..30415015db4a 100644
>>>--- a/drivers/gpu/drm/xe/xe_device.c
>>>+++ b/drivers/gpu/drm/xe/xe_device.c
>>>@@ -70,6 +70,7 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>>> 		xe_engine_put(e);
>>> 	}
>>> 	mutex_unlock(&xef->engine.lock);
>>>+	xa_destroy(&xef->engine.xa);
>>> 	mutex_destroy(&xef->engine.lock);
>>> 	device_kill_persistent_engines(xe, xef);
>>>
>>>@@ -77,6 +78,7 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>>> 	xa_for_each(&xef->vm.xa, idx, vm)
>>> 		xe_vm_close_and_put(vm);
>>> 	mutex_unlock(&xef->vm.lock);
>>>+	xa_destroy(&xef->vm.xa);
>>
>> looks good to me considering the xe_file_open() initialized it. But
>> shouldn't we print a warning or something on !xa_empty()?
>
>The xe_vm_close_and_put() does not remove from the list.
>If I add warn here, it gets triggered.
>So some work is needed on vm side if we want to enforce
>it being empty at this point.

ok, I think we can keep it like this.


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi