From 1655c893af08997175e3404039e79f384c925ee3 Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Wed, 19 Jul 2023 20:44:25 -0700 Subject: [PATCH] drm/xe: Reduce the number list links in xe_vma Combine the userptr, rebind, and destroy links into a union as the lists these links belong to are mutually exclusive. v2: Adjust which lists are combined (Thomas H) v3: Add kernel doc why this is safe (Thomas H), remove related change of list_del_init -> list_del (Rodrigo) Reviewed-by: Rodrigo Vivi Signed-off-by: Matthew Brost Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_bo.c | 6 +++-- drivers/gpu/drm/xe/xe_exec.c | 2 +- drivers/gpu/drm/xe/xe_pt.c | 2 +- drivers/gpu/drm/xe/xe_vm.c | 43 ++++++++++++++++---------------- drivers/gpu/drm/xe/xe_vm_types.h | 37 ++++++++++++++++----------- 5 files changed, 49 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c index 49c80e95222b..a78ac158e967 100644 --- a/drivers/gpu/drm/xe/xe_bo.c +++ b/drivers/gpu/drm/xe/xe_bo.c @@ -476,8 +476,10 @@ static int xe_bo_trigger_rebind(struct xe_device *xe, struct xe_bo *bo, } xe_vm_assert_held(vm); - if (list_empty(&vma->rebind_link) && vma->tile_present) - list_add_tail(&vma->rebind_link, &vm->rebind_list); + if (list_empty(&vma->combined_links.rebind) && + vma->tile_present) + list_add_tail(&vma->combined_links.rebind, + &vm->rebind_list); if (vm_resv_locked) dma_resv_unlock(xe_vm_resv(vm)); diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c index 0209f325dda0..89d167a432f6 100644 --- a/drivers/gpu/drm/xe/xe_exec.c +++ b/drivers/gpu/drm/xe/xe_exec.c @@ -120,7 +120,7 @@ retry: * BOs have valid placements possibly moving an evicted BO back * to a location where the GPU can access it). */ - list_for_each_entry(vma, &vm->rebind_list, rebind_link) { + list_for_each_entry(vma, &vm->rebind_list, combined_links.rebind) { XE_WARN_ON(xe_vma_is_null(vma)); if (xe_vma_is_userptr(vma)) diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c index bc7fa5a850e2..b67144768af0 100644 --- a/drivers/gpu/drm/xe/xe_pt.c +++ b/drivers/gpu/drm/xe/xe_pt.c @@ -1805,7 +1805,7 @@ __xe_pt_unbind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_engine *e } if (!vma->tile_present) - list_del_init(&vma->rebind_link); + list_del_init(&vma->combined_links.rebind); if (unbind_pt_update.locked) { XE_WARN_ON(!xe_vma_is_userptr(vma)); diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c index bdff1999058f..29a950a02a36 100644 --- a/drivers/gpu/drm/xe/xe_vm.c +++ b/drivers/gpu/drm/xe/xe_vm.c @@ -467,7 +467,8 @@ int xe_vm_lock_dma_resv(struct xe_vm *vm, struct ww_acquire_ctx *ww, list_del_init(&vma->notifier.rebind_link); if (vma->tile_present && !(vma->gpuva.flags & XE_VMA_DESTROYED)) - list_move_tail(&vma->rebind_link, &vm->rebind_list); + list_move_tail(&vma->combined_links.rebind, + &vm->rebind_list); } spin_unlock(&vm->notifier.list_lock); @@ -608,7 +609,7 @@ retry: if (err) goto out_unlock; - list_for_each_entry(vma, &vm->rebind_list, rebind_link) { + list_for_each_entry(vma, &vm->rebind_list, combined_links.rebind) { if (xe_vma_has_no_bo(vma) || vma->gpuva.flags & XE_VMA_DESTROYED) continue; @@ -780,17 +781,20 @@ int xe_vm_userptr_pin(struct xe_vm *vm) list_for_each_entry_safe(vma, next, &vm->userptr.invalidated, userptr.invalidate_link) { list_del_init(&vma->userptr.invalidate_link); - list_move_tail(&vma->userptr_link, &vm->userptr.repin_list); + if (list_empty(&vma->combined_links.userptr)) + list_move_tail(&vma->combined_links.userptr, + &vm->userptr.repin_list); } spin_unlock(&vm->userptr.invalidated_lock); /* Pin and move to temporary list */ - list_for_each_entry_safe(vma, next, &vm->userptr.repin_list, userptr_link) { + list_for_each_entry_safe(vma, next, &vm->userptr.repin_list, + combined_links.userptr) { err = xe_vma_userptr_pin_pages(vma); if (err < 0) goto out_err; - list_move_tail(&vma->userptr_link, &tmp_evict); + list_move_tail(&vma->combined_links.userptr, &tmp_evict); } /* Take lock and move to rebind_list for rebinding. */ @@ -798,10 +802,8 @@ int xe_vm_userptr_pin(struct xe_vm *vm) if (err) goto out_err; - list_for_each_entry_safe(vma, next, &tmp_evict, userptr_link) { - list_del_init(&vma->userptr_link); - list_move_tail(&vma->rebind_link, &vm->rebind_list); - } + list_for_each_entry_safe(vma, next, &tmp_evict, combined_links.userptr) + list_move_tail(&vma->combined_links.rebind, &vm->rebind_list); dma_resv_unlock(xe_vm_resv(vm)); @@ -845,10 +847,11 @@ struct dma_fence *xe_vm_rebind(struct xe_vm *vm, bool rebind_worker) return NULL; xe_vm_assert_held(vm); - list_for_each_entry_safe(vma, next, &vm->rebind_list, rebind_link) { + list_for_each_entry_safe(vma, next, &vm->rebind_list, + combined_links.rebind) { XE_WARN_ON(!vma->tile_present); - list_del_init(&vma->rebind_link); + list_del_init(&vma->combined_links.rebind); dma_fence_put(fence); if (rebind_worker) trace_xe_vma_rebind_worker(vma); @@ -883,9 +886,7 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm, return vma; } - INIT_LIST_HEAD(&vma->rebind_link); - INIT_LIST_HEAD(&vma->unbind_link); - INIT_LIST_HEAD(&vma->userptr_link); + INIT_LIST_HEAD(&vma->combined_links.rebind); INIT_LIST_HEAD(&vma->userptr.invalidate_link); INIT_LIST_HEAD(&vma->notifier.rebind_link); INIT_LIST_HEAD(&vma->extobj.link); @@ -1070,7 +1071,7 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence) struct xe_vm *vm = xe_vma_vm(vma); lockdep_assert_held_write(&vm->lock); - XE_BUG_ON(!list_empty(&vma->unbind_link)); + XE_BUG_ON(!list_empty(&vma->combined_links.destroy)); if (xe_vma_is_userptr(vma)) { XE_WARN_ON(!(vma->gpuva.flags & XE_VMA_DESTROYED)); @@ -1078,7 +1079,6 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence) spin_lock(&vm->userptr.invalidated_lock); list_del_init(&vma->userptr.invalidate_link); spin_unlock(&vm->userptr.invalidated_lock); - list_del(&vma->userptr_link); } else if (!xe_vma_is_null(vma)) { xe_bo_assert_held(xe_vma_bo(vma)); @@ -1099,9 +1099,6 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence) } xe_vm_assert_held(vm); - if (!list_empty(&vma->rebind_link)) - list_del(&vma->rebind_link); - if (fence) { int ret = dma_fence_add_callback(fence, &vma->destroy_cb, vma_destroy_cb); @@ -1451,11 +1448,12 @@ void xe_vm_close_and_put(struct xe_vm *vm) /* easy case, remove from VMA? */ if (xe_vma_has_no_bo(vma) || xe_vma_bo(vma)->vm) { + list_del_init(&vma->combined_links.rebind); xe_vma_destroy(vma, NULL); continue; } - list_add_tail(&vma->unbind_link, &contested); + list_move_tail(&vma->combined_links.destroy, &contested); } /* @@ -1487,8 +1485,9 @@ void xe_vm_close_and_put(struct xe_vm *vm) * Since we hold a refcount to the bo, we can remove and free * the members safely without locking. */ - list_for_each_entry_safe(vma, next_vma, &contested, unbind_link) { - list_del_init(&vma->unbind_link); + list_for_each_entry_safe(vma, next_vma, &contested, + combined_links.destroy) { + list_del_init(&vma->combined_links.destroy); xe_vma_destroy_unlocked(vma); } diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h index a7386e8c2870..00bf0065d514 100644 --- a/drivers/gpu/drm/xe/xe_vm_types.h +++ b/drivers/gpu/drm/xe/xe_vm_types.h @@ -50,21 +50,28 @@ struct xe_vma { */ u64 tile_present; - /** @userptr_link: link into VM repin list if userptr */ - struct list_head userptr_link; - - /** - * @rebind_link: link into VM if this VMA needs rebinding, and - * if it's a bo (not userptr) needs validation after a possible - * eviction. Protected by the vm's resv lock. - */ - struct list_head rebind_link; - - /** - * @unbind_link: link or list head if an unbind of multiple VMAs, in - * single unbind op, is being done. - */ - struct list_head unbind_link; + /** @combined_links: links into lists which are mutually exclusive */ + union { + /** + * @userptr: link into VM repin list if userptr. Protected by + * vm->lock in write mode. + */ + struct list_head userptr; + /** + * @rebind: link into VM if this VMA needs rebinding, and + * if it's a bo (not userptr) needs validation after a possible + * eviction. Protected by the vm's resv lock and typically + * vm->lock is also held in write mode. The only place where + * vm->lock isn't held is the BO eviction path which has + * mutually exclusive execution with userptr. + */ + struct list_head rebind; + /** + * @destroy: link to contested list when VM is being closed. + * Protected by vm->lock in write mode and vm's resv lock. + */ + struct list_head destroy; + } combined_links; /** @destroy_cb: callback to destroy VMA when unbind job is done */ struct dma_fence_cb destroy_cb;