[PATCH 3/6] drm/amdgpu: screen freeze and userq driver crash

[Christian König]

Am 11.09.24 um 07:00 schrieb Paneer Selvam, Arunpravin:
> Hi Christian,
>
> On 9/5/2024 4:50 PM, Christian König wrote:
>> Am 30.08.24 um 20:43 schrieb Arunpravin Paneer Selvam:
>>> Screen freeze and userq fence driver crash while playing Xonotic
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam 
>>> <Arunpravin.PaneerSelvam at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 12 +++++++-----
>>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> index d2e1d0d0f29f..8a3b5374cc1a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> @@ -187,6 +187,7 @@ int amdgpu_userq_fence_create(struct 
>>> amdgpu_usermode_queue *userq,
>>>       struct amdgpu_userq_fence_driver *fence_drv;
>>>       struct amdgpu_userq_fence *userq_fence;
>>>       struct dma_fence *fence;
>>> +    unsigned long flags;
>>>         fence_drv = userq->fence_drv;
>>>       if (!fence_drv)
>>> @@ -230,14 +231,15 @@ int amdgpu_userq_fence_create(struct 
>>> amdgpu_usermode_queue *userq,
>>>           }
>>>       }
>>>   -    spin_lock(&fence_drv->fence_list_lock);
>>>       /* Check if hardware has already processed the job */
>>> -    if (!dma_fence_is_signaled(fence))
>>> +    if (!dma_fence_is_signaled(fence)) {
>>> +        spin_lock_irqsave(&fence_drv->fence_list_lock, flags);
>>
>> As far as I can see that still won't work correctly since you can't 
>> guarantee that the fence doesn't signal in between testing and 
>> grabbing the lock.
> Can I move the spin lock little up before the if .. else check.

Maybe, I need to see the full code to judge.

But keep in mind that you need to use dma_fence_is_signaled_locked() then.

Regards,
Christian.

>
> Thanks,
> Arun.
>>
>> Christian.
>>
>>
>>
>>> list_add_tail(&userq_fence->link, &fence_drv->fences);
>>> -    else
>>> -        dma_fence_put(fence);
>>>   -    spin_unlock(&fence_drv->fence_list_lock);
>>> + spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags);
>>> +    } else {
>>> +        dma_fence_put(fence);
>>> +    }
>>>         *f = fence;
>>
>