* [PATCH] drm/amdgpu: band aid validating VM PTs
@ 2018-06-19 12:57 Christian König
[not found] ` <20180619125700.55218-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-06-19 12:57 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Always validating the VM PTs takes to much time. Only always validate
the per VM BOs for now.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 819949418495..7c30451ba897 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1082,7 +1082,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
struct amdgpu_vm_bo_base,
vm_status);
bo_base->moved = false;
- list_move(&bo_base->vm_status, &vm->idle);
+ list_del_init(&bo_base->vm_status);
bo = bo_base->bo->parent;
if (!bo)
--
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] 6+ messages in thread[parent not found: <20180619125700.55218-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: band aid validating VM PTs [not found] ` <20180619125700.55218-1-christian.koenig-5C7GfCeVMHo@public.gmane.org> @ 2018-06-20 11:25 ` Huang Rui 2018-06-20 14:45 ` Christian König 2018-06-22 0:52 ` Zhang, Jerry (Junwei) 1 sibling, 1 reply; 6+ messages in thread From: Huang Rui @ 2018-06-20 11:25 UTC (permalink / raw) To: Christian König; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Tue, Jun 19, 2018 at 02:57:00PM +0200, Christian König wrote: > Always validating the VM PTs takes to much time. Only always validate > the per VM BOs for now. Christian, you delete PTE BO instead of moving them into idle list. The intention is to avoid them do evction back when do vm_validate_pt_bos, right? But in that function, I just see that it will walk over the idle list and move the bo into lru list, and didn't find the evction behaviour called explicitly. So why will it save the performance cost? Thanks, Ray > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 819949418495..7c30451ba897 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1082,7 +1082,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev, > struct amdgpu_vm_bo_base, > vm_status); > bo_base->moved = false; > - list_move(&bo_base->vm_status, &vm->idle); > + list_del_init(&bo_base->vm_status); > > bo = bo_base->bo->parent; > if (!bo) > -- > 2.14.1 > > _______________________________________________ > 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] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: band aid validating VM PTs 2018-06-20 11:25 ` Huang Rui @ 2018-06-20 14:45 ` Christian König [not found] ` <e2bbcdbe-d35a-490f-2e15-1a9096add20d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Christian König @ 2018-06-20 14:45 UTC (permalink / raw) To: Huang Rui; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 20.06.2018 um 13:25 schrieb Huang Rui: > On Tue, Jun 19, 2018 at 02:57:00PM +0200, Christian König wrote: >> Always validating the VM PTs takes to much time. Only always validate >> the per VM BOs for now. > Christian, you delete PTE BO instead of moving them into idle list. The > intention is to avoid them do evction back when do vm_validate_pt_bos, > right? No, the intention is to avoid the extra command submission overhead with moving them on the LRU. This messes the LRU for VMs up a bit, but that shouldn't matter much because when we need to evict PTs the performance is not relevant anymore anyway. Christian. > > But in that function, I just see that it will walk over the idle list and > move the bo into lru list, and didn't find the evction behaviour called > explicitly. So why will it save the performance cost? > > Thanks, > Ray > >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 819949418495..7c30451ba897 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1082,7 +1082,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev, >> struct amdgpu_vm_bo_base, >> vm_status); >> bo_base->moved = false; >> - list_move(&bo_base->vm_status, &vm->idle); >> + list_del_init(&bo_base->vm_status); >> >> bo = bo_base->bo->parent; >> if (!bo) >> -- >> 2.14.1 >> >> _______________________________________________ >> 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] 6+ messages in thread
[parent not found: <e2bbcdbe-d35a-490f-2e15-1a9096add20d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: band aid validating VM PTs [not found] ` <e2bbcdbe-d35a-490f-2e15-1a9096add20d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2018-06-21 13:15 ` Huang Rui 2018-06-21 14:57 ` Christian König 0 siblings, 1 reply; 6+ messages in thread From: Huang Rui @ 2018-06-21 13:15 UTC (permalink / raw) To: Koenig, Christian Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org On Wed, Jun 20, 2018 at 10:45:44PM +0800, Christian König wrote: > Am 20.06.2018 um 13:25 schrieb Huang Rui: > > On Tue, Jun 19, 2018 at 02:57:00PM +0200, Christian König wrote: > >> Always validating the VM PTs takes to much time. Only always validate > >> the per VM BOs for now. > > Christian, you delete PTE BO instead of moving them into idle list. The > > intention is to avoid them do evction back when do vm_validate_pt_bos, > > right? > > No, the intention is to avoid the extra command submission overhead with > moving them on the LRU. > Thanks. I only see it's to decrease bo numbers in the lru list (man->lru). Is the extra command submission overhead caused by many times of evcition or bo validation cost (like the purpose of per-vm that to decrease the work of bo list parser). Thanks, Ray > This messes the LRU for VMs up a bit, but that shouldn't matter much > because when we need to evict PTs the performance is not relevant > anymore anyway. > > Christian. > > > > > But in that function, I just see that it will walk over the idle list and > > move the bo into lru list, and didn't find the evction behaviour called > > explicitly. So why will it save the performance cost? > > > > Thanks, > > Ray > > > >> Signed-off-by: Christian König <christian.koenig@amd.com> > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >> index 819949418495..7c30451ba897 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >> @@ -1082,7 +1082,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev, > >> struct amdgpu_vm_bo_base, > >> vm_status); > >> bo_base->moved = false; > >> - list_move(&bo_base->vm_status, &vm->idle); > >> + list_del_init(&bo_base->vm_status); > >> > >> bo = bo_base->bo->parent; > >> if (!bo) > >> -- > >> 2.14.1 > >> > >> _______________________________________________ > >> 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] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: band aid validating VM PTs 2018-06-21 13:15 ` Huang Rui @ 2018-06-21 14:57 ` Christian König 0 siblings, 0 replies; 6+ messages in thread From: Christian König @ 2018-06-21 14:57 UTC (permalink / raw) To: Huang Rui, Koenig, Christian Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Am 21.06.2018 um 15:15 schrieb Huang Rui: > On Wed, Jun 20, 2018 at 10:45:44PM +0800, Christian König wrote: >> Am 20.06.2018 um 13:25 schrieb Huang Rui: >>> On Tue, Jun 19, 2018 at 02:57:00PM +0200, Christian König wrote: >>>> Always validating the VM PTs takes to much time. Only always validate >>>> the per VM BOs for now. >>> Christian, you delete PTE BO instead of moving them into idle list. The >>> intention is to avoid them do evction back when do vm_validate_pt_bos, >>> right? >> No, the intention is to avoid the extra command submission overhead with >> moving them on the LRU. >> > Thanks. I only see it's to decrease bo numbers in the lru list (man->lru). Well, it doesn't decrease the number of BOs on the LRU list, but rather the number of times we move the BOs on the LRU. > Is the extra command submission overhead caused by many times of evcition > or bo validation cost (like the purpose of per-vm that to decrease the work > of bo list parser). Neither, it was caused by the many BOs we moved to the end of the LRU. I should probably take another look into bulk moving them on the LRU instead of one by one. Christian. > > Thanks, > Ray > >> This messes the LRU for VMs up a bit, but that shouldn't matter much >> because when we need to evict PTs the performance is not relevant >> anymore anyway. >> >> Christian. >> >>> But in that function, I just see that it will walk over the idle list and >>> move the bo into lru list, and didn't find the evction behaviour called >>> explicitly. So why will it save the performance cost? >>> >>> Thanks, >>> Ray >>> >>>> Signed-off-by: Christian König <christian.koenig@amd.com> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> index 819949418495..7c30451ba897 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> @@ -1082,7 +1082,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev, >>>> struct amdgpu_vm_bo_base, >>>> vm_status); >>>> bo_base->moved = false; >>>> - list_move(&bo_base->vm_status, &vm->idle); >>>> + list_del_init(&bo_base->vm_status); >>>> >>>> bo = bo_base->bo->parent; >>>> if (!bo) >>>> -- >>>> 2.14.1 >>>> >>>> _______________________________________________ >>>> 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] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: band aid validating VM PTs [not found] ` <20180619125700.55218-1-christian.koenig-5C7GfCeVMHo@public.gmane.org> 2018-06-20 11:25 ` Huang Rui @ 2018-06-22 0:52 ` Zhang, Jerry (Junwei) 1 sibling, 0 replies; 6+ messages in thread From: Zhang, Jerry (Junwei) @ 2018-06-22 0:52 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 06/19/2018 08:57 PM, Christian König wrote: > Always validating the VM PTs takes to much time. Only always validate > the per VM BOs for now. > > Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 819949418495..7c30451ba897 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1082,7 +1082,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev, > struct amdgpu_vm_bo_base, > vm_status); > bo_base->moved = false; > - list_move(&bo_base->vm_status, &vm->idle); > + list_del_init(&bo_base->vm_status); > > bo = bo_base->bo->parent; > if (!bo) > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-22 0:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-19 12:57 [PATCH] drm/amdgpu: band aid validating VM PTs Christian König
[not found] ` <20180619125700.55218-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-06-20 11:25 ` Huang Rui
2018-06-20 14:45 ` Christian König
[not found] ` <e2bbcdbe-d35a-490f-2e15-1a9096add20d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-06-21 13:15 ` Huang Rui
2018-06-21 14:57 ` Christian König
2018-06-22 0:52 ` 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.