[5/6] drm/amdgpu: stop reserving the BO in the MMU callback

Submitted by Christian König on Sept. 5, 2017, 3:37 p.m.

Details

Message ID 1504625871-1756-5-git-send-email-deathsimple@vodafone.de
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Christian König Sept. 5, 2017, 3:37 p.m.
From: Christian König <christian.koenig@amd.com>

Instead take the callback lock during the final parts of CS.

This should solve the last remaining locking order problems with BO reservations.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  8 ++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 29 +++++++++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 22 +++++++++++++---------
 3 files changed, 44 insertions(+), 15 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5d1f9cc..466330e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -180,6 +180,7 @@  struct amdgpu_cs_parser;
 struct amdgpu_job;
 struct amdgpu_irq_src;
 struct amdgpu_fpriv;
+struct amdgpu_mn;
 
 enum amdgpu_cp_irq {
 	AMDGPU_CP_IRQ_GFX_EOP = 0,
@@ -1059,6 +1060,7 @@  struct amdgpu_cs_parser {
 	/* buffer objects */
 	struct ww_acquire_ctx		ticket;
 	struct amdgpu_bo_list		*bo_list;
+	struct amdgpu_mn		*mn;
 	struct amdgpu_bo_list_entry	vm_pd;
 	struct list_head		validated;
 	struct dma_fence		*fence;
@@ -1200,14 +1202,20 @@  void amdgpu_test_moves(struct amdgpu_device *adev);
  * MMU Notifier
  */
 #if defined(CONFIG_MMU_NOTIFIER)
+struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev);
 int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr);
 void amdgpu_mn_unregister(struct amdgpu_bo *bo);
+void amdgpu_mn_lock(struct amdgpu_mn *mn);
+void amdgpu_mn_unlock(struct amdgpu_mn *mn);
 #else
+struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev) { return NULL; }
 static inline int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
 {
 	return -ENODEV;
 }
 static inline void amdgpu_mn_unregister(struct amdgpu_bo *bo) {}
+void amdgpu_mn_lock(struct amdgpu_mn *mn) {}
+void amdgpu_mn_unlock(struct amdgpu_mn *mn) {}
 #endif
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index aa45e25f..2857eb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -510,8 +510,11 @@  static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	INIT_LIST_HEAD(&p->validated);
 
 	p->bo_list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);
-	if (p->bo_list)
+	if (p->bo_list) {
 		amdgpu_bo_list_get_list(p->bo_list, &p->validated);
+		if (p->bo_list->first_userptr != p->bo_list->num_entries)
+			p->mn = amdgpu_mn_get(p->adev);
+	}
 
 	INIT_LIST_HEAD(&duplicates);
 	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
@@ -718,11 +721,7 @@  static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error,
 {
 	unsigned i;
 
-	if (!error)
-		ttm_eu_fence_buffer_objects(&parser->ticket,
-					    &parser->validated,
-					    parser->fence);
-	else if (backoff)
+	if (error && backoff)
 		ttm_eu_backoff_reservation(&parser->ticket,
 					   &parser->validated);
 	dma_fence_put(parser->fence);
@@ -1041,14 +1040,29 @@  static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	struct amdgpu_ring *ring = p->job->ring;
 	struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity;
 	struct amdgpu_job *job;
+	unsigned i;
 	int r;
 
+	amdgpu_mn_lock(p->mn);
+	if (p->bo_list) {
+		for (i = p->bo_list->first_userptr;
+		     i < p->bo_list->num_entries; ++i) {
+			struct amdgpu_bo *bo = p->bo_list->array[i].robj;
+
+			if (amdgpu_ttm_tt_userptr_needs_pages(bo->tbo.ttm)) {
+				amdgpu_mn_unlock(p->mn);
+				return -ERESTARTSYS;
+			}
+		}
+	}
+
 	job = p->job;
 	p->job = NULL;
 
 	r = amd_sched_job_init(&job->base, &ring->sched, entity, p->filp);
 	if (r) {
 		amdgpu_job_free(job);
+		amdgpu_mn_unlock(p->mn);
 		return r;
 	}
 
@@ -1062,6 +1076,9 @@  static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	trace_amdgpu_cs_ioctl(job);
 	amd_sched_entity_push_job(&job->base);
 
+	ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence);
+	amdgpu_mn_unlock(p->mn);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index cb10073..651a746 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -126,18 +126,10 @@  static void amdgpu_mn_invalidate_node(struct amdgpu_mn_node *node,
 		if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, start, end))
 			continue;
 
-		r = amdgpu_bo_reserve(bo, true);
-		if (r) {
-			DRM_ERROR("(%ld) failed to reserve user bo\n", r);
-			continue;
-		}
-
 		r = reservation_object_wait_timeout_rcu(bo->tbo.resv,
 			true, false, MAX_SCHEDULE_TIMEOUT);
 		if (r <= 0)
 			DRM_ERROR("(%ld) failed to wait for user bo\n", r);
-
-		amdgpu_bo_unreserve(bo);
 	}
 }
 
@@ -221,7 +213,7 @@  static const struct mmu_notifier_ops amdgpu_mn_ops = {
  *
  * Creates a notifier context for current->mm.
  */
-static struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev)
+struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev)
 {
 	struct mm_struct *mm = current->mm;
 	struct amdgpu_mn *rmn;
@@ -366,3 +358,15 @@  void amdgpu_mn_unregister(struct amdgpu_bo *bo)
 	up_write(&rmn->lock);
 	mutex_unlock(&adev->mn_lock);
 }
+
+void amdgpu_mn_lock(struct amdgpu_mn *mn)
+{
+	if (mn)
+		down_write(&mn->lock);
+}
+
+void amdgpu_mn_unlock(struct amdgpu_mn *mn)
+{
+	if (mn)
+		up_write(&mn->lock);
+}

Comments

On 2017-09-05 11:37 AM, Christian König wrote:
> +struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev) { return NULL; }
>  static inline int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
>  {
>  	return -ENODEV;
>  }
>  static inline void amdgpu_mn_unregister(struct amdgpu_bo *bo) {}
> +void amdgpu_mn_lock(struct amdgpu_mn *mn) {}
> +void amdgpu_mn_unlock(struct amdgpu_mn *mn) {}
The added dummy unctions here should be static inline.

Regards,
  Felix