diff mbox

[23/48] staging: etnaviv: fix oops caused by scanning for free blocks

Message ID 1443182280-15868-24-git-send-email-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Stach Sept. 25, 2015, 11:57 a.m. UTC
From: Russell King <rmk+kernel@arm.linux.org.uk>

The drm_mm scanning for free MMU blocks walks the list of inactive
objects, looking up their vram node.  Unfortunately, the code assumes
that if it has a vram node, it is registered into drm_mm.

However, there are two cases when a vram node is created - whenever a
MMU entry has to be allocated (when it is registered into the drm_mm)
and when a direct mapping is registered (which isn't.)  The direct
mapping case has vram_node.mm NULL, which causes an oops in
drm_mm_scan_add_block().

The list of objects on the scanning list was also broken - we would end
up calling drm_mm_remove_block() multiple times for the same 'free'
vram node, instead of walking the list of scanned blocks.  Fix this by
having a separate list for scanned blocks.

Fixes: ("staging: etnaviv: allow to map buffer object into multiple address spaces")
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/staging/etnaviv/etnaviv_gem.c |  4 ++--
 drivers/staging/etnaviv/etnaviv_gem.h |  5 ++++-
 drivers/staging/etnaviv/etnaviv_mmu.c | 36 +++++++++++++++++++++--------------
 3 files changed, 28 insertions(+), 17 deletions(-)
diff mbox

Patch

diff --git a/drivers/staging/etnaviv/etnaviv_gem.c b/drivers/staging/etnaviv/etnaviv_gem.c
index 23e655de7925..256ec0775133 100644
--- a/drivers/staging/etnaviv/etnaviv_gem.c
+++ b/drivers/staging/etnaviv/etnaviv_gem.c
@@ -508,7 +508,7 @@  static void etnaviv_free_obj(struct drm_gem_object *obj)
 	struct etnaviv_vram_mapping *mapping, *tmp;
 
 	list_for_each_entry_safe(mapping, tmp, &etnaviv_obj->vram_list,
-				 obj_head) {
+				 obj_node) {
 		etnaviv_iommu_unmap_gem(mapping->mmu, etnaviv_obj, mapping);
 	}
 }
@@ -722,7 +722,7 @@  etnaviv_gem_get_vram_mapping(struct etnaviv_gem_object *obj,
 {
 	struct etnaviv_vram_mapping *mapping;
 
-	list_for_each_entry(mapping, &obj->vram_list, obj_head) {
+	list_for_each_entry(mapping, &obj->vram_list, obj_node) {
 		if (mapping->mmu == mmu)
 			return mapping;
 	}
diff --git a/drivers/staging/etnaviv/etnaviv_gem.h b/drivers/staging/etnaviv/etnaviv_gem.h
index 08f0e6464cc7..f3136d86d73e 100644
--- a/drivers/staging/etnaviv/etnaviv_gem.h
+++ b/drivers/staging/etnaviv/etnaviv_gem.h
@@ -21,6 +21,7 @@ 
 #include "etnaviv_drv.h"
 
 struct etnaviv_gem_ops;
+struct etnaviv_gem_object;
 
 struct etnaviv_gem_userptr {
 	uintptr_t ptr;
@@ -30,7 +31,9 @@  struct etnaviv_gem_userptr {
 };
 
 struct etnaviv_vram_mapping {
-	struct list_head obj_head;
+	struct list_head obj_node;
+	struct list_head scan_node;
+	struct etnaviv_gem_object *object;
 	struct etnaviv_iommu *mmu;
 	struct drm_mm_node vram_node;
 	u32 iova;
diff --git a/drivers/staging/etnaviv/etnaviv_mmu.c b/drivers/staging/etnaviv/etnaviv_mmu.c
index f40d4ec5ade7..b327d37f5111 100644
--- a/drivers/staging/etnaviv/etnaviv_mmu.c
+++ b/drivers/staging/etnaviv/etnaviv_mmu.c
@@ -104,6 +104,8 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 	if (!mapping)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&mapping->scan_node);
+	mapping->object = etnaviv_obj;
 	mapping->mmu = mmu;
 
 	/* v1 MMU can optimize single entry (contiguous) scatterlists */
@@ -113,7 +115,7 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 		iova = sg_dma_address(sgt->sgl) - memory_base;
 		if (iova < 0x80000000 - sg_dma_len(sgt->sgl)) {
 			mapping->iova = iova;
-			list_add_tail(&mapping->obj_head,
+			list_add_tail(&mapping->obj_node,
 				      &etnaviv_obj->vram_list);
 			if (out_mapping)
 				*out_mapping = mapping;
@@ -123,7 +125,8 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 
 	node = &mapping->vram_node;
 	while (1) {
-		struct etnaviv_gem_object *o, *n;
+		struct etnaviv_gem_object *o;
+		struct etnaviv_vram_mapping *m, *n;
 		struct list_head list;
 		bool found;
 
@@ -155,13 +158,19 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 				continue;
 
 			/*
+			 * If this vram node has not been used, skip this.
+			 */
+			if (!free->vram_node.mm)
+				continue;
+
+			/*
 			 * If it's on the submit list, then it is part of
 			 * a submission, and we want to keep its entry.
 			 */
 			if (!list_empty(&o->submit_entry))
 				continue;
 
-			list_add(&o->submit_entry, &list);
+			list_add(&free->scan_node, &list);
 			if (drm_mm_scan_add_block(&free->vram_node)) {
 				found = true;
 				break;
@@ -170,8 +179,8 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 
 		if (!found) {
 			/* Nothing found, clean up and fail */
-			list_for_each_entry_safe(o, n, &list, submit_entry)
-				BUG_ON(drm_mm_scan_remove_block(&free->vram_node));
+			list_for_each_entry_safe(m, n, &list, scan_node)
+				BUG_ON(drm_mm_scan_remove_block(&m->vram_node));
 			break;
 		}
 
@@ -181,13 +190,13 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 		 * If drm_mm_scan_remove_block() returns false, we
 		 * can leave the block pinned.
 		 */
-		list_for_each_entry_safe(o, n, &list, submit_entry)
-			if (!drm_mm_scan_remove_block(&free->vram_node))
-				list_del_init(&o->submit_entry);
+		list_for_each_entry_safe(m, n, &list, scan_node)
+			if (!drm_mm_scan_remove_block(&m->vram_node))
+				list_del_init(&m->scan_node);
 
-		list_for_each_entry_safe(o, n, &list, submit_entry) {
-			list_del_init(&o->submit_entry);
-			etnaviv_iommu_unmap_gem(mmu, o, free);
+		list_for_each_entry_safe(m, n, &list, scan_node) {
+			list_del_init(&m->scan_node);
+			etnaviv_iommu_unmap_gem(mmu, m->object, m);
 		}
 
 		/*
@@ -214,7 +223,7 @@  int etnaviv_iommu_map_gem(struct etnaviv_iommu *mmu,
 		return ret;
 	}
 
-	list_add_tail(&mapping->obj_head, &etnaviv_obj->vram_list);
+	list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
 	if (out_mapping)
 		*out_mapping = mapping;
 
@@ -233,9 +242,8 @@  void etnaviv_iommu_unmap_gem(struct etnaviv_iommu *mmu,
 					    etnaviv_obj->base.size);
 			drm_mm_remove_node(&mapping->vram_node);
 		}
-		list_del(&mapping->obj_head);
+		list_del(&mapping->obj_node);
 		kfree(mapping);
-		mapping = NULL;
 	}
 }