[PATCH] drm: Eliminate dev->vmalist

Chris Wilson chris at chris-wilson.co.uk
Fri Aug 16 04:24:37 PDT 2013


The primary purpose of this list is for pretty printing the maps through
debugfs/vma - per process information can be found in /proc/*/map et al.
However, the list manipulation eats processor cycles resulting in a near
indefinite (I got bored) stall for a leaky i-g-t/gem_concurrent_blit.

There is one true user of the vmalist though. drm_vm_shm_close() uses
the list to count the number of drm_local_map references. For this, we
switch to using a counter inside drm_local_map.

This patch kills drm_vm_open_locked and drm_vm_close_locked, and
redirects the one user outside of the core (exoynos) to use the now
semantically equivalent drm_gem_vm_open.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Dave Airlie <airlied at redhat.com>
Cc: Rob Clark <robdclark at gmail.com>
Cc: Inki Dae <inki.dae at samsung.com>
---
 drivers/gpu/drm/drm_bufs.c              |  1 +
 drivers/gpu/drm/drm_drv.c               |  8 -----
 drivers/gpu/drm/drm_gem.c               | 16 +++------
 drivers/gpu/drm/drm_info.c              | 38 ---------------------
 drivers/gpu/drm/drm_stub.c              |  1 -
 drivers/gpu/drm/drm_vm.c                | 58 +++++++--------------------------
 drivers/gpu/drm/exynos/exynos_drm_gem.c |  9 +----
 include/drm/drmP.h                      |  7 ++--
 8 files changed, 20 insertions(+), 118 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index bef4abf..b0aa2eb 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -152,6 +152,7 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
 	map->size = size;
 	map->flags = flags;
 	map->type = type;
+	map->count = 0;
 
 	/* Only allow shared memory to be removable since we only keep enough
 	 * book keeping information about shared memory to allow for removal
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dddd799..7ec9959 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -182,8 +182,6 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  */
 int drm_lastclose(struct drm_device * dev)
 {
-	struct drm_vma_entry *vma, *vma_temp;
-
 	DRM_DEBUG("\n");
 
 	if (dev->driver->lastclose)
@@ -203,12 +201,6 @@ int drm_lastclose(struct drm_device * dev)
 		dev->sg = NULL;
 	}
 
-	/* Clear vma list (only built for debugging) */
-	list_for_each_entry_safe(vma, vma_temp, &dev->vmalist, head) {
-		list_del(&vma->head);
-		kfree(vma);
-	}
-
 	if (drm_core_check_feature(dev, DRIVER_HAVE_DMA) &&
 	    !drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_dma_takedown(dev);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 9ab038c..84a5834 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -573,22 +573,16 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
 	struct drm_gem_object *obj = vma->vm_private_data;
 
 	drm_gem_object_reference(obj);
-
-	mutex_lock(&obj->dev->struct_mutex);
-	drm_vm_open_locked(obj->dev, vma);
-	mutex_unlock(&obj->dev->struct_mutex);
+	drm_vma_open(obj->dev, vma);
 }
 EXPORT_SYMBOL(drm_gem_vm_open);
 
 void drm_gem_vm_close(struct vm_area_struct *vma)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
-	struct drm_device *dev = obj->dev;
 
-	mutex_lock(&dev->struct_mutex);
-	drm_vm_close_locked(obj->dev, vma);
-	drm_gem_object_unreference(obj);
-	mutex_unlock(&dev->struct_mutex);
+	drm_vma_close(obj->dev, vma);
+	drm_gem_object_unreference_unlocked(obj);
 }
 EXPORT_SYMBOL(drm_gem_vm_close);
 
@@ -639,9 +633,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 	 * (which should happen whether the vma was created by this call, or
 	 * by a vm_open due to mremap or partial unmap or whatever).
 	 */
-	drm_gem_object_reference(obj);
-
-	drm_vm_open_locked(dev, vma);
+	drm_gem_vm_open(vma);
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_mmap_obj);
diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index d4b20ce..6ea817c 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -228,49 +228,11 @@ int drm_vma_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
-	struct drm_vma_entry *pt;
-	struct vm_area_struct *vma;
-#if defined(__i386__)
-	unsigned int pgprot;
-#endif
 
-	mutex_lock(&dev->struct_mutex);
 	seq_printf(m, "vma use count: %d, high_memory = %pK, 0x%pK\n",
 		   atomic_read(&dev->vma_count),
 		   high_memory, (void *)(unsigned long)virt_to_phys(high_memory));
 
-	list_for_each_entry(pt, &dev->vmalist, head) {
-		vma = pt->vma;
-		if (!vma)
-			continue;
-		seq_printf(m,
-			   "\n%5d 0x%pK-0x%pK %c%c%c%c%c%c 0x%08lx000",
-			   pt->pid,
-			   (void *)vma->vm_start, (void *)vma->vm_end,
-			   vma->vm_flags & VM_READ ? 'r' : '-',
-			   vma->vm_flags & VM_WRITE ? 'w' : '-',
-			   vma->vm_flags & VM_EXEC ? 'x' : '-',
-			   vma->vm_flags & VM_MAYSHARE ? 's' : 'p',
-			   vma->vm_flags & VM_LOCKED ? 'l' : '-',
-			   vma->vm_flags & VM_IO ? 'i' : '-',
-			   vma->vm_pgoff);
-
-#if defined(__i386__)
-		pgprot = pgprot_val(vma->vm_page_prot);
-		seq_printf(m, " %c%c%c%c%c%c%c%c%c",
-			   pgprot & _PAGE_PRESENT ? 'p' : '-',
-			   pgprot & _PAGE_RW ? 'w' : 'r',
-			   pgprot & _PAGE_USER ? 'u' : 's',
-			   pgprot & _PAGE_PWT ? 't' : 'b',
-			   pgprot & _PAGE_PCD ? 'u' : 'c',
-			   pgprot & _PAGE_ACCESSED ? 'a' : '-',
-			   pgprot & _PAGE_DIRTY ? 'd' : '-',
-			   pgprot & _PAGE_PSE ? 'm' : 'k',
-			   pgprot & _PAGE_GLOBAL ? 'g' : 'l');
-#endif
-		seq_printf(m, "\n");
-	}
-	mutex_unlock(&dev->struct_mutex);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index d663f7d..0a1db60 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -258,7 +258,6 @@ int drm_fill_in_dev(struct drm_device *dev,
 
 	INIT_LIST_HEAD(&dev->filelist);
 	INIT_LIST_HEAD(&dev->ctxlist);
-	INIT_LIST_HEAD(&dev->vmalist);
 	INIT_LIST_HEAD(&dev->maplist);
 	INIT_LIST_HEAD(&dev->vblank_event_list);
 
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index feb2003..bc2b26f 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -213,7 +213,6 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
 {
 	struct drm_file *priv = vma->vm_file->private_data;
 	struct drm_device *dev = priv->minor->dev;
-	struct drm_vma_entry *pt, *temp;
 	struct drm_local_map *map;
 	struct drm_map_list *r_list;
 	int found_maps = 0;
@@ -225,17 +224,8 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
 	map = vma->vm_private_data;
 
 	mutex_lock(&dev->struct_mutex);
-	list_for_each_entry_safe(pt, temp, &dev->vmalist, head) {
-		if (pt->vma->vm_private_data == map)
-			found_maps++;
-		if (pt->vma == vma) {
-			list_del(&pt->head);
-			kfree(pt);
-		}
-	}
-
 	/* We were the only map that was found */
-	if (found_maps == 1 && map->flags & _DRM_REMOVABLE) {
+	if (--map->count == 0) {
 		/* Check to see if we are in the maplist, if we are not, then
 		 * we delete this mappings information.
 		 */
@@ -402,68 +392,41 @@ static const struct vm_operations_struct drm_vm_sg_ops = {
  * Create a new drm_vma_entry structure as the \p vma private data entry and
  * add it to drm_device::vmalist.
  */
-void drm_vm_open_locked(struct drm_device *dev,
-		struct vm_area_struct *vma)
+void drm_vma_open(struct drm_device *dev,
+		  struct vm_area_struct *vma)
 {
-	struct drm_vma_entry *vma_entry;
-
 	DRM_DEBUG("0x%08lx,0x%08lx\n",
 		  vma->vm_start, vma->vm_end - vma->vm_start);
 	atomic_inc(&dev->vma_count);
-
-	vma_entry = kmalloc(sizeof(*vma_entry), GFP_KERNEL);
-	if (vma_entry) {
-		vma_entry->vma = vma;
-		vma_entry->pid = current->pid;
-		list_add(&vma_entry->head, &dev->vmalist);
-	}
 }
-EXPORT_SYMBOL_GPL(drm_vm_open_locked);
 
 static void drm_vm_open(struct vm_area_struct *vma)
 {
 	struct drm_file *priv = vma->vm_file->private_data;
 	struct drm_device *dev = priv->minor->dev;
 
-	mutex_lock(&dev->struct_mutex);
-	drm_vm_open_locked(dev, vma);
-	mutex_unlock(&dev->struct_mutex);
+	drm_vma_open(dev, vma);
 }
 
-void drm_vm_close_locked(struct drm_device *dev,
-		struct vm_area_struct *vma)
+void drm_vma_close(struct drm_device *dev,
+		   struct vm_area_struct *vma)
 {
-	struct drm_vma_entry *pt, *temp;
-
 	DRM_DEBUG("0x%08lx,0x%08lx\n",
 		  vma->vm_start, vma->vm_end - vma->vm_start);
 	atomic_dec(&dev->vma_count);
-
-	list_for_each_entry_safe(pt, temp, &dev->vmalist, head) {
-		if (pt->vma == vma) {
-			list_del(&pt->head);
-			kfree(pt);
-			break;
-		}
-	}
 }
 
 /**
  * \c close method for all virtual memory types.
  *
  * \param vma virtual memory area.
- *
- * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
- * free it.
  */
 static void drm_vm_close(struct vm_area_struct *vma)
 {
 	struct drm_file *priv = vma->vm_file->private_data;
 	struct drm_device *dev = priv->minor->dev;
 
-	mutex_lock(&dev->struct_mutex);
-	drm_vm_close_locked(dev, vma);
-	mutex_unlock(&dev->struct_mutex);
+	drm_vma_close(dev, vma);
 }
 
 /**
@@ -513,7 +476,7 @@ static int drm_mmap_dma(struct file *filp, struct vm_area_struct *vma)
 
 	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
 
-	drm_vm_open_locked(dev, vma);
+	drm_vma_open(dev, vma);
 	return 0;
 }
 
@@ -543,7 +506,7 @@ int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma)
 {
 	struct drm_file *priv = filp->private_data;
 	struct drm_device *dev = priv->minor->dev;
-	struct drm_local_map *map = NULL;
+	struct drm_local_map *map;
 	resource_size_t offset = 0;
 	struct drm_hash_item *hash;
 
@@ -648,8 +611,9 @@ int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma)
 		return -EINVAL;	/* This should never happen. */
 	}
 	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+	map->count++;
 
-	drm_vm_open_locked(dev, vma);
+	drm_vma_open(dev, vma);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index b904633..04cb3ea 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -401,14 +401,7 @@ static int exynos_drm_gem_mmap_buffer(struct file *filp,
 		return ret;
 	}
 
-	/*
-	 * take a reference to this mapping of the object. And this reference
-	 * is unreferenced by the corresponding vm_close call.
-	 */
-	drm_gem_object_reference(obj);
-
-	drm_vm_open_locked(drm_dev, vma);
-
+	drm_gem_vm_open(vma);
 	return 0;
 }
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3ecdde6..e4b8658 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -573,6 +573,7 @@ struct drm_local_map {
 	void *handle;		 /**< User-space: "Handle" to pass to mmap() */
 				 /**< Kernel-space: kernel-virtual address */
 	int mtrr;		 /**< MTRR slot used */
+	int count;
 };
 
 typedef struct drm_local_map drm_local_map_t;
@@ -1116,8 +1117,6 @@ struct drm_device {
 
 	struct idr ctx_idr;
 
-	struct list_head vmalist;	/**< List of vmas (for debugging) */
-
 	/*@} */
 
 	/** \name DMA support */
@@ -1274,8 +1273,8 @@ extern int drm_release(struct inode *inode, struct file *filp);
 				/* Mapping support (drm_vm.h) */
 extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
 extern int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma);
-extern void drm_vm_open_locked(struct drm_device *dev, struct vm_area_struct *vma);
-extern void drm_vm_close_locked(struct drm_device *dev, struct vm_area_struct *vma);
+extern void drm_vma_open(struct drm_device *dev, struct vm_area_struct *vma);
+extern void drm_vma_close(struct drm_device *dev, struct vm_area_struct *vma);
 extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
 
 				/* Memory management support (drm_memory.h) */
-- 
1.8.4.rc2



More information about the dri-devel mailing list