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

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

On 8/27/2024 10:48 AM, Matthew Auld wrote:
> On 26/08/2024 17:20, Nirmoy Das wrote:
>> Simplify memory unwinding on error also fixing current memory
>> leak that can happen on error.
>>
>> v2: use devm_kcalloc(Matt A)
>>
>> 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 | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c 
>> b/drivers/gpu/drm/xe/xe_gt_pagefault.c
>> index 0be4687bfc20..91810d0a9afc 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
>> @@ -388,7 +388,6 @@ static void pagefault_fini(void *arg)
>>   {
>>       struct xe_gt *gt = arg;
>>       struct xe_device *xe = gt_to_xe(gt);
>> -    int i;
>>         if (!xe->info.has_usm)
>>           return;
>> @@ -396,12 +395,11 @@ static void pagefault_fini(void *arg)
>>       destroy_workqueue(gt->usm.acc_wq);
>>       destroy_workqueue(gt->usm.pf_wq);
>
> No need for the new line here?


Me with my telescoping vision. I will remove that before merge, if that 
is fine.

>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>


Thanks,

Nirmoy

>
>> -    for (i = 0; i < NUM_PF_QUEUE; ++i)
>> -        kfree(gt->usm.pf_queue[i].data);
>>   }
>>     static int xe_alloc_pf_queue(struct xe_gt *gt, struct pf_queue 
>> *pf_queue)
>>   {
>> +    struct xe_device *xe = gt_to_xe(gt);
>>       xe_dss_mask_t all_dss;
>>       int num_dss, num_eus;
>>   @@ -417,7 +415,8 @@ static int xe_alloc_pf_queue(struct xe_gt *gt, 
>> struct pf_queue *pf_queue)
>>           (num_eus + XE_NUM_HW_ENGINES) * PF_MSG_LEN_DW;
>>         pf_queue->gt = gt;
>> -    pf_queue->data = kcalloc(pf_queue->num_dw, sizeof(u32), 
>> GFP_KERNEL);
>> +    pf_queue->data = devm_kcalloc(xe->drm.dev, pf_queue->num_dw,
>> +                      sizeof(u32), GFP_KERNEL);
>>       if (!pf_queue->data)
>>           return -ENOMEM;