Re: [PATCH v2 2/2] drm/xe: Add a WARN_ON for NULL job in xe_sync_entry_signal

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

On 3/18/2024 6:41 PM, Michal Wajdeczko wrote:
>
> On 18.03.2024 17:43, Nirmoy Das wrote:
>> Add a warn for NULL job when sync->type is
>> DRM_XE_SYNC_TYPE_USER_FENCE. This should be a programming
>> error and should never happen so warn and let the kernel crash
>> if that ever happens.
> IMO adding WARN and then let kernel crash is pointless as you will have
> almost exactly the same report as from NPD

WARN should give us the line number which is not the case for NDP I think.


Regards,

Nirmoy

>
> for programming errors we should xe_assert() that will provide necessary
> hint during code refactor but will be compiled on production builds
>
> only if you feel that that job could be still (but unlikely) NULL then
> you should use drm_WARN/xe_gt_WARN and provide necessary fallback
>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_sync.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
>> index 02c9577fe418..247505c3478d 100644
>> --- a/drivers/gpu/drm/xe/xe_sync.c
>> +++ b/drivers/gpu/drm/xe/xe_sync.c
>> @@ -255,6 +255,7 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct xe_sched_job *job,
>>   			dma_fence_put(fence);
>>   		}
>>   	} else if (sync->type == DRM_XE_SYNC_TYPE_USER_FENCE) {
>> +		XE_WARN_ON(!job);
>>   		job->user_fence.used = true;
>>   		job->user_fence.addr = sync->addr;
>>   		job->user_fence.value = sync->timeline_value;