Re: [patch] paravirt: isolate module ops
From: Christoph Hellwig
Date: Sun Jan 07 2007 - 13:41:44 EST
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.
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);
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);