Re: [patch] paravirt: isolate module ops

[Christoph Hellwig <hch@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

On Sat, Jan 06, 2007 at 07:31:52PM +0000, Christoph Hellwig wrote:
> On Sun, Jan 07, 2007 at 03:18:45AM +1100, Rusty Russell wrote:
> > PS.  drm_memory.h has a "drm_follow_page": this forces us to uninline
> > various page tables ops.  Can this use follow_page() somehow, or do we
> > need an "__follow_page" export for this case?
> 
> Not if avoidable.  And it seems avoidable as drm really should be using
> vmalloc_to_page.  Untested patch below:

Even better we can actualy avid most of the page table walks completely.
First there is a number of places that can never have the vmalloc case
an may use ioremap/iounmap directly.  Secondly drm_core_ioremap/
drm_core_ioremapfree already have the right drm_map to check wich kind
of mapping we have - we just need to use it instead of discarding that
information!  The only leaves direct drm_ioremapfree in i810_dma.c and
i830_dma.c which I don't quite manage to follow.  Maybe Dave has an
idea how to get rid of those aswell.

[-- Attachment #2: drm-avoid-drm_ioremap --]
[-- Type: text/plain, Size: 1987 bytes --]

Index: linux-2.6/drivers/char/drm/drm_bufs.c
===================================================================
--- linux-2.6.orig/drivers/char/drm/drm_bufs.c	2007-01-07 14:07:18.000000000 +0100
+++ linux-2.6/drivers/char/drm/drm_bufs.c	2007-01-07 14:09:40.000000000 +0100
@@ -178,7 +178,7 @@
 			}
 		}
 		if (map->type == _DRM_REGISTERS)
-			map->handle = drm_ioremap(map->offset, map->size, dev);
+			map->handle = ioremap(map->offset, map->size);
 		break;
 
 	case _DRM_SHM:
@@ -238,7 +238,7 @@
 	list = drm_alloc(sizeof(*list), DRM_MEM_MAPS);
 	if (!list) {
 		if (map->type == _DRM_REGISTERS)
-			drm_ioremapfree(map->handle, map->size, dev);
+			iounmap(map->handle);
 		drm_free(map, sizeof(*map), DRM_MEM_MAPS);
 		return -EINVAL;
 	}
@@ -255,7 +255,7 @@
 	ret = drm_map_handle(dev, &list->hash, user_token, 0);
 	if (ret) {
 		if (map->type == _DRM_REGISTERS)
-			drm_ioremapfree(map->handle, map->size, dev);
+			iounmap(map->handle);
 		drm_free(map, sizeof(*map), DRM_MEM_MAPS);
 		drm_free(list, sizeof(*list), DRM_MEM_MAPS);
 		mutex_unlock(&dev->struct_mutex);
@@ -362,7 +362,7 @@
 
 	switch (map->type) {
 	case _DRM_REGISTERS:
-		drm_ioremapfree(map->handle, map->size, dev);
+		iounmap(map->handle);
 		/* FALLTHROUGH */
 	case _DRM_FRAME_BUFFER:
 		if (drm_core_has_MTRR(dev) && map->mtrr >= 0) {
Index: linux-2.6/drivers/char/drm/drm_vm.c
===================================================================
--- linux-2.6.orig/drivers/char/drm/drm_vm.c	2007-01-07 14:07:18.000000000 +0100
+++ linux-2.6/drivers/char/drm/drm_vm.c	2007-01-07 14:10:50.000000000 +0100
@@ -227,7 +227,12 @@
 							   map->size);
 					DRM_DEBUG("mtrr_del = %d\n", retcode);
 				}
-				drm_ioremapfree(map->handle, map->size, dev);
+				/*
+				 * XXX(hch) autmatic mapping/unmapping only happens for
+				 * _DRM_REGISTERS in all other places.  Should we have
+				 * this check here aswell?
+				 */
+				iounmap(map->handle);
 				break;
 			case _DRM_SHM:
 				vfree(map->handle);

[-- Attachment #3: simplify-drm_core_ioremap --]
[-- Type: text/plain, Size: 2816 bytes --]

Index: linux-2.6/drivers/char/drm/drmP.h
===================================================================
--- linux-2.6.orig/drivers/char/drm/drmP.h	2007-01-07 14:13:48.000000000 +0100
+++ linux-2.6/drivers/char/drm/drmP.h	2007-01-07 14:15:32.000000000 +0100
@@ -1059,27 +1059,8 @@
 extern int drm_mm_init(drm_mm_t *mm, unsigned long start, unsigned long size);
 extern void drm_mm_takedown(drm_mm_t *mm);
 
-/* Inline replacements for DRM_IOREMAP macros */
-static __inline__ void drm_core_ioremap(struct drm_map *map,
-					struct drm_device *dev)
-{
-	map->handle = drm_ioremap(map->offset, map->size, dev);
-}
-
-#if 0
-static __inline__ void drm_core_ioremap_nocache(struct drm_map *map,
-						struct drm_device *dev)
-{
-	map->handle = drm_ioremap_nocache(map->offset, map->size, dev);
-}
-#endif  /*  0  */
-
-static __inline__ void drm_core_ioremapfree(struct drm_map *map,
-					    struct drm_device *dev)
-{
-	if (map->handle && map->size)
-		drm_ioremapfree(map->handle, map->size, dev);
-}
+extern void drm_core_ioremap(struct drm_map *map, struct drm_device *dev);
+extern void drm_core_ioremapfree(struct drm_map *map, struct drm_device *dev);
 
 static __inline__ struct drm_map *drm_core_findmap(struct drm_device *dev,
 						   unsigned int token)
Index: linux-2.6/drivers/char/drm/drm_memory.c
===================================================================
--- linux-2.6.orig/drivers/char/drm/drm_memory.c	2007-01-07 14:13:48.000000000 +0100
+++ linux-2.6/drivers/char/drm/drm_memory.c	2007-01-07 14:19:34.000000000 +0100
@@ -197,20 +197,6 @@
 }
 EXPORT_SYMBOL(drm_ioremap);
 
-#if 0
-void *drm_ioremap_nocache(unsigned long offset,
-					unsigned long size, drm_device_t * dev)
-{
-	if (drm_core_has_AGP(dev) && dev->agp && dev->agp->cant_use_aperture) {
-		drm_map_t *map = drm_lookup_map(offset, size, dev);
-
-		if (map && map->type == _DRM_AGP)
-			return agp_remap(offset, size, dev);
-	}
-	return ioremap_nocache(offset, size);
-}
-#endif  /*  0  */
-
 void drm_ioremapfree(void *pt, unsigned long size,
 				   drm_device_t * dev)
 {
@@ -238,3 +224,26 @@
 EXPORT_SYMBOL(drm_ioremapfree);
 
 #endif				/* debug_memory */
+
+void drm_core_ioremap(struct drm_map *map, struct drm_device *dev)
+{
+	if (drm_core_has_AGP(dev) &&
+	    dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
+		map->handle = agp_remap(map->offset, map->size, dev);
+	else
+		map->handle = ioremap(map->offset, map->size);
+}
+EXPORT_SYMBOL_GPL(drm_core_ioremap);
+
+void drm_core_ioremapfree(struct drm_map *map, struct drm_device *dev)
+{
+	if (!map->handle || !map->size)
+		return;
+
+	if (drm_core_has_AGP(dev) &&
+	    dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
+		vunmap(map->handle);
+	else
+		iounmap(map->handle);
+}
+EXPORT_SYMBOL_GPL(drm_core_ioremapfree);