* [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini @ 2017-01-13 4:11 Monk Liu [not found] ` <1484280664-22845-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Monk Liu @ 2017-01-13 4:11 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Monk Liu otherwise CSA may unmapped before gpu_scheduler scheduling jobs and trigger VM fault on CSA address Change-Id: Ib2e25ededf89bca44c764477dd2f9127024ca78c Signed-off-by: Monk Liu <Monk.Liu@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 -------- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 45484c0..e13cdde 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -694,14 +694,6 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev, amdgpu_uvd_free_handles(adev, file_priv); amdgpu_vce_free_handles(adev, file_priv); - if (amdgpu_sriov_vf(adev)) { - /* TODO: how to handle reserve failure */ - BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); - amdgpu_vm_bo_rmv(adev, fpriv->vm.csa_bo_va); - fpriv->vm.csa_bo_va = NULL; - amdgpu_bo_unreserve(adev->virt.csa_obj); - } - amdgpu_vm_fini(adev, &fpriv->vm); idr_for_each_entry(&fpriv->bo_list_handles, list, handle) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index d05546e..94098bc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1608,6 +1608,14 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) amd_sched_entity_fini(vm->entity.sched, &vm->entity); + if (amdgpu_sriov_vf(adev)) { + /* TODO: how to handle reserve failure */ + BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); + amdgpu_vm_bo_rmv(adev, vm->csa_bo_va); + vm->csa_bo_va = NULL; + amdgpu_bo_unreserve(adev->virt.csa_obj); + } + if (!RB_EMPTY_ROOT(&vm->va)) { dev_err(adev->dev, "still active bo inside vm\n"); } -- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <1484280664-22845-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini [not found] ` <1484280664-22845-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org> @ 2017-01-13 9:25 ` Christian König [not found] ` <dadee34c-ca4c-6a2a-8053-4bfdeb1466c3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Christian König @ 2017-01-13 9:25 UTC (permalink / raw) To: Monk Liu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 13.01.2017 um 05:11 schrieb Monk Liu: > otherwise CSA may unmapped before gpu_scheduler scheduling > jobs and trigger VM fault on CSA address > > Change-Id: Ib2e25ededf89bca44c764477dd2f9127024ca78c > Signed-off-by: Monk Liu <Monk.Liu@amd.com> Did you really run into an issue because of that? Calling amdgpu_vm_bo_rmv() shouldn't affect the page tables nor already submitted command submissions in any way. Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 -------- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index 45484c0..e13cdde 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -694,14 +694,6 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev, > amdgpu_uvd_free_handles(adev, file_priv); > amdgpu_vce_free_handles(adev, file_priv); > > - if (amdgpu_sriov_vf(adev)) { > - /* TODO: how to handle reserve failure */ > - BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); > - amdgpu_vm_bo_rmv(adev, fpriv->vm.csa_bo_va); > - fpriv->vm.csa_bo_va = NULL; > - amdgpu_bo_unreserve(adev->virt.csa_obj); > - } > - > amdgpu_vm_fini(adev, &fpriv->vm); > > idr_for_each_entry(&fpriv->bo_list_handles, list, handle) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index d05546e..94098bc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1608,6 +1608,14 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) > > amd_sched_entity_fini(vm->entity.sched, &vm->entity); > > + if (amdgpu_sriov_vf(adev)) { > + /* TODO: how to handle reserve failure */ > + BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); > + amdgpu_vm_bo_rmv(adev, vm->csa_bo_va); > + vm->csa_bo_va = NULL; > + amdgpu_bo_unreserve(adev->virt.csa_obj); > + } > + > if (!RB_EMPTY_ROOT(&vm->va)) { > dev_err(adev->dev, "still active bo inside vm\n"); > } _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <dadee34c-ca4c-6a2a-8053-4bfdeb1466c3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* 答复: [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini [not found] ` <dadee34c-ca4c-6a2a-8053-4bfdeb1466c3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2017-01-13 9:56 ` Liu, Monk [not found] ` <BY2PR1201MB11102468908DDFD083C45B5384780-O28G1zQ8oGliQkyLPkmea2rFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org> 0 siblings, 1 reply; 4+ messages in thread From: Liu, Monk @ 2017-01-13 9:56 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW [-- Attachment #1.1: Type: text/plain, Size: 3349 bytes --] only with amdgpu_vm_bo_rmv() won't has such bug, but in another branch for sriov, we not only call vm_bo_rmv(), and we also set csa_addr to NULL after it, so the NULL address is inserted in RB, and when preemption occured, CP backup snapshot to NULL address. although in staging-4.9 we didn't set csa_addr to NULL (because as you suggested we always use HARDCODE/MACRO for CSA address), but logically we'd better put CSA unmapping stuffs behind "sched_entity_fini", which is more reasonable ... BR Monk ________________________________ 发件人: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> 代表 Christian König <deathsimple@vodafone.de> 发送时间: 2017年1月13日 17:25:09 收件人: Liu, Monk; amd-gfx@lists.freedesktop.org 主题: Re: [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini Am 13.01.2017 um 05:11 schrieb Monk Liu: > otherwise CSA may unmapped before gpu_scheduler scheduling > jobs and trigger VM fault on CSA address > > Change-Id: Ib2e25ededf89bca44c764477dd2f9127024ca78c > Signed-off-by: Monk Liu <Monk.Liu@amd.com> Did you really run into an issue because of that? Calling amdgpu_vm_bo_rmv() shouldn't affect the page tables nor already submitted command submissions in any way. Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 -------- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index 45484c0..e13cdde 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -694,14 +694,6 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev, > amdgpu_uvd_free_handles(adev, file_priv); > amdgpu_vce_free_handles(adev, file_priv); > > - if (amdgpu_sriov_vf(adev)) { > - /* TODO: how to handle reserve failure */ > - BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); > - amdgpu_vm_bo_rmv(adev, fpriv->vm.csa_bo_va); > - fpriv->vm.csa_bo_va = NULL; > - amdgpu_bo_unreserve(adev->virt.csa_obj); > - } > - > amdgpu_vm_fini(adev, &fpriv->vm); > > idr_for_each_entry(&fpriv->bo_list_handles, list, handle) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index d05546e..94098bc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1608,6 +1608,14 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) > > amd_sched_entity_fini(vm->entity.sched, &vm->entity); > > + if (amdgpu_sriov_vf(adev)) { > + /* TODO: how to handle reserve failure */ > + BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); > + amdgpu_vm_bo_rmv(adev, vm->csa_bo_va); > + vm->csa_bo_va = NULL; > + amdgpu_bo_unreserve(adev->virt.csa_obj); > + } > + > if (!RB_EMPTY_ROOT(&vm->va)) { > dev_err(adev->dev, "still active bo inside vm\n"); > } _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx [-- Attachment #1.2: Type: text/html, Size: 6069 bytes --] [-- Attachment #2: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <BY2PR1201MB11102468908DDFD083C45B5384780-O28G1zQ8oGliQkyLPkmea2rFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>]
* Re: 答复: [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini [not found] ` <BY2PR1201MB11102468908DDFD083C45B5384780-O28G1zQ8oGliQkyLPkmea2rFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org> @ 2017-01-13 10:23 ` Christian König 0 siblings, 0 replies; 4+ messages in thread From: Christian König @ 2017-01-13 10:23 UTC (permalink / raw) To: Liu, Monk, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW [-- Attachment #1.1: Type: text/plain, Size: 4109 bytes --] Ah, in this case please separate the amdgpu_vm_bo_rmv() from setting csa_addr to NULL. Cause amdgpu_vm_bo_rmv() should come before amdgpu_vm_fini() and that in turn should become before waiting for the scheduler so that the MM knows that the memory is about to be freed. Regards, Christian. Am 13.01.2017 um 10:56 schrieb Liu, Monk: > > only with amdgpu_vm_bo_rmv() won't has such bug, but in another branch > for sriov, we not only call vm_bo_rmv(), and we also set csa_addr to > NULL after it, so the NULL address is inserted in RB, and when > preemption occured, CP backup snapshot to NULL address. > > > although in staging-4.9 we didn't set csa_addr to NULL (because as you > suggested we always use HARDCODE/MACRO for CSA address), but logically > we'd better put CSA unmapping stuffs behind "sched_entity_fini", which > is more reasonable ... > > > BR Monk > > ------------------------------------------------------------------------ > *发件人:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> 代表 Christian > König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> > *发送时间:* 2017年1月13日 17:25:09 > *收件人:* Liu, Monk; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > *主题:* Re: [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini > Am 13.01.2017 um 05:11 schrieb Monk Liu: > > otherwise CSA may unmapped before gpu_scheduler scheduling > > jobs and trigger VM fault on CSA address > > > > Change-Id: Ib2e25ededf89bca44c764477dd2f9127024ca78c > > Signed-off-by: Monk Liu <Monk.Liu-5C7GfCeVMHo@public.gmane.org> > > Did you really run into an issue because of that? > > Calling amdgpu_vm_bo_rmv() shouldn't affect the page tables nor already > submitted command submissions in any way. > > Regards, > Christian. > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 -------- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++++ > > 2 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > index 45484c0..e13cdde 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > @@ -694,14 +694,6 @@ void amdgpu_driver_postclose_kms(struct > drm_device *dev, > > amdgpu_uvd_free_handles(adev, file_priv); > > amdgpu_vce_free_handles(adev, file_priv); > > > > - if (amdgpu_sriov_vf(adev)) { > > - /* TODO: how to handle reserve failure */ > > - BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); > > - amdgpu_vm_bo_rmv(adev, fpriv->vm.csa_bo_va); > > - fpriv->vm.csa_bo_va = NULL; > > - amdgpu_bo_unreserve(adev->virt.csa_obj); > > - } > > - > > amdgpu_vm_fini(adev, &fpriv->vm); > > > > idr_for_each_entry(&fpriv->bo_list_handles, list, handle) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > index d05546e..94098bc 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > @@ -1608,6 +1608,14 @@ void amdgpu_vm_fini(struct amdgpu_device > *adev, struct amdgpu_vm *vm) > > > > amd_sched_entity_fini(vm->entity.sched, &vm->entity); > > > > + if (amdgpu_sriov_vf(adev)) { > > + /* TODO: how to handle reserve failure */ > > + BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false)); > > + amdgpu_vm_bo_rmv(adev, vm->csa_bo_va); > > + vm->csa_bo_va = NULL; > > + amdgpu_bo_unreserve(adev->virt.csa_obj); > > + } > > + > > if (!RB_EMPTY_ROOT(&vm->va)) { > > dev_err(adev->dev, "still active bo inside vm\n"); > > } > > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx [-- Attachment #1.2: Type: text/html, Size: 8337 bytes --] [-- Attachment #2: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-13 10:23 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-13 4:11 [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini Monk Liu [not found] ` <1484280664-22845-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org> 2017-01-13 9:25 ` Christian König [not found] ` <dadee34c-ca4c-6a2a-8053-4bfdeb1466c3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 2017-01-13 9:56 ` 答复: " Liu, Monk [not found] ` <BY2PR1201MB11102468908DDFD083C45B5384780-O28G1zQ8oGliQkyLPkmea2rFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org> 2017-01-13 10:23 ` Christian König
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.