[PATCH v3 3/6] drm/amdgpu: delay the use of amdgpu_vm_set_task_info

[Pierre-Eric Pelloux-Prayer]

Le 24/09/2024 à 10:43, Tvrtko Ursulin a écrit :
> 
> On 24/09/2024 09:23, Christian König wrote:
>> Am 23.09.24 um 12:25 schrieb Tvrtko Ursulin:
>>>
>>> On 20/09/2024 10:06, Pierre-Eric Pelloux-Prayer wrote:
>>>> At this point the vm is locked so we safely modify it without risk of
>>>> concurrent access.
>>>
>>> To which particular lock this is referring to and does this imply previous placement was unsafe?
>>
>> We use the root PDs dma_resv object as VM lock to protect most field inside the VM structure, only 
>> a few are protected by an additional spinlock.
>>
>> And yes, previously it was possible that you got a mangled process/task name because no lock was 
>> protecting the task_info structure.
> 
> Got it, thanks Christian!
> 
> In this case I only suggest to be more explicit in the commit message and clearly say it is fixing 
> an existing bug. Like it stands I wasn't sure if it was that, or the movement was just enabling the 
> changes which come later in the series.

Good idea, will do.

Pierre-Eric

> 
> Regards,
> 
> Tvrtko
> 
>>>> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 +++---
>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> index 1e475eb01417..891128ecee6d 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> @@ -309,9 +309,6 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
>>>>           p->gang_leader->uf_addr = uf_offset;
>>>>       kvfree(chunk_array);
>>>>   -    /* Use this opportunity to fill in task info for the vm */
>>>> -    amdgpu_vm_set_task_info(vm);
>>>> -
>>>>       return 0;
>>>>     free_all_kdata:
>>>> @@ -1180,6 +1177,9 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>>>>           job->vm_pd_addr = amdgpu_gmc_pd_addr(vm->root.bo);
>>>>       }
>>>>   +    /* Use this opportunity to fill in task info for the vm */
>>>> +    amdgpu_vm_set_task_info(vm);
>>>> +
>>>>       if (adev->debug_vm) {
>>>>           /* Invalidate all BOs to test for userspace bugs */
>>>>           amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>>