[PATCH 2/4] drm/amdgpu: minor optimize VM moved handling v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 03.01.2018 um 12:57 schrieb Zhou, David(ChunMing):
> >>>> +        else if (reservation_object_trylock(resv))
> >>>> +            clear = false;
>
> this will effect bo in bo list�wont it?

Ah, now I get what you mean.

Yeah, that should be fixed or otherwise it will completely disable 
vm_debug support.

Going to send a patch,
Christian.

>
> å??è?ªå??æ?? Pro
>
> Koenig, Christian <Christian.Koenig at amd.com> äº? 2018å¹´1æ??3æ?¥ 
> ä¸?å??6:47å??é??ï¼?
>
> Am 03.01.2018 um 11:43 schrieb Chunming Zhou:
> >
> >
> > On 2018å¹´01æ??03æ?¥ 17:25, Christian König wrote:
> >> Am 03.01.2018 um 09:10 schrieb Zhou, David(ChunMing):
> >>>
> >>> On 2018å¹´01æ??02æ?¥ 22:47, Christian König wrote:
> >>>> Try to lock moved BOs if it's successful we can update the
> >>>> PTEs directly to the new location.
> >>>>
> >>>> v2: rebase
> >>>>
> >>>> Signed-off-by: Christian König <christian.koenig at amd.com>
> >>>> ---
> >>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 ++++++++++++++-
> >>>>    1 file changed, 14 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >>>> index 3632c69f1814..c1c5ccdee783 100644
> >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >>>> @@ -1697,18 +1697,31 @@ int amdgpu_vm_handle_moved(struct
> >>>> amdgpu_device *adev,
> >>>>        spin_lock(&vm->status_lock);
> >>>>        while (!list_empty(&vm->moved)) {
> >>>>            struct amdgpu_bo_va *bo_va;
> >>>> +        struct reservation_object *resv;
> >>>>               bo_va = list_first_entry(&vm->moved,
> >>>>                struct amdgpu_bo_va, base.vm_status);
> >>>> spin_unlock(&vm->status_lock);
> >>>>    +        resv = bo_va->base.bo->tbo.resv;
> >>>> +
> >>>>            /* Per VM BOs never need to bo cleared in the page
> >>>> tables */
> >>> This reminders us Per-VM-BOs need to cleared as well after we allow to
> >>> evict/swap out per-vm-bos.
> >>
> >> Actually they don't. The page tables only need to be valid during CS.
> >>
> >> So what happens is that the per VM-BOs are validated in right before
> >> we call amdgpu_vm_handle_moved().
> > Yeah, agree it for per-vm-bo situation after I checked all adding
> > moved list cases:
> > 1. validate pt bos
> > 2. bo invalidate
> > 3. insert_map for per-vm-bo
> > item #1 and #3 both are per-vm-bo, they are already validated before
> > handle_moved().
> >
> > For item #2, there are three places to call it:
> > a. amdgpu_bo_vm_update_pte in CS for amdgpu_vm_debug
> > b. amdgpu_gem_op_ioctl, but it is for evicted list, nothing with moved
> > list.
> > c. amdgpu_bo_move_notify when bo validate.
> >
> > For c case, your optimization is valid, we don't need clear for
> > validate bo.
> > But for a case, yours will break amdgpu_vm_debug functionality.
> >
> > Right?
>
> Interesting point, but no that should be handled as well.
>
> The vm_debug handling is only for the BOs on the BO-list. E.g. per VM
> BOs are never handled here.
>
> Regards,
> Christian.
>
> >
> > Regards,
> > David Zhou
> >
> >>
> >> Regards,
> >> Christian.
> >>
> >>>
> >>> Regards,
> >>> David Zhou
> >>>> -        clear = bo_va->base.bo->tbo.resv !=
> >>>> vm->root.base.bo->tbo.resv;
> >>>> +        if (resv == vm->root.base.bo->tbo.resv)
> >>>> +            clear = false;
> >>>> +        /* Try to reserve the BO to avoid clearing its ptes */
> >>>> +        else if (reservation_object_trylock(resv))
> >>>> +            clear = false;
> >>>> +        /* Somebody else is using the BO right now */
> >>>> +        else
> >>>> +            clear = true;
> >>>>               r = amdgpu_vm_bo_update(adev, bo_va, clear);
> >>>>            if (r)
> >>>>                return r;
> >>>>    +        if (!clear && resv != vm->root.base.bo->tbo.resv)
> >>>> + reservation_object_unlock(resv);
> >>>> +
> >>>> spin_lock(&vm->status_lock);
> >>>>        }
> >>>> spin_unlock(&vm->status_lock);
> >>
> >
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180103/151f82a2/attachment.html>


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux