Commit d490ecf5 authored by Thomas Hellström's avatar Thomas Hellström Committed by Rodrigo Vivi
Browse files

drm/xe: Rework xe_exec and the VM rebind worker to use the drm_exec helper 



Replace the calls to ttm_eu_reserve_buffers() by using the drm_exec
helper instead. Also make sure the locking loop covers any calls to
xe_bo_validate() / ttm_bo_validate() so that these function calls may
easily benefit from being called from within an unsealed locking
transaction and may thus perform blocking dma_resv locks in the future.

For the unlock we remove an assert that the vm->rebind_list is empty
when locks are released. Since if the error path is hit with a partly
locked list, that assert may no longer hold true we chose to remove it.

v3:
- Don't accept duplicate bo locks in the rebind worker.
v5:
- Loop over drm_exec objects in reverse when unlocking.
v6:
- We can't keep the WW ticket when retrying validation on OOM. Fix.

Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: default avatarMatthew Brost <matthew.brost@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230908091716.36984-5-thomas.hellstrom@linux.intel.com


Signed-off-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
parent b7ab8c4f

drivers/gpu/drm/xe/Kconfig

+2 −0
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ config DRM_XE 
	select SHMEM

	select TMPFS

	select DRM_BUDDY

	select DRM_EXEC

	select DRM_KMS_HELPER

	select DRM_PANEL

	select DRM_SUBALLOC_HELPER
@@ -21,6 +22,7 @@ config DRM_XE 
	select VMAP_PFN

	select DRM_TTM

	select DRM_TTM_HELPER

	select DRM_EXEC

	select DRM_GPUVM

	select DRM_SCHED

	select MMU_NOTIFIER

drivers/gpu/drm/xe/xe_exec.c

+23 −54
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ 
#include "xe_exec.h"



#include <drm/drm_device.h>

#include <drm/drm_exec.h>

#include <drm/drm_file.h>

#include <drm/xe_drm.h>

#include <linux/delay.h>
@@ -93,25 +94,16 @@ 
 *	Unlock all

 */



#define XE_EXEC_BIND_RETRY_TIMEOUT_MS 1000



static int xe_exec_begin(struct xe_exec_queue *q, struct ww_acquire_ctx *ww,

			 struct ttm_validate_buffer tv_onstack[],

			 struct ttm_validate_buffer **tv,

			 struct list_head *objs)

static int xe_exec_begin(struct drm_exec *exec, struct xe_vm *vm)

{

	struct xe_vm *vm = q->vm;

	struct xe_vma *vma;

	LIST_HEAD(dups);

	ktime_t end = 0;

	int err = 0;



	*tv = NULL;

	if (xe_vm_no_dma_fences(q->vm))

	if (xe_vm_no_dma_fences(vm))

		return 0;



retry:

	err = xe_vm_lock_dma_resv(vm, ww, tv_onstack, tv, objs, true, 1);

	err = xe_vm_lock_dma_resv(vm, exec, 1, true);

	if (err)

		return err;
@@ -127,42 +119,13 @@ static int xe_exec_begin(struct xe_exec_queue *q, struct ww_acquire_ctx *ww, 
			continue;



		err = xe_bo_validate(xe_vma_bo(vma), vm, false);

		if (err) {

			xe_vm_unlock_dma_resv(vm, tv_onstack, *tv, ww, objs);

			*tv = NULL;

		if (err)

			break;

	}

	}



	/*

	 * With multiple active VMs, under memory pressure, it is possible that

	 * ttm_bo_validate() run into -EDEADLK and in such case returns -ENOMEM.

	 * Until ttm properly handles locking in such scenarios, best thing the

	 * driver can do is retry with a timeout.

	 */

	if (err == -ENOMEM) {

		ktime_t cur = ktime_get();



		end = end ? : ktime_add_ms(cur, XE_EXEC_BIND_RETRY_TIMEOUT_MS);

		if (ktime_before(cur, end)) {

			msleep(20);

			goto retry;

		}

	}



	return err;

}



static void xe_exec_end(struct xe_exec_queue *q,

			struct ttm_validate_buffer *tv_onstack,

			struct ttm_validate_buffer *tv,

			struct ww_acquire_ctx *ww,

			struct list_head *objs)

{

	if (!xe_vm_no_dma_fences(q->vm))

		xe_vm_unlock_dma_resv(q->vm, tv_onstack, tv, ww, objs);

}



int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file)

{

	struct xe_device *xe = to_xe_device(dev);
@@ -173,15 +136,13 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file) 
	struct xe_exec_queue *q;

	struct xe_sync_entry *syncs = NULL;

	u64 addresses[XE_HW_ENGINE_MAX_INSTANCE];

	struct ttm_validate_buffer tv_onstack[XE_ONSTACK_TV];

	struct ttm_validate_buffer *tv = NULL;

	struct drm_exec exec;

	u32 i, num_syncs = 0;

	struct xe_sched_job *job;

	struct dma_fence *rebind_fence;

	struct xe_vm *vm;

	struct ww_acquire_ctx ww;

	struct list_head objs;

	bool write_locked;

	ktime_t end = 0;

	int err = 0;



	if (XE_IOCTL_DBG(xe, args->extensions) ||
@@ -294,26 +255,34 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file) 
			goto err_unlock_list;

	}



	err = xe_exec_begin(q, &ww, tv_onstack, &tv, &objs);

	if (err)

	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);

	drm_exec_until_all_locked(&exec) {

		err = xe_exec_begin(&exec, vm);

		drm_exec_retry_on_contention(&exec);

		if (err && xe_vm_validate_should_retry(&exec, err, &end)) {

			err = -EAGAIN;

			goto err_unlock_list;

		}

		if (err)

			goto err_exec;

	}



	if (xe_vm_is_closed_or_banned(q->vm)) {

		drm_warn(&xe->drm, "Trying to schedule after vm is closed or banned\n");

		err = -ECANCELED;

		goto err_exec_queue_end;

		goto err_exec;

	}



	if (xe_exec_queue_is_lr(q) && xe_exec_queue_ring_full(q)) {

		err = -EWOULDBLOCK;

		goto err_exec_queue_end;

		goto err_exec;

	}



	job = xe_sched_job_create(q, xe_exec_queue_is_parallel(q) ?

				  addresses : &args->address);

	if (IS_ERR(job)) {

		err = PTR_ERR(job);

		goto err_exec_queue_end;

		goto err_exec;

	}



	/*
@@ -412,8 +381,8 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file) 
err_put_job:

	if (err)

		xe_sched_job_put(job);

err_exec_queue_end:

	xe_exec_end(q, tv_onstack, tv, &ww, &objs);

err_exec:

	drm_exec_fini(&exec);

err_unlock_list:

	if (write_locked)

		up_write(&vm->lock);

drivers/gpu/drm/xe/xe_vm.c

+123 −148
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ 


#include <linux/dma-fence-array.h>



#include <drm/drm_exec.h>

#include <drm/drm_print.h>

#include <drm/ttm/ttm_execbuf_util.h>

#include <drm/ttm/ttm_tt.h>
@@ -327,10 +328,7 @@ static void resume_and_reinstall_preempt_fences(struct xe_vm *vm) 


int xe_vm_add_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q)

{

	struct ttm_validate_buffer tv_onstack[XE_ONSTACK_TV];

	struct ttm_validate_buffer *tv;

	struct ww_acquire_ctx ww;

	struct list_head objs;

	struct drm_exec exec;

	struct dma_fence *pfence;

	int err;

	bool wait;
@@ -338,10 +336,13 @@ int xe_vm_add_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q) 
	XE_WARN_ON(!xe_vm_in_compute_mode(vm));



	down_write(&vm->lock);



	err = xe_vm_lock_dma_resv(vm, &ww, tv_onstack, &tv, &objs, true, 1);

	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);

	drm_exec_until_all_locked(&exec) {

		err = xe_vm_lock_dma_resv(vm, &exec, 1, true);

		drm_exec_retry_on_contention(&exec);

		if (err)

		goto out_unlock_outer;

			goto out_unlock;

	}



	pfence = xe_preempt_fence_create(q, q->compute.context,

					 ++q->compute.seqno);
@@ -373,8 +374,7 @@ int xe_vm_add_compute_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q) 
	up_read(&vm->userptr.notifier_lock);



out_unlock:

	xe_vm_unlock_dma_resv(vm, tv_onstack, tv, &ww, &objs);

out_unlock_outer:

	drm_exec_fini(&exec);

	up_write(&vm->lock);



	return err;
@@ -403,68 +403,35 @@ int __xe_vm_userptr_needs_repin(struct xe_vm *vm) 
 * xe_vm_lock_dma_resv() - Lock the vm dma_resv object and the dma_resv

 * objects of the vm's external buffer objects.

 * @vm: The vm.

 * @ww: Pointer to a struct ww_acquire_ctx locking context.

 * @tv_onstack: Array size XE_ONSTACK_TV of storage for the struct

 * ttm_validate_buffers used for locking.

 * @tv: Pointer to a pointer that on output contains the actual storage used.

 * @objs: List head for the buffer objects locked.

 * @intr: Whether to lock interruptible.

 * @exec: Pointer to a struct drm_exec locking context.

 * @num_shared: Number of dma-fence slots to reserve in the locked objects.

 * @lock_vm: Lock also the vm's dma_resv.

 *

 * Locks the vm dma-resv objects and all the dma-resv objects of the

 * buffer objects on the vm external object list. The TTM utilities require

 * a list of struct ttm_validate_buffers pointing to the actual buffer

 * objects to lock. Storage for those struct ttm_validate_buffers should

 * be provided in @tv_onstack, and is typically reserved on the stack

 * of the caller. If the size of @tv_onstack isn't sufficient, then

 * storage will be allocated internally using kvmalloc().

 *

 * The function performs deadlock handling internally, and after a

 * successful return the ww locking transaction should be considered

 * sealed.

 * buffer objects on the vm external object list.

 *

 * Return: 0 on success, Negative error code on error. In particular if

 * @intr is set to true, -EINTR or -ERESTARTSYS may be returned. In case

 * of error, any locking performed has been reverted.

 * @intr is set to true, -EINTR or -ERESTARTSYS may be returned.

 */

int xe_vm_lock_dma_resv(struct xe_vm *vm, struct ww_acquire_ctx *ww,

			struct ttm_validate_buffer *tv_onstack,

			struct ttm_validate_buffer **tv,

			struct list_head *objs,

			bool intr,

			unsigned int num_shared)

{

	struct ttm_validate_buffer *tv_vm, *tv_bo;

int xe_vm_lock_dma_resv(struct xe_vm *vm, struct drm_exec *exec,

			unsigned int num_shared, bool lock_vm)

{

	struct xe_vma *vma, *next;

	LIST_HEAD(dups);

	int err;

	int err = 0;



	lockdep_assert_held(&vm->lock);



	if (vm->extobj.entries < XE_ONSTACK_TV) {

		tv_vm = tv_onstack;

	} else {

		tv_vm = kvmalloc_array(vm->extobj.entries + 1, sizeof(*tv_vm),

				       GFP_KERNEL);

		if (!tv_vm)

			return -ENOMEM;

	if (lock_vm) {

		err = drm_exec_prepare_obj(exec, &xe_vm_ttm_bo(vm)->base, num_shared);

		if (err)

			return err;

	}

	tv_bo = tv_vm + 1;



	INIT_LIST_HEAD(objs);

	list_for_each_entry(vma, &vm->extobj.list, extobj.link) {

		tv_bo->num_shared = num_shared;

		tv_bo->bo = &xe_vma_bo(vma)->ttm;



		list_add_tail(&tv_bo->head, objs);

		tv_bo++;

	}

	tv_vm->num_shared = num_shared;

	tv_vm->bo = xe_vm_ttm_bo(vm);

	list_add_tail(&tv_vm->head, objs);

	err = ttm_eu_reserve_buffers(ww, objs, intr, &dups);

		err = drm_exec_prepare_obj(exec, &xe_vma_bo(vma)->ttm.base, num_shared);

		if (err)

		goto out_err;

			return err;

	}



	spin_lock(&vm->notifier.list_lock);

	list_for_each_entry_safe(vma, next, &vm->notifier.rebind_list,
@@ -478,45 +445,7 @@ int xe_vm_lock_dma_resv(struct xe_vm *vm, struct ww_acquire_ctx *ww, 
	}

	spin_unlock(&vm->notifier.list_lock);



	*tv = tv_vm;

	return 0;



out_err:

	if (tv_vm != tv_onstack)

		kvfree(tv_vm);



	return err;

}



/**

 * xe_vm_unlock_dma_resv() - Unlock reservation objects locked by

 * xe_vm_lock_dma_resv()

 * @vm: The vm.

 * @tv_onstack: The @tv_onstack array given to xe_vm_lock_dma_resv().

 * @tv: The value of *@tv given by xe_vm_lock_dma_resv().

 * @ww: The ww_acquire_context used for locking.

 * @objs: The list returned from xe_vm_lock_dma_resv().

 *

 * Unlocks the reservation objects and frees any memory allocated by

 * xe_vm_lock_dma_resv().

 */

void xe_vm_unlock_dma_resv(struct xe_vm *vm,

			   struct ttm_validate_buffer *tv_onstack,

			   struct ttm_validate_buffer *tv,

			   struct ww_acquire_ctx *ww,

			   struct list_head *objs)

{

	/*

	 * Nothing should've been able to enter the list while we were locked,

	 * since we've held the dma-resvs of all the vm's external objects,

	 * and holding the dma_resv of an object is required for list

	 * addition, and we shouldn't add ourselves.

	 */

	XE_WARN_ON(!list_empty(&vm->notifier.rebind_list));



	ttm_eu_backoff_reservation(ww, objs);

	if (tv && tv != tv_onstack)

		kvfree(tv);

}



#define XE_VM_REBIND_RETRY_TIMEOUT_MS 1000
@@ -538,14 +467,94 @@ static void xe_vm_kill(struct xe_vm *vm) 
	/* TODO: Inform user the VM is banned */

}



/**

 * xe_vm_validate_should_retry() - Whether to retry after a validate error.

 * @exec: The drm_exec object used for locking before validation.

 * @err: The error returned from ttm_bo_validate().

 * @end: A ktime_t cookie that should be set to 0 before first use and

 * that should be reused on subsequent calls.

 *

 * With multiple active VMs, under memory pressure, it is possible that

 * ttm_bo_validate() run into -EDEADLK and in such case returns -ENOMEM.

 * Until ttm properly handles locking in such scenarios, best thing the

 * driver can do is retry with a timeout. Check if that is necessary, and

 * if so unlock the drm_exec's objects while keeping the ticket to prepare

 * for a rerun.

 *

 * Return: true if a retry after drm_exec_init() is recommended;

 * false otherwise.

 */

bool xe_vm_validate_should_retry(struct drm_exec *exec, int err, ktime_t *end)

{

	ktime_t cur;



	if (err != -ENOMEM)

		return false;



	cur = ktime_get();

	*end = *end ? : ktime_add_ms(cur, XE_VM_REBIND_RETRY_TIMEOUT_MS);

	if (!ktime_before(cur, *end))

		return false;



	/*

	 * We would like to keep the ticket here with

	 * drm_exec_unlock_all(), but WW mutex asserts currently

	 * stop us from that. In any case this function could go away

	 * with proper TTM -EDEADLK handling.

	 */

	drm_exec_fini(exec);



	msleep(20);

	return true;

}



static int xe_preempt_work_begin(struct drm_exec *exec, struct xe_vm *vm,

				 bool *done)

{

	struct xe_vma *vma;

	int err;



	err = drm_exec_prepare_obj(exec, &xe_vm_ttm_bo(vm)->base,

				   vm->preempt.num_exec_queues);

	if (err)

		return err;



	if (xe_vm_is_idle(vm)) {

		vm->preempt.rebind_deactivated = true;

		*done = true;

		return 0;

	}



	if (!preempt_fences_waiting(vm)) {

		*done = true;

		return 0;

	}



	err = xe_vm_lock_dma_resv(vm, exec, vm->preempt.num_exec_queues, false);

	if (err)

		return err;



	err = wait_for_existing_preempt_fences(vm);

	if (err)

		return err;



	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;



		err = xe_bo_validate(xe_vma_bo(vma), vm, false);

		if (err)

			break;

	}



	return err;

}



static void preempt_rebind_work_func(struct work_struct *w)

{

	struct xe_vm *vm = container_of(w, struct xe_vm, preempt.rebind_work);

	struct xe_vma *vma;

	struct ttm_validate_buffer tv_onstack[XE_ONSTACK_TV];

	struct ttm_validate_buffer *tv;

	struct ww_acquire_ctx ww;

	struct list_head objs;

	struct drm_exec exec;

	struct dma_fence *rebind_fence;

	unsigned int fence_count = 0;

	LIST_HEAD(preempt_fences);
@@ -588,42 +597,25 @@ static void preempt_rebind_work_func(struct work_struct *w) 
			goto out_unlock_outer;

	}



	err = xe_vm_lock_dma_resv(vm, &ww, tv_onstack, &tv, &objs,

				  false, vm->preempt.num_exec_queues);

	if (err)

		goto out_unlock_outer;



	if (xe_vm_is_idle(vm)) {

		vm->preempt.rebind_deactivated = true;

		goto out_unlock;

	}

	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);



	/* Fresh preempt fences already installed. Everyting is running. */

	if (!preempt_fences_waiting(vm))

		goto out_unlock;

	drm_exec_until_all_locked(&exec) {

		bool done = false;



	/*

	 * This makes sure vm is completely suspended and also balances

	 * xe_engine suspend- and resume; we resume *all* vm engines below.

	 */

	err = wait_for_existing_preempt_fences(vm);

	if (err)

		err = xe_preempt_work_begin(&exec, vm, &done);

		drm_exec_retry_on_contention(&exec);

		if (err && xe_vm_validate_should_retry(&exec, err, &end)) {

			err = -EAGAIN;

			goto out_unlock_outer;

		}

		if (err || done)

			goto out_unlock;

	}



	err = alloc_preempt_fences(vm, &preempt_fences, &fence_count);

	if (err)

		goto out_unlock;



	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;



		err = xe_bo_validate(xe_vma_bo(vma), vm, false);

		if (err)

			goto out_unlock;

	}



	rebind_fence = xe_vm_rebind(vm, true);

	if (IS_ERR(rebind_fence)) {

		err = PTR_ERR(rebind_fence);
@@ -668,30 +660,13 @@ static void preempt_rebind_work_func(struct work_struct *w) 
	up_read(&vm->userptr.notifier_lock);



out_unlock:

	xe_vm_unlock_dma_resv(vm, tv_onstack, tv, &ww, &objs);

	drm_exec_fini(&exec);

out_unlock_outer:

	if (err == -EAGAIN) {

		trace_xe_vm_rebind_worker_retry(vm);

		goto retry;

	}



	/*

	 * With multiple active VMs, under memory pressure, it is possible that

	 * ttm_bo_validate() run into -EDEADLK and in such case returns -ENOMEM.

	 * Until ttm properly handles locking in such scenarios, best thing the

	 * driver can do is retry with a timeout. Killing the VM or putting it

	 * in error state after timeout or other error scenarios is still TBD.

	 */

	if (err == -ENOMEM) {

		ktime_t cur = ktime_get();



		end = end ? : ktime_add_ms(cur, XE_VM_REBIND_RETRY_TIMEOUT_MS);

		if (ktime_before(cur, end)) {

			msleep(20);

			trace_xe_vm_rebind_worker_retry(vm);

			goto retry;

		}

	}

	if (err) {

		drm_warn(&vm->xe->drm, "VM worker error: %d\n", err);

		xe_vm_kill(vm);

drivers/gpu/drm/xe/xe_vm.h

+5 −17
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ struct ttm_validate_buffer; 
struct xe_exec_queue;

struct xe_file;

struct xe_sync_entry;

struct drm_exec;



struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags);
@@ -208,23 +209,10 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma); 


int xe_vma_userptr_check_repin(struct xe_vma *vma);



/*

 * XE_ONSTACK_TV is used to size the tv_onstack array that is input

 * to xe_vm_lock_dma_resv() and xe_vm_unlock_dma_resv().

 */

#define XE_ONSTACK_TV 20

int xe_vm_lock_dma_resv(struct xe_vm *vm, struct ww_acquire_ctx *ww,

			struct ttm_validate_buffer *tv_onstack,

			struct ttm_validate_buffer **tv,

			struct list_head *objs,

			bool intr,

			unsigned int num_shared);



void xe_vm_unlock_dma_resv(struct xe_vm *vm,

			   struct ttm_validate_buffer *tv_onstack,

			   struct ttm_validate_buffer *tv,

			   struct ww_acquire_ctx *ww,

			   struct list_head *objs);

bool xe_vm_validate_should_retry(struct drm_exec *exec, int err, ktime_t *end);



int xe_vm_lock_dma_resv(struct xe_vm *vm, struct drm_exec *exec,

			unsigned int num_shared, bool lock_vm);



void xe_vm_fence_all_extobjs(struct xe_vm *vm, struct dma_fence *fence,

			     enum dma_resv_usage usage);