From d7006964d46d76930a44e14f1aae401ccb1797da Mon Sep 17 00:00:00 2001 From: Christian König Date: Mon, 8 Feb 2016 10:57:22 +0100 Subject: drm/amdgpu: fix issue with overlapping userptrs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise we could try to evict overlapping userptr BOs in get_user_pages(), leading to a possible circular locking dependency. Signed-off-by: Christian König Acked-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index b1969f2b2038..d4e2780c0796 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -142,7 +142,8 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, list_for_each_entry(bo, &node->bos, mn_list) { - if (!bo->tbo.ttm || bo->tbo.ttm->state != tt_bound) + if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, start, + end)) continue; r = amdgpu_bo_reserve(bo, true); -- cgit v1.2.3 From c41d271d751ea023f67f1ea755e144b49079b6d5 Mon Sep 17 00:00:00 2001 From: Christian König Date: Tue, 9 Feb 2016 16:13:37 +0100 Subject: drm/amdgpu: remove the userptr rmn->lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid a lock inversion problem by just using the mmap_sem to protect the entries of the intervall tree. Signed-off-by: Christian König Reviewed-by: Felix Kuehling --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index d4e2780c0796..61f0e3c3172a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -48,8 +48,7 @@ struct amdgpu_mn { /* protected by adev->mn_lock */ struct hlist_node node; - /* objects protected by lock */ - struct mutex lock; + /* objects protected by mm->mmap_sem */ struct rb_root objects; }; @@ -72,8 +71,8 @@ static void amdgpu_mn_destroy(struct work_struct *work) struct amdgpu_mn_node *node, *next_node; struct amdgpu_bo *bo, *next_bo; + down_write(&rmn->mm->mmap_sem); mutex_lock(&adev->mn_lock); - mutex_lock(&rmn->lock); hash_del(&rmn->node); rbtree_postorder_for_each_entry_safe(node, next_node, &rmn->objects, it.rb) { @@ -85,8 +84,8 @@ static void amdgpu_mn_destroy(struct work_struct *work) } kfree(node); } - mutex_unlock(&rmn->lock); mutex_unlock(&adev->mn_lock); + up_write(&rmn->mm->mmap_sem); mmu_notifier_unregister(&rmn->mn, rmn->mm); kfree(rmn); } @@ -129,8 +128,6 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, /* notification is exclusive, but interval is inclusive */ end -= 1; - mutex_lock(&rmn->lock); - it = interval_tree_iter_first(&rmn->objects, start, end); while (it) { struct amdgpu_mn_node *node; @@ -165,8 +162,6 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, amdgpu_bo_unreserve(bo); } } - - mutex_unlock(&rmn->lock); } static const struct mmu_notifier_ops amdgpu_mn_ops = { @@ -203,7 +198,6 @@ static struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev) rmn->adev = adev; rmn->mm = mm; rmn->mn.ops = &amdgpu_mn_ops; - mutex_init(&rmn->lock); rmn->objects = RB_ROOT; r = __mmu_notifier_register(&rmn->mn, mm); @@ -250,7 +244,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) INIT_LIST_HEAD(&bos); - mutex_lock(&rmn->lock); + down_write(&rmn->mm->mmap_sem); while ((it = interval_tree_iter_first(&rmn->objects, addr, end))) { kfree(node); @@ -264,7 +258,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) if (!node) { node = kmalloc(sizeof(struct amdgpu_mn_node), GFP_KERNEL); if (!node) { - mutex_unlock(&rmn->lock); + up_write(&rmn->mm->mmap_sem); return -ENOMEM; } } @@ -279,7 +273,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) interval_tree_insert(&node->it, &rmn->objects); - mutex_unlock(&rmn->lock); + up_write(&rmn->mm->mmap_sem); return 0; } @@ -294,17 +288,15 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) void amdgpu_mn_unregister(struct amdgpu_bo *bo) { struct amdgpu_device *adev = bo->adev; - struct amdgpu_mn *rmn; + struct amdgpu_mn *rmn = bo->mn; struct list_head *head; - mutex_lock(&adev->mn_lock); - rmn = bo->mn; - if (rmn == NULL) { - mutex_unlock(&adev->mn_lock); + if (rmn == NULL) return; - } - mutex_lock(&rmn->lock); + down_write(&rmn->mm->mmap_sem); + mutex_lock(&adev->mn_lock); + /* save the next list entry for later */ head = bo->mn_list.next; @@ -318,6 +310,6 @@ void amdgpu_mn_unregister(struct amdgpu_bo *bo) kfree(node); } - mutex_unlock(&rmn->lock); mutex_unlock(&adev->mn_lock); + up_write(&rmn->mm->mmap_sem); } -- cgit v1.2.3 From fa5b5000308136ad61a33126d8ad380cf1fef016 Mon Sep 17 00:00:00 2001 From: Felix Kuehling Date: Thu, 14 Jan 2016 00:35:08 -0500 Subject: drm/amdgpu: Fix race condition in MMU notifier release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release notifier can get called a second time from mmu_notifier_unregister depending on a race between __mmu_notifier_release and amdgpu_mn_destroy. Use mmu_notifier_unregister_no_release to avoid this. Signed-off-by: Felix Kuehling Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index 61f0e3c3172a..1b2105ca5af1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -86,7 +86,7 @@ static void amdgpu_mn_destroy(struct work_struct *work) } mutex_unlock(&adev->mn_lock); up_write(&rmn->mm->mmap_sem); - mmu_notifier_unregister(&rmn->mn, rmn->mm); + mmu_notifier_unregister_no_release(&rmn->mn, rmn->mm); kfree(rmn); } -- cgit v1.2.3 From b8ea3783467586246d0b2b08f2e3f42853219d94 Mon Sep 17 00:00:00 2001 From: Felix Kuehling Date: Tue, 16 Feb 2016 15:29:23 -0500 Subject: drm/amdgpu: Fix race condition in amdgpu_mn_unregister MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exchange locking order of adev->mn_lock and mm_sem, so that rmn->mm->mmap_sem can be taken safely, protected by adev->mn_lock, when amdgpu_mn_destroy runs concurrently. Signed-off-by: Felix Kuehling Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index 1b2105ca5af1..d12dff92f3ca 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -71,12 +71,11 @@ static void amdgpu_mn_destroy(struct work_struct *work) struct amdgpu_mn_node *node, *next_node; struct amdgpu_bo *bo, *next_bo; - down_write(&rmn->mm->mmap_sem); mutex_lock(&adev->mn_lock); + down_write(&rmn->mm->mmap_sem); hash_del(&rmn->node); rbtree_postorder_for_each_entry_safe(node, next_node, &rmn->objects, it.rb) { - interval_tree_remove(&node->it, &rmn->objects); list_for_each_entry_safe(bo, next_bo, &node->bos, mn_list) { bo->mn = NULL; @@ -84,8 +83,8 @@ static void amdgpu_mn_destroy(struct work_struct *work) } kfree(node); } - mutex_unlock(&adev->mn_lock); up_write(&rmn->mm->mmap_sem); + mutex_unlock(&adev->mn_lock); mmu_notifier_unregister_no_release(&rmn->mn, rmn->mm); kfree(rmn); } @@ -182,8 +181,8 @@ static struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev) struct amdgpu_mn *rmn; int r; - down_write(&mm->mmap_sem); mutex_lock(&adev->mn_lock); + down_write(&mm->mmap_sem); hash_for_each_possible(adev->mn_hash, rmn, node, (unsigned long)mm) if (rmn->mm == mm) @@ -207,14 +206,14 @@ static struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev) hash_add(adev->mn_hash, &rmn->node, (unsigned long)mm); release_locks: - mutex_unlock(&adev->mn_lock); up_write(&mm->mmap_sem); + mutex_unlock(&adev->mn_lock); return rmn; free_rmn: - mutex_unlock(&adev->mn_lock); up_write(&mm->mmap_sem); + mutex_unlock(&adev->mn_lock); kfree(rmn); return ERR_PTR(r); @@ -288,14 +287,18 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) void amdgpu_mn_unregister(struct amdgpu_bo *bo) { struct amdgpu_device *adev = bo->adev; - struct amdgpu_mn *rmn = bo->mn; + struct amdgpu_mn *rmn; struct list_head *head; - if (rmn == NULL) + mutex_lock(&adev->mn_lock); + + rmn = bo->mn; + if (rmn == NULL) { + mutex_unlock(&adev->mn_lock); return; + } down_write(&rmn->mm->mmap_sem); - mutex_lock(&adev->mn_lock); /* save the next list entry for later */ head = bo->mn_list.next; @@ -310,6 +313,6 @@ void amdgpu_mn_unregister(struct amdgpu_bo *bo) kfree(node); } - mutex_unlock(&adev->mn_lock); up_write(&rmn->mm->mmap_sem); + mutex_unlock(&adev->mn_lock); } -- cgit v1.2.3 From 390be2824fa4211c2e973c69b72e04000559bba3 Mon Sep 17 00:00:00 2001 From: Felix Kuehling Date: Tue, 16 Feb 2016 15:31:30 -0500 Subject: drm/amdgpu: Don't call interval_tree_remove in amdgpu_mn_destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rbtree_postorder_for_each_entry_safe can skip over some entries if the tree is rebalanced in interval_tree_remove. interval_tree_remove is also redundant when the tree is just about to be freed. Signed-off-by: Felix Kuehling Reviewed-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index d12dff92f3ca..d7ec9bd6755f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -76,7 +76,6 @@ static void amdgpu_mn_destroy(struct work_struct *work) hash_del(&rmn->node); rbtree_postorder_for_each_entry_safe(node, next_node, &rmn->objects, it.rb) { - interval_tree_remove(&node->it, &rmn->objects); list_for_each_entry_safe(bo, next_bo, &node->bos, mn_list) { bo->mn = NULL; list_del_init(&bo->mn_list); -- cgit v1.2.3 From 0d2b42b0bdba45c82d29d794ea30a4c90e3f4098 Mon Sep 17 00:00:00 2001 From: Christian König Date: Fri, 18 Mar 2016 19:29:51 +0100 Subject: drm/amdgpu: Revert "remove the userptr rmn->lock" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c02196834456f2d5fad334088b70e98ce4967c34. In the meantime we moved get_user_pages() outside of the reservation lock, so that shouldn't be an issue any more Signed-off-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index d7ec9bd6755f..c47f22224a65 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -48,7 +48,8 @@ struct amdgpu_mn { /* protected by adev->mn_lock */ struct hlist_node node; - /* objects protected by mm->mmap_sem */ + /* objects protected by lock */ + struct mutex lock; struct rb_root objects; }; @@ -72,7 +73,7 @@ static void amdgpu_mn_destroy(struct work_struct *work) struct amdgpu_bo *bo, *next_bo; mutex_lock(&adev->mn_lock); - down_write(&rmn->mm->mmap_sem); + mutex_lock(&rmn->lock); hash_del(&rmn->node); rbtree_postorder_for_each_entry_safe(node, next_node, &rmn->objects, it.rb) { @@ -82,7 +83,7 @@ static void amdgpu_mn_destroy(struct work_struct *work) } kfree(node); } - up_write(&rmn->mm->mmap_sem); + mutex_unlock(&rmn->lock); mutex_unlock(&adev->mn_lock); mmu_notifier_unregister_no_release(&rmn->mn, rmn->mm); kfree(rmn); @@ -126,6 +127,8 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, /* notification is exclusive, but interval is inclusive */ end -= 1; + mutex_lock(&rmn->lock); + it = interval_tree_iter_first(&rmn->objects, start, end); while (it) { struct amdgpu_mn_node *node; @@ -160,6 +163,8 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, amdgpu_bo_unreserve(bo); } } + + mutex_unlock(&rmn->lock); } static const struct mmu_notifier_ops amdgpu_mn_ops = { @@ -196,6 +201,7 @@ static struct amdgpu_mn *amdgpu_mn_get(struct amdgpu_device *adev) rmn->adev = adev; rmn->mm = mm; rmn->mn.ops = &amdgpu_mn_ops; + mutex_init(&rmn->lock); rmn->objects = RB_ROOT; r = __mmu_notifier_register(&rmn->mn, mm); @@ -242,7 +248,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) INIT_LIST_HEAD(&bos); - down_write(&rmn->mm->mmap_sem); + mutex_lock(&rmn->lock); while ((it = interval_tree_iter_first(&rmn->objects, addr, end))) { kfree(node); @@ -256,7 +262,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) if (!node) { node = kmalloc(sizeof(struct amdgpu_mn_node), GFP_KERNEL); if (!node) { - up_write(&rmn->mm->mmap_sem); + mutex_unlock(&rmn->lock); return -ENOMEM; } } @@ -271,7 +277,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr) interval_tree_insert(&node->it, &rmn->objects); - up_write(&rmn->mm->mmap_sem); + mutex_unlock(&rmn->lock); return 0; } @@ -297,7 +303,7 @@ void amdgpu_mn_unregister(struct amdgpu_bo *bo) return; } - down_write(&rmn->mm->mmap_sem); + mutex_lock(&rmn->lock); /* save the next list entry for later */ head = bo->mn_list.next; @@ -312,6 +318,6 @@ void amdgpu_mn_unregister(struct amdgpu_bo *bo) kfree(node); } - up_write(&rmn->mm->mmap_sem); + mutex_unlock(&rmn->lock); mutex_unlock(&adev->mn_lock); } -- cgit v1.2.3 From ae20f12d2de6629ee6f679ccf22f9b7b209c464d Mon Sep 17 00:00:00 2001 From: Christian König Date: Fri, 18 Mar 2016 19:29:52 +0100 Subject: drm/amdgpu: add invalidate_page callback for userptrs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise we can run into problems with the writeback code. Signed-off-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 98 +++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 26 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index c47f22224a65..9f4a45cd2aab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -105,6 +105,76 @@ static void amdgpu_mn_release(struct mmu_notifier *mn, schedule_work(&rmn->work); } +/** + * amdgpu_mn_invalidate_node - unmap all BOs of a node + * + * @node: the node with the BOs to unmap + * + * We block for all BOs and unmap them by move them + * into system domain again. + */ +static void amdgpu_mn_invalidate_node(struct amdgpu_mn_node *node, + unsigned long start, + unsigned long end) +{ + struct amdgpu_bo *bo; + long r; + + list_for_each_entry(bo, &node->bos, mn_list) { + + 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_ttm_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU); + r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false); + if (r) + DRM_ERROR("(%ld) failed to validate user bo\n", r); + + amdgpu_bo_unreserve(bo); + } +} + +/** + * amdgpu_mn_invalidate_page - callback to notify about mm change + * + * @mn: our notifier + * @mn: the mm this callback is about + * @address: address of invalidate page + * + * Invalidation of a single page. Blocks for all BOs mapping it + * and unmap them by move them into system domain again. + */ +static void amdgpu_mn_invalidate_page(struct mmu_notifier *mn, + struct mm_struct *mm, + unsigned long address) +{ + struct amdgpu_mn *rmn = container_of(mn, struct amdgpu_mn, mn); + struct interval_tree_node *it; + + mutex_lock(&rmn->lock); + + it = interval_tree_iter_first(&rmn->objects, address, address); + if (it) { + struct amdgpu_mn_node *node; + + node = container_of(it, struct amdgpu_mn_node, it); + amdgpu_mn_invalidate_node(node, address, address); + } + + mutex_unlock(&rmn->lock); +} + /** * amdgpu_mn_invalidate_range_start - callback to notify about mm change * @@ -132,36 +202,11 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, it = interval_tree_iter_first(&rmn->objects, start, end); while (it) { struct amdgpu_mn_node *node; - struct amdgpu_bo *bo; - long r; node = container_of(it, struct amdgpu_mn_node, it); it = interval_tree_iter_next(it, start, end); - list_for_each_entry(bo, &node->bos, mn_list) { - - 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_ttm_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU); - r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false); - if (r) - DRM_ERROR("(%ld) failed to validate user bo\n", r); - - amdgpu_bo_unreserve(bo); - } + amdgpu_mn_invalidate_node(node, start, end); } mutex_unlock(&rmn->lock); @@ -169,6 +214,7 @@ static void amdgpu_mn_invalidate_range_start(struct mmu_notifier *mn, static const struct mmu_notifier_ops amdgpu_mn_ops = { .release = amdgpu_mn_release, + .invalidate_page = amdgpu_mn_invalidate_page, .invalidate_range_start = amdgpu_mn_invalidate_range_start, }; -- cgit v1.2.3