All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: fix list not initialized
@ 2018-04-18 10:37 Chunming Zhou
       [not found] ` <20180418103710.17421-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Chunming Zhou @ 2018-04-18 10:37 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w, Chunming Zhou

Otherwise, cpu stuck for 22s with kernel panic.

Change-Id: I5b87cde662a4658c9ab253ba88d009c9628a44ca
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f0fbc331aa30..7131ad13c5b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1563,10 +1563,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 		 * the evicted list so that it gets validated again on the
 		 * next command submission.
 		 */
+		list_del_init(&bo_va->base.vm_status);
 		if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type)))
 			list_add_tail(&bo_va->base.vm_status, &vm->evicted);
-		else
-			list_del_init(&bo_va->base.vm_status);
 	} else {
 		list_del_init(&bo_va->base.vm_status);
 	}
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: fix list not initialized
       [not found] ` <20180418103710.17421-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-18 11:17   ` Christian König
  2018-04-19  1:48   ` Zhang, Jerry (Junwei)
  1 sibling, 0 replies; 5+ messages in thread
From: Christian König @ 2018-04-18 11:17 UTC (permalink / raw)
  To: Chunming Zhou, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 18.04.2018 um 12:37 schrieb Chunming Zhou:
> Otherwise, cpu stuck for 22s with kernel panic.
>
> Change-Id: I5b87cde662a4658c9ab253ba88d009c9628a44ca
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f0fbc331aa30..7131ad13c5b5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1563,10 +1563,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   		 * the evicted list so that it gets validated again on the
>   		 * next command submission.
>   		 */
> +		list_del_init(&bo_va->base.vm_status);
>   		if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type)))
>   			list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> -		else
> -			list_del_init(&bo_va->base.vm_status);

Good catch, but I think I would prefer to replace list_add_tail() with 
list_move_tail() instead of moving the list_del_init().

But just a nit pick. Either way the patch is Reviewed-by: Christian 
König <christian.koenig@amd.com>.

Regards,
Christian.

>   	} else {
>   		list_del_init(&bo_va->base.vm_status);
>   	}

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: fix list not initialized
       [not found] ` <20180418103710.17421-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
  2018-04-18 11:17   ` Christian König
@ 2018-04-19  1:48   ` Zhang, Jerry (Junwei)
       [not found]     ` <5AD7F558.5060105-5C7GfCeVMHo@public.gmane.org>
  1 sibling, 1 reply; 5+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-04-19  1:48 UTC (permalink / raw)
  To: Chunming Zhou, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w

On 04/18/2018 06:37 PM, Chunming Zhou wrote:
> Otherwise, cpu stuck for 22s with kernel panic.
>
> Change-Id: I5b87cde662a4658c9ab253ba88d009c9628a44ca
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f0fbc331aa30..7131ad13c5b5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1563,10 +1563,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   		 * the evicted list so that it gets validated again on the
>   		 * next command submission.
>   		 */
> +		list_del_init(&bo_va->base.vm_status);
>   		if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type)))
>   			list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> -		else
> -			list_del_init(&bo_va->base.vm_status);
>   	} else {
>   		list_del_init(&bo_va->base.vm_status);
>   	}
We may simplify the logic as below.
How do you think?

         list_del_init(&bo_va->base.vm_status);
         unsigned mem_type = bo->tbo.mem.mem_type;
         /* If the BO is not in its preferred location add it back to
          * the evicted list so that it gets validated again on the
          * next command submission.
          */
         if ((bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) &&
                 (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type))))
                         list_add_tail(&bo_va->base.vm_status, &vm->evicted);

Jerry

>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: fix list not initialized
       [not found]     ` <5AD7F558.5060105-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-19  2:30       ` zhoucm1
       [not found]         ` <337da6ff-3950-2dad-4c06-b5d72b3ff397-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: zhoucm1 @ 2018-04-19  2:30 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW



On 2018年04月19日 09:48, Zhang, Jerry (Junwei) wrote:
> On 04/18/2018 06:37 PM, Chunming Zhou wrote:
>> Otherwise, cpu stuck for 22s with kernel panic.
>>
>> Change-Id: I5b87cde662a4658c9ab253ba88d009c9628a44ca
>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index f0fbc331aa30..7131ad13c5b5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1563,10 +1563,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
>> *adev,
>>            * the evicted list so that it gets validated again on the
>>            * next command submission.
>>            */
>> +        list_del_init(&bo_va->base.vm_status);
>>           if (!(bo->preferred_domains & 
>> amdgpu_mem_type_to_domain(mem_type)))
>>               list_add_tail(&bo_va->base.vm_status, &vm->evicted);
>> -        else
>> -            list_del_init(&bo_va->base.vm_status);
>>       } else {
>>           list_del_init(&bo_va->base.vm_status);
>>       }
> We may simplify the logic as below.
> How do you think?
>
>         list_del_init(&bo_va->base.vm_status);
>         unsigned mem_type = bo->tbo.mem.mem_type;
>         /* If the BO is not in its preferred location add it back to
>          * the evicted list so that it gets validated again on the
>          * next command submission.
>          */
>         if ((bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) &&
>                 (!(bo->preferred_domains & 
> amdgpu_mem_type_to_domain(mem_type))))
> list_add_tail(&bo_va->base.vm_status, &vm->evicted);
Looks good, but I already pushed that patch just now. if you like, you 
can make a simplify patch with your idea.

Regards,
David Zhou
>
> Jerry
>
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: fix list not initialized
       [not found]         ` <337da6ff-3950-2dad-4c06-b5d72b3ff397-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-19  4:32           ` Zhang, Jerry (Junwei)
  0 siblings, 0 replies; 5+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-04-19  4:32 UTC (permalink / raw)
  To: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 04/19/2018 10:30 AM, zhoucm1 wrote:
>
>
> On 2018年04月19日 09:48, Zhang, Jerry (Junwei) wrote:
>> On 04/18/2018 06:37 PM, Chunming Zhou wrote:
>>> Otherwise, cpu stuck for 22s with kernel panic.
>>>
>>> Change-Id: I5b87cde662a4658c9ab253ba88d009c9628a44ca
>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index f0fbc331aa30..7131ad13c5b5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1563,10 +1563,9 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>>            * the evicted list so that it gets validated again on the
>>>            * next command submission.
>>>            */
>>> +        list_del_init(&bo_va->base.vm_status);
>>>           if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type)))
>>>               list_add_tail(&bo_va->base.vm_status, &vm->evicted);
>>> -        else
>>> -            list_del_init(&bo_va->base.vm_status);
>>>       } else {
>>>           list_del_init(&bo_va->base.vm_status);
>>>       }
>> We may simplify the logic as below.
>> How do you think?
>>
>>         list_del_init(&bo_va->base.vm_status);
>>         unsigned mem_type = bo->tbo.mem.mem_type;
>>         /* If the BO is not in its preferred location add it back to
>>          * the evicted list so that it gets validated again on the
>>          * next command submission.
>>          */
>>         if ((bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) &&
>>                 (!(bo->preferred_domains & amdgpu_mem_type_to_domain(mem_type))))
>> list_add_tail(&bo_va->base.vm_status, &vm->evicted);
> Looks good, but I already pushed that patch just now. if you like, you can make
> a simplify patch with your idea.

Sure, I will.

Jerry

>
> Regards,
> David Zhou
>>
>> Jerry
>>
>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-19  4:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-18 10:37 [PATCH] drm/amdgpu: fix list not initialized Chunming Zhou
     [not found] ` <20180418103710.17421-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
2018-04-18 11:17   ` Christian König
2018-04-19  1:48   ` Zhang, Jerry (Junwei)
     [not found]     ` <5AD7F558.5060105-5C7GfCeVMHo@public.gmane.org>
2018-04-19  2:30       ` zhoucm1
     [not found]         ` <337da6ff-3950-2dad-4c06-b5d72b3ff397-5C7GfCeVMHo@public.gmane.org>
2018-04-19  4:32           ` Zhang, Jerry (Junwei)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.