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

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

On 8/27/2024 11:48 AM, Nirmoy Das wrote:
>
> 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.

Merged it to dem-xe-next with removed extra line.


Thanks,

Nirmoy

>
>>
>> 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;