Re: [PATCH] drm/xe: Destroy mutex on gpu_buddy_init failure in VRAM manager init

[Matthew Auld <matthew.auld@intel.com>]

On 28/04/2026 12:40, Upadhyay, Tejas wrote:
> 
> 
>> -----Original Message-----
>> From: Auld, Matthew <matthew.auld@intel.com>
>> Sent: 28 April 2026 16:36
>> To: Upadhyay, Tejas <tejas.upadhyay@intel.com>; intel-
>> xe@lists.freedesktop.org
>> Cc: Brost, Matthew <matthew.brost@intel.com>;
>> thomas.hellstrom@linux.intel.com; Ghimiray, Himal Prasad
>> <himal.prasad.ghimiray@intel.com>
>> Subject: Re: [PATCH] drm/xe: Destroy mutex on gpu_buddy_init failure in
>> VRAM manager init
>>
>> On 28/04/2026 11:38, Matthew Auld wrote:
>>> On 28/04/2026 11:32, Tejas Upadhyay wrote:
>>>> In __xe_ttm_vram_mgr_init(), if gpu_buddy_init() fails, the function
>>>> returns early without calling mutex_destroy() on mgr->lock which was
>>>> previously initialized via mutex_init(). The normal cleanup path in
>>>> xe_ttm_vram_mgr_fini() is registered via drmm_add_action_or_reset()
>>>> only after gpu_buddy_init() succeeds, so it never runs for this error
>>>> case.
>>>>
>>>> Add mutex_destroy() before the early return to properly clean up the
>>>> initialized mutex.
>>>>
>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM buddy based VRAM
>>>> manager")
>>
>> Wait, where does this commit title come from? How did you generate this?
> 
> Took claude AI recommendation on wording.

Ok, we shouldn't make up new commits that don't exist. The fixes tag 
needs to always match a real commit, including the commit title. Also, 
AFAIK this needs to be labelled with Assisted-by: <>, to be fully 
transparent.

https://docs.kernel.org/process/coding-assistants.html

> 
> Tejas
>>
>>>> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/
>>>> xe/xe_ttm_vram_mgr.c index 5fd0d5506a7e..3189e4b7ad05 100644
>>>> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>> @@ -319,8 +319,10 @@ int __xe_ttm_vram_mgr_init(struct xe_device
>> *xe,
>>>> struct xe_ttm_vram_mgr *mgr,
>>>>        ttm_resource_manager_init(man, &xe->ttm, size);
>>>>        err = gpu_buddy_init(&mgr->mm, man->size, default_page_size);
>>>> -    if (err)
>>>> +    if (err) {
>>>> +        mutex_destroy(&mgr->lock);
>>>>            return err;
>>>> +    }
>>>
>>> Can we not use drmm_mutex_init() instead? Also can then drop the
>>> manual destroy in fini(().
>>>
>>>>        ttm_set_driver_manager(&xe->ttm, mem_type, &mgr->manager);
>>>>        ttm_resource_manager_set_used(&mgr->manager, true);
>>>
>