Re: [PATCH] drm/xe: Fix memory leak on xe_alloc_pf_queue failure

[Nirmoy Das <nirmoy.das@linux.intel.com>]

On 8/22/2024 10:52 AM, Matthew Auld wrote:
> On 22/08/2024 09:13, Nirmoy Das wrote:
>> Free up previously allocated pf_queue[i].data on error.
>>
>> Fixes: 3338e4f90c14 ("drm/xe: Use topology to determine page fault 
>> queue size")
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Stuart Summers <stuart.summers@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt_pagefault.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c 
>> b/drivers/gpu/drm/xe/xe_gt_pagefault.c
>> index 0be4687bfc20..c19944eed5bd 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
>> @@ -437,9 +437,13 @@ int xe_gt_pagefault_init(struct xe_gt *gt)
>>         for (i = 0; i < NUM_PF_QUEUE; ++i) {
>>           ret = xe_alloc_pf_queue(gt, &gt->usm.pf_queue[i]);
>> -        if (ret)
>> +        if (ret) {
>> +            while (i-- > 0)
>> +                kfree(gt->usm.pf_queue[i].data);
>>               return ret;
>> +        }
>>       }
>
> I think this will then also leak below, if one of the queue create 
> fails? Maybe just convert this over to devm_calloc or similar that way 
> we don't need to do the manual unwind.

Ah, right! let me look into managed calloc way.

Nirmoy


>
>> +
>>       for (i = 0; i < NUM_ACC_QUEUE; ++i) {
>>           gt->usm.acc_queue[i].gt = gt;
>>           spin_lock_init(&gt->usm.acc_queue[i].lock);