dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/ttm: fix re-init of global structures
@ 2019-04-16 10:53 Christian König
  2019-04-16 10:53 ` [PATCH 2/2] drm/ttm: remove pointers to globals Christian König
  2019-04-16 11:34 ` [PATCH 1/2] drm/ttm: fix re-init of global structures Karol Herbst
  0 siblings, 2 replies; 3+ messages in thread
From: Christian König @ 2019-04-16 10:53 UTC (permalink / raw)
  To: dri-devel, kherbst, itsbriany

When a driver unloads without unloading TTM we don't correctly
clear the global structures leading to errors on re-init.

Next step should probably be to remove the global structures and
kobjs all together, but this is tricky since we need to maintain
backward compatibility.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c     | 10 +++++-----
 drivers/gpu/drm/ttm/ttm_memory.c |  5 +++--
 include/drm/ttm/ttm_bo_driver.h  |  1 -
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 41d07faa2eae..2845fceb2fbd 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -49,9 +49,8 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj);
  * ttm_global_mutex - protecting the global BO state
  */
 DEFINE_MUTEX(ttm_global_mutex);
-struct ttm_bo_global ttm_bo_glob = {
-	.use_count = 0
-};
+unsigned ttm_bo_glob_use_count;
+struct ttm_bo_global ttm_bo_glob;
 
 static struct attribute ttm_bo_count = {
 	.name = "bo_count",
@@ -1531,12 +1530,13 @@ static void ttm_bo_global_release(void)
 	struct ttm_bo_global *glob = &ttm_bo_glob;
 
 	mutex_lock(&ttm_global_mutex);
-	if (--glob->use_count > 0)
+	if (--ttm_bo_glob_use_count > 0)
 		goto out;
 
 	kobject_del(&glob->kobj);
 	kobject_put(&glob->kobj);
 	ttm_mem_global_release(&ttm_mem_glob);
+	memset(glob, 0, sizeof(*glob));
 out:
 	mutex_unlock(&ttm_global_mutex);
 }
@@ -1548,7 +1548,7 @@ static int ttm_bo_global_init(void)
 	unsigned i;
 
 	mutex_lock(&ttm_global_mutex);
-	if (++glob->use_count > 1)
+	if (++ttm_bo_glob_use_count > 1)
 		goto out;
 
 	ret = ttm_mem_global_init(&ttm_mem_glob);
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 699fed9e08ee..8617958b7ae6 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -461,8 +461,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
 
 void ttm_mem_global_release(struct ttm_mem_global *glob)
 {
-	unsigned int i;
 	struct ttm_mem_zone *zone;
+	unsigned int i;
 
 	/* let the page allocator first stop the shrink work. */
 	ttm_page_alloc_fini();
@@ -475,9 +475,10 @@ void ttm_mem_global_release(struct ttm_mem_global *glob)
 		zone = glob->zones[i];
 		kobject_del(&zone->kobj);
 		kobject_put(&zone->kobj);
-			}
+	}
 	kobject_del(&glob->kobj);
 	kobject_put(&glob->kobj);
+	memset(glob, 0, sizeof(*glob));
 }
 
 static void ttm_check_swapping(struct ttm_mem_global *glob)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 3db265bf2d4e..c008346c2401 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -420,7 +420,6 @@ extern struct ttm_bo_global {
 	/**
 	 * Protected by ttm_global_mutex.
 	 */
-	unsigned int use_count;
 	struct list_head device_list;
 
 	/**
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] drm/ttm: remove pointers to globals
  2019-04-16 10:53 [PATCH 1/2] drm/ttm: fix re-init of global structures Christian König
@ 2019-04-16 10:53 ` Christian König
  2019-04-16 11:34 ` [PATCH 1/2] drm/ttm: fix re-init of global structures Karol Herbst
  1 sibling, 0 replies; 3+ messages in thread
From: Christian König @ 2019-04-16 10:53 UTC (permalink / raw)
  To: dri-devel, kherbst, itsbriany

As the name says global memory and bo accounting is global. So it doesn't
make to much sense having pointers to global structures all around the code.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   |  2 +-
 drivers/gpu/drm/qxl/qxl_release.c        |  4 +--
 drivers/gpu/drm/qxl/qxl_ttm.c            |  2 +-
 drivers/gpu/drm/ttm/ttm_agp_backend.c    |  2 +-
 drivers/gpu/drm/ttm/ttm_bo.c             | 38 +++++++++++-------------
 drivers/gpu/drm/ttm/ttm_bo_util.c        |  2 +-
 drivers/gpu/drm/ttm/ttm_bo_vm.c          |  4 +--
 drivers/gpu/drm/ttm/ttm_execbuf_util.c   | 20 ++++---------
 drivers/gpu/drm/ttm/ttm_memory.c         |  2 +-
 drivers/gpu/drm/ttm/ttm_page_alloc.c     |  4 +--
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c |  4 +--
 include/drm/ttm/ttm_bo_driver.h          |  6 ++--
 include/drm/ttm/ttm_memory.h             |  1 -
 14 files changed, 37 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 6d11e1721147..03ec278b2080 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -68,7 +68,7 @@
  */
 static int amdgpu_gart_dummy_page_init(struct amdgpu_device *adev)
 {
-	struct page *dummy_page = adev->mman.bdev.glob->dummy_read_page;
+	struct page *dummy_page = ttm_bo_glob.dummy_read_page;
 
 	if (adev->dummy_page_addr)
 		return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 56d838fe6633..a376adb426e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -600,7 +600,7 @@ void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo)
 void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
 				struct amdgpu_vm *vm)
 {
-	struct ttm_bo_global *glob = adev->mman.bdev.glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct amdgpu_vm_bo_base *bo_base;
 
 	if (vm->bulk_moveable) {
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 30f85f0130cb..2b6b0258a459 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -424,8 +424,8 @@ void qxl_release_unmap(struct qxl_device *qdev,
 
 void qxl_release_fence_buffer_objects(struct qxl_release *release)
 {
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct ttm_buffer_object *bo;
-	struct ttm_bo_global *glob;
 	struct ttm_bo_device *bdev;
 	struct ttm_validate_buffer *entry;
 	struct qxl_device *qdev;
@@ -447,8 +447,6 @@ void qxl_release_fence_buffer_objects(struct qxl_release *release)
 		       release->id | 0xf0000000, release->base.seqno);
 	trace_dma_fence_emit(&release->base);
 
-	glob = bdev->glob;
-
 	spin_lock(&glob->lru_lock);
 
 	list_for_each_entry(entry, &release->bos, head) {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 3bc6d53deb57..6225dbdcedc5 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -360,8 +360,8 @@ static int qxl_mm_dump_table(struct seq_file *m, void *data)
 	struct drm_mm *mm = (struct drm_mm *)node->info_ent->data;
 	struct drm_device *dev = node->minor->dev;
 	struct qxl_device *rdev = dev->dev_private;
-	struct ttm_bo_global *glob = rdev->mman.bdev.glob;
 	struct drm_printer p = drm_seq_file_printer(m);
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 
 	spin_lock(&glob->lru_lock);
 	drm_mm_print(mm, &p);
diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c b/drivers/gpu/drm/ttm/ttm_agp_backend.c
index ea4d59eb8966..6050dc846894 100644
--- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
+++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
@@ -51,7 +51,7 @@ struct ttm_agp_backend {
 static int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_mem_reg *bo_mem)
 {
 	struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
-	struct page *dummy_read_page = ttm->bdev->glob->dummy_read_page;
+	struct page *dummy_read_page = ttm_bo_glob.dummy_read_page;
 	struct drm_mm_node *node = bo_mem->mm_node;
 	struct agp_memory *mem;
 	int ret, cached = (bo_mem->placement & TTM_PL_FLAG_CACHED);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2845fceb2fbd..e2f9abe3920d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -51,6 +51,7 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj);
 DEFINE_MUTEX(ttm_global_mutex);
 unsigned ttm_bo_glob_use_count;
 struct ttm_bo_global ttm_bo_glob;
+EXPORT_SYMBOL(ttm_bo_glob);
 
 static struct attribute ttm_bo_count = {
 	.name = "bo_count",
@@ -148,7 +149,6 @@ static void ttm_bo_release_list(struct kref *list_kref)
 {
 	struct ttm_buffer_object *bo =
 	    container_of(list_kref, struct ttm_buffer_object, list_kref);
-	struct ttm_bo_device *bdev = bo->bdev;
 	size_t acc_size = bo->acc_size;
 
 	BUG_ON(kref_read(&bo->list_kref));
@@ -158,12 +158,12 @@ static void ttm_bo_release_list(struct kref *list_kref)
 	BUG_ON(!list_empty(&bo->lru));
 	BUG_ON(!list_empty(&bo->ddestroy));
 	ttm_tt_destroy(bo->ttm);
-	atomic_dec(&bo->bdev->glob->bo_count);
+	atomic_dec(&ttm_bo_glob.bo_count);
 	dma_fence_put(bo->moving);
 	reservation_object_fini(&bo->ttm_resv);
 	mutex_destroy(&bo->wu_mutex);
 	bo->destroy(bo);
-	ttm_mem_global_free(bdev->glob->mem_glob, acc_size);
+	ttm_mem_global_free(&ttm_mem_glob, acc_size);
 }
 
 void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
@@ -183,7 +183,7 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
 		if (bo->ttm && !(bo->ttm->page_flags &
 				 (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) {
 			list_add_tail(&bo->swap,
-				      &bdev->glob->swap_lru[bo->priority]);
+				      &ttm_bo_glob.swap_lru[bo->priority]);
 			kref_get(&bo->list_kref);
 		}
 	}
@@ -217,7 +217,7 @@ void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
 
 void ttm_bo_del_sub_from_lru(struct ttm_buffer_object *bo)
 {
-	struct ttm_bo_global *glob = bo->bdev->glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 
 	spin_lock(&glob->lru_lock);
 	ttm_bo_del_from_lru(bo);
@@ -302,7 +302,7 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
 		reservation_object_assert_held(pos->first->resv);
 		reservation_object_assert_held(pos->last->resv);
 
-		lru = &pos->first->bdev->glob->swap_lru[i];
+		lru = &ttm_bo_glob.swap_lru[i];
 		list_bulk_move_tail(lru, &pos->first->swap, &pos->last->swap);
 	}
 }
@@ -466,7 +466,7 @@ static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo)
 static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
-	struct ttm_bo_global *glob = bdev->glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	int ret;
 
 	ret = ttm_bo_individualize_resv(bo);
@@ -537,7 +537,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
 			       bool interruptible, bool no_wait_gpu,
 			       bool unlock_resv)
 {
-	struct ttm_bo_global *glob = bo->bdev->glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct reservation_object *resv;
 	int ret;
 
@@ -609,7 +609,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
  */
 static bool ttm_bo_delayed_delete(struct ttm_bo_device *bdev, bool remove_all)
 {
-	struct ttm_bo_global *glob = bdev->glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct list_head removed;
 	bool empty;
 
@@ -789,7 +789,7 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
 			       const struct ttm_place *place,
 			       struct ttm_operation_ctx *ctx)
 {
-	struct ttm_bo_global *glob = bdev->glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
 	struct ttm_buffer_object *bo = NULL;
 	bool locked = false;
@@ -1199,9 +1199,9 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
 			 struct reservation_object *resv,
 			 void (*destroy) (struct ttm_buffer_object *))
 {
+	struct ttm_mem_global *mem_glob = &ttm_mem_glob;
 	int ret = 0;
 	unsigned long num_pages;
-	struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
 	bool locked;
 
 	ret = ttm_mem_global_alloc(mem_glob, acc_size, ctx);
@@ -1255,7 +1255,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
 		bo->resv = &bo->ttm_resv;
 	}
 	reservation_object_init(&bo->ttm_resv);
-	atomic_inc(&bo->bdev->glob->bo_count);
+	atomic_inc(&ttm_bo_glob.bo_count);
 	drm_vma_node_reset(&bo->vma_node);
 
 	/*
@@ -1287,9 +1287,9 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
 	}
 
 	if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
-		spin_lock(&bdev->glob->lru_lock);
+		spin_lock(&ttm_bo_glob.lru_lock);
 		ttm_bo_add_to_lru(bo);
-		spin_unlock(&bdev->glob->lru_lock);
+		spin_unlock(&ttm_bo_glob.lru_lock);
 	}
 
 	return ret;
@@ -1388,7 +1388,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev,
 		.flags = TTM_OPT_FLAG_FORCE_ALLOC
 	};
 	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
-	struct ttm_bo_global *glob = bdev->glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct dma_fence *fence;
 	int ret;
 	unsigned i;
@@ -1556,8 +1556,6 @@ static int ttm_bo_global_init(void)
 		goto out;
 
 	spin_lock_init(&glob->lru_lock);
-	glob->mem_glob = &ttm_mem_glob;
-	glob->mem_glob->bo_glob = glob;
 	glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32);
 
 	if (unlikely(glob->dummy_read_page == NULL)) {
@@ -1581,10 +1579,10 @@ static int ttm_bo_global_init(void)
 
 int ttm_bo_device_release(struct ttm_bo_device *bdev)
 {
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	int ret = 0;
 	unsigned i = TTM_NUM_MEM_TYPES;
 	struct ttm_mem_type_manager *man;
-	struct ttm_bo_global *glob = bdev->glob;
 
 	while (i--) {
 		man = &bdev->man[i];
@@ -1653,7 +1651,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
 	INIT_LIST_HEAD(&bdev->ddestroy);
 	bdev->dev_mapping = mapping;
-	bdev->glob = glob;
 	bdev->need_dma32 = need_dma32;
 	mutex_lock(&ttm_global_mutex);
 	list_add_tail(&bdev->device_list, &glob->device_list);
@@ -1856,8 +1853,7 @@ void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
 		.no_wait_gpu = false
 	};
 
-	while (ttm_bo_swapout(bdev->glob, &ctx) == 0)
-		;
+	while (ttm_bo_swapout(&ttm_bo_glob, &ctx) == 0);
 }
 EXPORT_SYMBOL(ttm_bo_swapout_all);
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 895d77d799e4..57cc808270cb 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -503,7 +503,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
 	 * TODO: Explicit member copy would probably be better here.
 	 */
 
-	atomic_inc(&bo->bdev->glob->bo_count);
+	atomic_inc(&ttm_bo_glob.bo_count);
 	INIT_LIST_HEAD(&fbo->base.ddestroy);
 	INIT_LIST_HEAD(&fbo->base.lru);
 	INIT_LIST_HEAD(&fbo->base.swap);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 6dacff49c1cc..19f049855f97 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -177,9 +177,9 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
 		}
 
 		if (bo->moving != moving) {
-			spin_lock(&bdev->glob->lru_lock);
+			spin_lock(&ttm_bo_glob.lru_lock);
 			ttm_bo_move_to_lru_tail(bo, NULL);
-			spin_unlock(&bdev->glob->lru_lock);
+			spin_unlock(&ttm_bo_glob.lru_lock);
 		}
 		dma_fence_put(moving);
 	}
diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 0075eb9a0b52..62dbdacf44e2 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -56,15 +56,12 @@ static void ttm_eu_del_from_lru_locked(struct list_head *list)
 void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket,
 				struct list_head *list)
 {
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct ttm_validate_buffer *entry;
-	struct ttm_bo_global *glob;
 
 	if (list_empty(list))
 		return;
 
-	entry = list_first_entry(list, struct ttm_validate_buffer, head);
-	glob = entry->bo->bdev->glob;
-
 	spin_lock(&glob->lru_lock);
 	list_for_each_entry(entry, list, head) {
 		struct ttm_buffer_object *bo = entry->bo;
@@ -95,16 +92,13 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 			   struct list_head *list, bool intr,
 			   struct list_head *dups)
 {
-	struct ttm_bo_global *glob;
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct ttm_validate_buffer *entry;
 	int ret;
 
 	if (list_empty(list))
 		return 0;
 
-	entry = list_first_entry(list, struct ttm_validate_buffer, head);
-	glob = entry->bo->bdev->glob;
-
 	if (ticket)
 		ww_acquire_init(ticket, &reservation_ww_class);
 
@@ -185,20 +179,16 @@ void ttm_eu_fence_buffer_objects(struct ww_acquire_ctx *ticket,
 				 struct list_head *list,
 				 struct dma_fence *fence)
 {
+	struct ttm_bo_global *glob = &ttm_bo_glob;
 	struct ttm_validate_buffer *entry;
-	struct ttm_buffer_object *bo;
-	struct ttm_bo_global *glob;
 
 	if (list_empty(list))
 		return;
 
-	bo = list_first_entry(list, struct ttm_validate_buffer, head)->bo;
-	glob = bo->bdev->glob;
-
 	spin_lock(&glob->lru_lock);
-
 	list_for_each_entry(entry, list, head) {
-		bo = entry->bo;
+		struct ttm_buffer_object *bo = entry->bo;
+
 		if (entry->num_shared)
 			reservation_object_add_shared_fence(bo->resv, fence);
 		else
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 8617958b7ae6..acd63b70d814 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -275,7 +275,7 @@ static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
 
 	while (ttm_zones_above_swap_target(glob, from_wq, extra)) {
 		spin_unlock(&glob->lock);
-		ret = ttm_bo_swapout(glob->bo_glob, ctx);
+		ret = ttm_bo_swapout(&ttm_bo_glob, ctx);
 		spin_lock(&glob->lock);
 		if (unlikely(ret != 0))
 			break;
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 627f8dc91d0e..b40a4678c296 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -1028,7 +1028,7 @@ void ttm_page_alloc_fini(void)
 static void
 ttm_pool_unpopulate_helper(struct ttm_tt *ttm, unsigned mem_count_update)
 {
-	struct ttm_mem_global *mem_glob = ttm->bdev->glob->mem_glob;
+	struct ttm_mem_global *mem_glob = &ttm_mem_glob;
 	unsigned i;
 
 	if (mem_count_update == 0)
@@ -1049,7 +1049,7 @@ ttm_pool_unpopulate_helper(struct ttm_tt *ttm, unsigned mem_count_update)
 
 int ttm_pool_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
 {
-	struct ttm_mem_global *mem_glob = ttm->bdev->glob->mem_glob;
+	struct ttm_mem_global *mem_glob = &ttm_mem_glob;
 	unsigned i;
 	int ret;
 
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index d594f7520b7b..9cb588a93d43 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -882,8 +882,8 @@ static gfp_t ttm_dma_pool_gfp_flags(struct ttm_dma_tt *ttm_dma, bool huge)
 int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev,
 			struct ttm_operation_ctx *ctx)
 {
+	struct ttm_mem_global *mem_glob = &ttm_mem_glob;
 	struct ttm_tt *ttm = &ttm_dma->ttm;
-	struct ttm_mem_global *mem_glob = ttm->bdev->glob->mem_glob;
 	unsigned long num_pages = ttm->num_pages;
 	struct dma_pool *pool;
 	struct dma_page *d_page;
@@ -987,8 +987,8 @@ EXPORT_SYMBOL_GPL(ttm_dma_populate);
 /* Put all pages in pages list to correct pool to wait for reuse */
 void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev)
 {
+	struct ttm_mem_global *mem_glob = &ttm_mem_glob;
 	struct ttm_tt *ttm = &ttm_dma->ttm;
-	struct ttm_mem_global *mem_glob = ttm->bdev->glob->mem_glob;
 	struct dma_pool *pool;
 	struct dma_page *d_page, *next;
 	enum pool_type type;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index c008346c2401..e0c1cae7fadc 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -413,7 +413,6 @@ extern struct ttm_bo_global {
 	 */
 
 	struct kobject kobj;
-	struct ttm_mem_global *mem_glob;
 	struct page *dummy_read_page;
 	spinlock_t lru_lock;
 
@@ -457,7 +456,6 @@ struct ttm_bo_device {
 	 * Constant after bo device init / atomic.
 	 */
 	struct list_head device_list;
-	struct ttm_bo_global *glob;
 	struct ttm_bo_driver *driver;
 	struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES];
 
@@ -768,9 +766,9 @@ static inline int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo,
 static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
 {
 	if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
-		spin_lock(&bo->bdev->glob->lru_lock);
+		spin_lock(&ttm_bo_glob.lru_lock);
 		ttm_bo_add_to_lru(bo);
-		spin_unlock(&bo->bdev->glob->lru_lock);
+		spin_unlock(&ttm_bo_glob.lru_lock);
 	}
 	reservation_object_unlock(bo->resv);
 }
diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
index 3ff48a0a2d7b..c78ea99c42cf 100644
--- a/include/drm/ttm/ttm_memory.h
+++ b/include/drm/ttm/ttm_memory.h
@@ -65,7 +65,6 @@
 struct ttm_mem_zone;
 extern struct ttm_mem_global {
 	struct kobject kobj;
-	struct ttm_bo_global *bo_glob;
 	struct workqueue_struct *swap_queue;
 	struct work_struct work;
 	spinlock_t lock;
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] drm/ttm: fix re-init of global structures
  2019-04-16 10:53 [PATCH 1/2] drm/ttm: fix re-init of global structures Christian König
  2019-04-16 10:53 ` [PATCH 2/2] drm/ttm: remove pointers to globals Christian König
@ 2019-04-16 11:34 ` Karol Herbst
  1 sibling, 0 replies; 3+ messages in thread
From: Karol Herbst @ 2019-04-16 11:34 UTC (permalink / raw)
  To: Christian König; +Cc: itsbriany, dri-devel

On Tue, Apr 16, 2019 at 12:53 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> When a driver unloads without unloading TTM we don't correctly
> clear the global structures leading to errors on re-init.
>
> Next step should probably be to remove the global structures and
> kobjs all together, but this is tricky since we need to maintain
> backward compatibility.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c     | 10 +++++-----
>  drivers/gpu/drm/ttm/ttm_memory.c |  5 +++--
>  include/drm/ttm/ttm_bo_driver.h  |  1 -
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 41d07faa2eae..2845fceb2fbd 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -49,9 +49,8 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj);
>   * ttm_global_mutex - protecting the global BO state
>   */
>  DEFINE_MUTEX(ttm_global_mutex);
> -struct ttm_bo_global ttm_bo_glob = {
> -       .use_count = 0
> -};
> +unsigned ttm_bo_glob_use_count;
> +struct ttm_bo_global ttm_bo_glob;
>
>  static struct attribute ttm_bo_count = {
>         .name = "bo_count",
> @@ -1531,12 +1530,13 @@ static void ttm_bo_global_release(void)
>         struct ttm_bo_global *glob = &ttm_bo_glob;
>
>         mutex_lock(&ttm_global_mutex);
> -       if (--glob->use_count > 0)
> +       if (--ttm_bo_glob_use_count > 0)
>                 goto out;
>
>         kobject_del(&glob->kobj);
>         kobject_put(&glob->kobj);
>         ttm_mem_global_release(&ttm_mem_glob);
> +       memset(glob, 0, sizeof(*glob));
>  out:
>         mutex_unlock(&ttm_global_mutex);
>  }
> @@ -1548,7 +1548,7 @@ static int ttm_bo_global_init(void)
>         unsigned i;
>
>         mutex_lock(&ttm_global_mutex);
> -       if (++glob->use_count > 1)
> +       if (++ttm_bo_glob_use_count > 1)
>                 goto out;
>
>         ret = ttm_mem_global_init(&ttm_mem_glob);
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index 699fed9e08ee..8617958b7ae6 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -461,8 +461,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
>
>  void ttm_mem_global_release(struct ttm_mem_global *glob)
>  {
> -       unsigned int i;
>         struct ttm_mem_zone *zone;
> +       unsigned int i;

comment below

>
>         /* let the page allocator first stop the shrink work. */
>         ttm_page_alloc_fini();
> @@ -475,9 +475,10 @@ void ttm_mem_global_release(struct ttm_mem_global *glob)
>                 zone = glob->zones[i];
>                 kobject_del(&zone->kobj);
>                 kobject_put(&zone->kobj);
> -                       }
> +       }

comment below

>         kobject_del(&glob->kobj);
>         kobject_put(&glob->kobj);
> +       memset(glob, 0, sizeof(*glob));
>  }
>
>  static void ttm_check_swapping(struct ttm_mem_global *glob)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 3db265bf2d4e..c008346c2401 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -420,7 +420,6 @@ extern struct ttm_bo_global {
>         /**
>          * Protected by ttm_global_mutex.
>          */
> -       unsigned int use_count;
>         struct list_head device_list;
>
>         /**
> --
> 2.17.1
>
tested on 5.0.7 and it fixes the crash I've encountered, so:
Tested-by: Karol Herbst <kherbst@redhat.com>

Personally I think we should split formatting and other unrelated
changes out of a patch for stable, but those are still fine changes,
anyway:
Reviewed-by: Karol Herbst <kherbst@redhat.com>

Thanks!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-04-16 11:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 10:53 [PATCH 1/2] drm/ttm: fix re-init of global structures Christian König
2019-04-16 10:53 ` [PATCH 2/2] drm/ttm: remove pointers to globals Christian König
2019-04-16 11:34 ` [PATCH 1/2] drm/ttm: fix re-init of global structures Karol Herbst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).