Re: [PATCH 3/3] drm/i915: Preallocate mm node for GTT mmap offset

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, 19 Dec 2012 08:10:24 +1000, Dave Airlie <airlied@gmail.com> wrote:
> > As the shrinker may be invoked for the allocation, and it may reap
> > neighbouring objects in the offset range mm, we need to be careful in
> > the order in which we allocate the node, search for free space and then
> > insert the node into the mmap offset range manager.
> 
> Maybe I'm being a bit stupid here, but this seems like a pointless
> micro optimisation thrown in with a
> deinlining.

The actual bug fix is the removal of the allocation between searching
for a free node and inserting it into the tree:

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 24efae4..b885f2c 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -353,8 +353,8 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
 	map->handle = obj;
 
 	/* Get a DRM GEM mmap offset allocated... */
-	list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
-			obj->size / PAGE_SIZE, 0, false);
+	list->file_offset_node = kzalloc(sizeof(*list->file_offset_node),
+					 GFP_KERNEL);
 
 	if (!list->file_offset_node) {
 		DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
@@ -362,12 +362,11 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
 		goto out_free_list;
 	}
 
-	list->file_offset_node = drm_mm_get_block(list->file_offset_node,
-			obj->size / PAGE_SIZE, 0);
-	if (!list->file_offset_node) {
-		ret = -ENOMEM;
+	ret = drm_mm_insert_node(&mm->offset_manage,
+				 list->file_offset_node,
+				 obj->size / PAGE_SIZE, 0);
+	if (ret)
 		goto out_free_list;
-	}
 
 	list->hash.key = list->file_offset_node->start;
 	ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);

-- 
Chris Wilson, Intel Open Source Technology Centre