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

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

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 ?


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