Re: [PATCH] drm/xe: Remove NULL check of lrc->bo in xe_lrc_snapshot_capture()

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

On 8/22/2024 7:23 PM, Rodrigo Vivi wrote:
> On Tue, Aug 20, 2024 at 11:43:12AM +0200, Nirmoy Das wrote:
>> Hi Rodrigo,
>>
>> On 8/16/2024 4:00 PM, Rodrigo Vivi wrote:
>>> On Fri, Aug 16, 2024 at 01:33:55PM +0530, apoorva.singh@intel.com wrote:
>>>> From: Apoorva Singh <apoorva.singh@intel.com>
>>>>
>>>> - lrc->bo NULL check is not needed in xe_lrc_snapshot_capture() as
>>>>     its already been taken care of in xe_lrc_init().
>>> I'm afraid this is not a good reason.
>>> This snapshot capture is coming from other places and apparently
>>> with risks of paths where bo was already freed?!
>>
>> After taking another look, I just realized the lrc bo is always pinned and
>> as far as I see, only place we unpin and free it,  is when we destroy the
>> lrc.
>>
>> Am I missing any condition when xe_lrc_snapshot_capture() can be called with
>> lrc->bo freed ?
> if you are confident we don't need it, let's kill...
>
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


Merged this to drm-xe-next.


Thanks,

Nirmoy

>
>>
>> Regards,
>>
>> Nirmoy
>>
>>
>>> why is this check bothering you so much?
>>>
>>>> Signed-off-by: Apoorva Singh <apoorva.singh@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_lrc.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>>>> index 974a9cd8c379..aec7db39c061 100644
>>>> --- a/drivers/gpu/drm/xe/xe_lrc.c
>>>> +++ b/drivers/gpu/drm/xe/xe_lrc.c
>>>> @@ -1649,7 +1649,7 @@ struct xe_lrc_snapshot *xe_lrc_snapshot_capture(struct xe_lrc *lrc)
>>>>    	if (!snapshot)
>>>>    		return NULL;
>>>> -	if (lrc->bo && lrc->bo->vm)
>>>> +	if (lrc->bo->vm)
>>>>    		xe_vm_get(lrc->bo->vm);
>>>>    	snapshot->context_desc = xe_lrc_ggtt_addr(lrc);
>>>> -- 
>>>> 2.34.1
>>>>