[PATCH 1/3] drm: Export routines for inserting preallocated nodes into the mm manager

[PATCH 1/3] drm: Export routines for inserting preallocated nodes into the mm manager

[Chris Wilson]

Required by i915 in order to avoid the allocation in the middle of
manipulating the drm_mm lists.

Use a pair of stubs to preserve the existing EXPORT_SYMBOLs for
backporting; to be removed later.

Cc: Dave Airlie <airlied@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_mm.c |   42 ++++++++++++++++++++++++++++++------------
 include/drm/drm_mm.h     |   27 +++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 0761a03..d93dc05 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -184,19 +184,27 @@ EXPORT_SYMBOL(drm_mm_get_block_generic);
  * -ENOSPC if no suitable free area is available. The preallocated memory node
  * must be cleared.
  */
-int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
-		       unsigned long size, unsigned alignment)
+int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
+			       unsigned long size, unsigned alignment,
+			       unsigned long color, int atomic)
 {
 	struct drm_mm_node *hole_node;
 
-	hole_node = drm_mm_search_free(mm, size, alignment, false);
+	hole_node = drm_mm_search_free_generic(mm, size, alignment,
+					       color, atomic);
 	if (!hole_node)
 		return -ENOSPC;
 
-	drm_mm_insert_helper(hole_node, node, size, alignment, 0);
-
+	drm_mm_insert_helper(hole_node, node, size, alignment, color);
 	return 0;
 }
+EXPORT_SYMBOL(drm_mm_insert_node_generic);
+
+int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
+		       unsigned long size, unsigned alignment)
+{
+	return drm_mm_insert_node_generic(mm, node, size, alignment, 0, false);
+}
 EXPORT_SYMBOL(drm_mm_insert_node);
 
 static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node,
@@ -275,22 +283,32 @@ EXPORT_SYMBOL(drm_mm_get_block_range_generic);
  * -ENOSPC if no suitable free area is available. This is for range
  * restricted allocations. The preallocated memory node must be cleared.
  */
-int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
-				unsigned long size, unsigned alignment,
-				unsigned long start, unsigned long end)
+int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node,
+					unsigned long size, unsigned alignment, unsigned long color,
+					unsigned long start, unsigned long end,
+					int atomic)
 {
 	struct drm_mm_node *hole_node;
 
-	hole_node = drm_mm_search_free_in_range(mm, size, alignment,
-						start, end, false);
+	hole_node = drm_mm_search_free_in_range_generic(mm,
+							size, alignment, color,
+							start, end, atomic);
 	if (!hole_node)
 		return -ENOSPC;
 
-	drm_mm_insert_helper_range(hole_node, node, size, alignment, 0,
+	drm_mm_insert_helper_range(hole_node, node,
+				   size, alignment, color,
 				   start, end);
-
 	return 0;
 }
+EXPORT_SYMBOL(drm_mm_insert_node_in_range_generic);
+
+int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
+				unsigned long size, unsigned alignment,
+				unsigned long start, unsigned long end)
+{
+	return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end, false);
+}
 EXPORT_SYMBOL(drm_mm_insert_node_in_range);
 
 /**
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 06d7f79..87b0968 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -158,12 +158,31 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
 	return drm_mm_get_block_range_generic(parent, size, alignment, 0,
 						start, end, 1);
 }
-extern int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
-			      unsigned long size, unsigned alignment);
+
+extern int drm_mm_insert_node(struct drm_mm *mm,
+			      struct drm_mm_node *node,
+			      unsigned long size,
+			      unsigned alignment);
 extern int drm_mm_insert_node_in_range(struct drm_mm *mm,
 				       struct drm_mm_node *node,
-				       unsigned long size, unsigned alignment,
-				       unsigned long start, unsigned long end);
+				       unsigned long size,
+				       unsigned alignment,
+				       unsigned long start,
+				       unsigned long end);
+extern int drm_mm_insert_node_generic(struct drm_mm *mm,
+				      struct drm_mm_node *node,
+				      unsigned long size,
+				      unsigned alignment,
+				      unsigned long color,
+				      int atomic);
+extern int drm_mm_insert_node_in_range_generic(struct drm_mm *mm,
+				       struct drm_mm_node *node,
+				       unsigned long size,
+				       unsigned alignment,
+				       unsigned long color,
+				       unsigned long start,
+				       unsigned long end,
+				       int atomic);
 extern void drm_mm_put_block(struct drm_mm_node *cur);
 extern void drm_mm_remove_node(struct drm_mm_node *node);
 extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new);
-- 
1.7.10.4

[PATCH 2/3] drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT drm_mm manager

[Chris Wilson]

As we may reap neighbouring objects in order to free up pages for
allocations, we need to be careful not to allocate in the middle of the
drm_mm manager. To accomplish this, we can simply allocate the
drm_mm_node up front and then use the combined search & insert
drm_mm routines, reducing our code footprint in the process.

Fixes (partially) i-g-t/gem_tiled_swapping

Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   64 +++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c1f6919..d17f52d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2890,7 +2890,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 {
 	struct drm_device *dev = obj->base.dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_mm_node *free_space;
+	struct drm_mm_node *node;
 	u32 size, fence_size, fence_alignment, unfenced_alignment;
 	bool mappable, fenceable;
 	int ret;
@@ -2936,66 +2936,56 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 	i915_gem_object_pin_pages(obj);
 
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (node == NULL) {
+		i915_gem_object_unpin_pages(obj);
+		return -ENOMEM;
+	}
+
  search_free:
 	if (map_and_fenceable)
-		free_space = drm_mm_search_free_in_range_color(&dev_priv->mm.gtt_space,
-							       size, alignment, obj->cache_level,
-							       0, dev_priv->mm.gtt_mappable_end,
-							       false);
+		ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space, node,
+							  size, alignment, obj->cache_level,
+							  0, dev_priv->mm.gtt_mappable_end,
+							  false);
 	else
-		free_space = drm_mm_search_free_color(&dev_priv->mm.gtt_space,
-						      size, alignment, obj->cache_level,
-						      false);
-
-	if (free_space != NULL) {
-		if (map_and_fenceable)
-			free_space =
-				drm_mm_get_block_range_generic(free_space,
-							       size, alignment, obj->cache_level,
-							       0, dev_priv->mm.gtt_mappable_end,
-							       false);
-		else
-			free_space =
-				drm_mm_get_block_generic(free_space,
-							 size, alignment, obj->cache_level,
-							 false);
-	}
-	if (free_space == NULL) {
+		ret = drm_mm_insert_node_generic(&dev_priv->mm.gtt_space, node,
+						 size, alignment, obj->cache_level,
+						 false);
+	if (ret) {
 		ret = i915_gem_evict_something(dev, size, alignment,
 					       obj->cache_level,
 					       map_and_fenceable,
 					       nonblocking);
-		if (ret) {
-			i915_gem_object_unpin_pages(obj);
-			return ret;
-		}
+		if (ret == 0)
+			goto search_free;
 
-		goto search_free;
+		i915_gem_object_unpin_pages(obj);
+		kfree(node);
+		return ret;
 	}
-	if (WARN_ON(!i915_gem_valid_gtt_space(dev,
-					      free_space,
-					      obj->cache_level))) {
+	if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, obj->cache_level))) {
 		i915_gem_object_unpin_pages(obj);
-		drm_mm_put_block(free_space);
+		drm_mm_put_block(node);
 		return -EINVAL;
 	}
 
 	ret = i915_gem_gtt_prepare_object(obj);
 	if (ret) {
 		i915_gem_object_unpin_pages(obj);
-		drm_mm_put_block(free_space);
+		drm_mm_put_block(node);
 		return ret;
 	}
 
 	list_move_tail(&obj->gtt_list, &dev_priv->mm.bound_list);
 	list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
 
-	obj->gtt_space = free_space;
-	obj->gtt_offset = free_space->start;
+	obj->gtt_space = node;
+	obj->gtt_offset = node->start;
 
 	fenceable =
-		free_space->size == fence_size &&
-		(free_space->start & (fence_alignment - 1)) == 0;
+		node->size == fence_size &&
+		(node->start & (fence_alignment - 1)) == 0;
 
 	mappable =
 		obj->gtt_offset + obj->base.size <= dev_priv->mm.gtt_mappable_end;
-- 
1.7.10.4

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

[Chris Wilson]

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.

Fixes i-g-t/gem_tiled_swapping

Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   59 ++++++++++++++++++++++++++++++++-------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d17f52d..3ab97c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1512,14 +1512,29 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
 static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
+	struct drm_gem_mm *mm = obj->base.dev->mm_private;
+	struct drm_map_list *list;
 	int ret;
 
-	if (obj->base.map_list.map)
+	list = &obj->base.map_list;
+	if (list->map)
 		return 0;
 
-	ret = drm_gem_create_mmap_offset(&obj->base);
-	if (ret != -ENOSPC)
-		return ret;
+	/* Set the object up for mmap'ing */
+	list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
+	if (!list->map)
+		return -ENOMEM;
+
+	list->map->type = _DRM_GEM;
+	list->map->size = obj->base.size;
+	list->map->handle = &obj->base;
+
+	/* Get a DRM GEM mmap offset allocated... */
+	list->file_offset_node = kzalloc(sizeof(struct drm_mm_node), GFP_KERNEL);
+	if (list->file_offset_node == NULL) {
+		ret = -ENOMEM;
+		goto out_free_list;
+	}
 
 	/* Badly fragmented mmap space? The only way we can recover
 	 * space is by destroying unwanted objects. We can't randomly release
@@ -1528,13 +1543,37 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
 	 * offsets on purgeable objects by truncating it and marking it purged,
 	 * which prevents userspace from ever using that object again.
 	 */
-	i915_gem_purge(dev_priv, obj->base.size >> PAGE_SHIFT);
-	ret = drm_gem_create_mmap_offset(&obj->base);
-	if (ret != -ENOSPC)
-		return ret;
+	ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node,
+				 obj->base.size / PAGE_SIZE, 0);
+	if (ret) {
+		i915_gem_purge(dev_priv, obj->base.size >> PAGE_SHIFT);
+		ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node,
+					 obj->base.size / PAGE_SIZE, 0);
+	}
+	if (ret) {
+		i915_gem_shrink_all(dev_priv);
+		ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node,
+					 obj->base.size / PAGE_SIZE, 0);
+	}
+	if (ret) {
+		kfree(list->file_offset_node);
+		goto out_free_list;
+	}
+
+	list->hash.key = list->file_offset_node->start;
+	ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
+	if (ret)
+		goto out_free_mm;
 
-	i915_gem_shrink_all(dev_priv);
-	return drm_gem_create_mmap_offset(&obj->base);
+	return 0;
+
+out_free_mm:
+	drm_mm_put_block(list->file_offset_node);
+out_free_list:
+	kfree(list->map);
+	list->map = NULL;
+
+	return ret;
 }
 
 static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
-- 
1.7.10.4

Re: [PATCH 1/3] drm: Export routines for inserting preallocated nodes into the mm manager

[Jani Nikula]

On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Required by i915 in order to avoid the allocation in the middle of
> manipulating the drm_mm lists.
>
> Use a pair of stubs to preserve the existing EXPORT_SYMBOLs for
> backporting; to be removed later.

Regardless of whether you choose to do anything about the two nitpicks
below or not,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_mm.c |   42 ++++++++++++++++++++++++++++++------------
>  include/drm/drm_mm.h     |   27 +++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 0761a03..d93dc05 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -184,19 +184,27 @@ EXPORT_SYMBOL(drm_mm_get_block_generic);
>   * -ENOSPC if no suitable free area is available. The preallocated memory node
>   * must be cleared.
>   */
> -int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> -		       unsigned long size, unsigned alignment)
> +int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
> +			       unsigned long size, unsigned alignment,
> +			       unsigned long color, int atomic)
>  {
>  	struct drm_mm_node *hole_node;
>  
> -	hole_node = drm_mm_search_free(mm, size, alignment, false);
> +	hole_node = drm_mm_search_free_generic(mm, size, alignment,
> +					       color, atomic);

The last param for drm_mm_search_free_generic is a bool, any reason not
to make atomic a bool too?

>  	if (!hole_node)
>  		return -ENOSPC;
>  
> -	drm_mm_insert_helper(hole_node, node, size, alignment, 0);
> -
> +	drm_mm_insert_helper(hole_node, node, size, alignment, color);
>  	return 0;
>  }
> +EXPORT_SYMBOL(drm_mm_insert_node_generic);
> +
> +int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> +		       unsigned long size, unsigned alignment)
> +{
> +	return drm_mm_insert_node_generic(mm, node, size, alignment, 0, false);
> +}
>  EXPORT_SYMBOL(drm_mm_insert_node);
>  
>  static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node,
> @@ -275,22 +283,32 @@ EXPORT_SYMBOL(drm_mm_get_block_range_generic);
>   * -ENOSPC if no suitable free area is available. This is for range
>   * restricted allocations. The preallocated memory node must be cleared.
>   */
> -int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
> -				unsigned long size, unsigned alignment,
> -				unsigned long start, unsigned long end)
> +int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node,
> +					unsigned long size, unsigned alignment, unsigned long color,
> +					unsigned long start, unsigned long end,
> +					int atomic)
>  {
>  	struct drm_mm_node *hole_node;
>  
> -	hole_node = drm_mm_search_free_in_range(mm, size, alignment,
> -						start, end, false);
> +	hole_node = drm_mm_search_free_in_range_generic(mm,
> +							size, alignment, color,
> +							start, end, atomic);

Same as above.

>  	if (!hole_node)
>  		return -ENOSPC;
>  
> -	drm_mm_insert_helper_range(hole_node, node, size, alignment, 0,
> +	drm_mm_insert_helper_range(hole_node, node,
> +				   size, alignment, color,
>  				   start, end);
> -
>  	return 0;
>  }
> +EXPORT_SYMBOL(drm_mm_insert_node_in_range_generic);
> +
> +int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
> +				unsigned long size, unsigned alignment,
> +				unsigned long start, unsigned long end)
> +{
> +	return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end, false);
> +}
>  EXPORT_SYMBOL(drm_mm_insert_node_in_range);
>  
>  /**
> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
> index 06d7f79..87b0968 100644
> --- a/include/drm/drm_mm.h
> +++ b/include/drm/drm_mm.h
> @@ -158,12 +158,31 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
>  	return drm_mm_get_block_range_generic(parent, size, alignment, 0,
>  						start, end, 1);
>  }
> -extern int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> -			      unsigned long size, unsigned alignment);
> +
> +extern int drm_mm_insert_node(struct drm_mm *mm,
> +			      struct drm_mm_node *node,
> +			      unsigned long size,
> +			      unsigned alignment);
>  extern int drm_mm_insert_node_in_range(struct drm_mm *mm,
>  				       struct drm_mm_node *node,
> -				       unsigned long size, unsigned alignment,
> -				       unsigned long start, unsigned long end);
> +				       unsigned long size,
> +				       unsigned alignment,
> +				       unsigned long start,
> +				       unsigned long end);
> +extern int drm_mm_insert_node_generic(struct drm_mm *mm,
> +				      struct drm_mm_node *node,
> +				      unsigned long size,
> +				      unsigned alignment,
> +				      unsigned long color,
> +				      int atomic);
> +extern int drm_mm_insert_node_in_range_generic(struct drm_mm *mm,
> +				       struct drm_mm_node *node,
> +				       unsigned long size,
> +				       unsigned alignment,
> +				       unsigned long color,
> +				       unsigned long start,
> +				       unsigned long end,
> +				       int atomic);
>  extern void drm_mm_put_block(struct drm_mm_node *cur);
>  extern void drm_mm_remove_node(struct drm_mm_node *node);
>  extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new);
> -- 
> 1.7.10.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT drm_mm manager

[Jani Nikula]

On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As we may reap neighbouring objects in order to free up pages for
> allocations, we need to be careful not to allocate in the middle of the
> drm_mm manager. To accomplish this, we can simply allocate the
> drm_mm_node up front and then use the combined search & insert
> drm_mm routines, reducing our code footprint in the process.
>
> Fixes (partially) i-g-t/gem_tiled_swapping
>
> Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   64 +++++++++++++++++----------------------
>  1 file changed, 27 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c1f6919..d17f52d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2890,7 +2890,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
>  {
>  	struct drm_device *dev = obj->base.dev;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	struct drm_mm_node *free_space;
> +	struct drm_mm_node *node;
>  	u32 size, fence_size, fence_alignment, unfenced_alignment;
>  	bool mappable, fenceable;
>  	int ret;
> @@ -2936,66 +2936,56 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
>  
>  	i915_gem_object_pin_pages(obj);
>  
> +	node = kzalloc(sizeof(*node), GFP_KERNEL);
> +	if (node == NULL) {
> +		i915_gem_object_unpin_pages(obj);
> +		return -ENOMEM;
> +	}

Any reason not to do the kzalloc before i915_gem_object_pin_pages, with
a slight simplification of the error path there?

Otherwise, with the disclaimer that I'm a newbie in drm mm,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +
>   search_free:
>  	if (map_and_fenceable)
> -		free_space = drm_mm_search_free_in_range_color(&dev_priv->mm.gtt_space,
> -							       size, alignment, obj->cache_level,
> -							       0, dev_priv->mm.gtt_mappable_end,
> -							       false);
> +		ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space, node,
> +							  size, alignment, obj->cache_level,
> +							  0, dev_priv->mm.gtt_mappable_end,
> +							  false);
>  	else
> -		free_space = drm_mm_search_free_color(&dev_priv->mm.gtt_space,
> -						      size, alignment, obj->cache_level,
> -						      false);
> -
> -	if (free_space != NULL) {
> -		if (map_and_fenceable)
> -			free_space =
> -				drm_mm_get_block_range_generic(free_space,
> -							       size, alignment, obj->cache_level,
> -							       0, dev_priv->mm.gtt_mappable_end,
> -							       false);
> -		else
> -			free_space =
> -				drm_mm_get_block_generic(free_space,
> -							 size, alignment, obj->cache_level,
> -							 false);
> -	}
> -	if (free_space == NULL) {
> +		ret = drm_mm_insert_node_generic(&dev_priv->mm.gtt_space, node,
> +						 size, alignment, obj->cache_level,
> +						 false);
> +	if (ret) {
>  		ret = i915_gem_evict_something(dev, size, alignment,
>  					       obj->cache_level,
>  					       map_and_fenceable,
>  					       nonblocking);
> -		if (ret) {
> -			i915_gem_object_unpin_pages(obj);
> -			return ret;
> -		}
> +		if (ret == 0)
> +			goto search_free;
>  
> -		goto search_free;
> +		i915_gem_object_unpin_pages(obj);
> +		kfree(node);
> +		return ret;
>  	}
> -	if (WARN_ON(!i915_gem_valid_gtt_space(dev,
> -					      free_space,
> -					      obj->cache_level))) {
> +	if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, obj->cache_level))) {
>  		i915_gem_object_unpin_pages(obj);
> -		drm_mm_put_block(free_space);
> +		drm_mm_put_block(node);
>  		return -EINVAL;
>  	}
>  
>  	ret = i915_gem_gtt_prepare_object(obj);
>  	if (ret) {
>  		i915_gem_object_unpin_pages(obj);
> -		drm_mm_put_block(free_space);
> +		drm_mm_put_block(node);
>  		return ret;
>  	}
>  
>  	list_move_tail(&obj->gtt_list, &dev_priv->mm.bound_list);
>  	list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
>  
> -	obj->gtt_space = free_space;
> -	obj->gtt_offset = free_space->start;
> +	obj->gtt_space = node;
> +	obj->gtt_offset = node->start;
>  
>  	fenceable =
> -		free_space->size == fence_size &&
> -		(free_space->start & (fence_alignment - 1)) == 0;
> +		node->size == fence_size &&
> +		(node->start & (fence_alignment - 1)) == 0;
>  
>  	mappable =
>  		obj->gtt_offset + obj->base.size <= dev_priv->mm.gtt_mappable_end;
> -- 
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT drm_mm manager

[Chris Wilson]

On Wed, 12 Dec 2012 12:18:35 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > As we may reap neighbouring objects in order to free up pages for
> > allocations, we need to be careful not to allocate in the middle of the
> > drm_mm manager. To accomplish this, we can simply allocate the
> > drm_mm_node up front and then use the combined search & insert
> > drm_mm routines, reducing our code footprint in the process.
> >
> > Fixes (partially) i-g-t/gem_tiled_swapping
> >
> > Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c |   64 +++++++++++++++++----------------------
> >  1 file changed, 27 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index c1f6919..d17f52d 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2890,7 +2890,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
> >  {
> >  	struct drm_device *dev = obj->base.dev;
> >  	drm_i915_private_t *dev_priv = dev->dev_private;
> > -	struct drm_mm_node *free_space;
> > +	struct drm_mm_node *node;
> >  	u32 size, fence_size, fence_alignment, unfenced_alignment;
> >  	bool mappable, fenceable;
> >  	int ret;
> > @@ -2936,66 +2936,56 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
> >  
> >  	i915_gem_object_pin_pages(obj);
> >  
> > +	node = kzalloc(sizeof(*node), GFP_KERNEL);
> > +	if (node == NULL) {
> > +		i915_gem_object_unpin_pages(obj);
> > +		return -ENOMEM;
> > +	}
> 
> Any reason not to do the kzalloc before i915_gem_object_pin_pages, with
> a slight simplification of the error path there?

No reason at all. In my defense, I was trying to make the code as similar
as possible...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Jani Nikula]

On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> 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.
>
> Fixes i-g-t/gem_tiled_swapping
>
> Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   59 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d17f52d..3ab97c6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1512,14 +1512,29 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
>  static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
>  {
>  	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> +	struct drm_gem_mm *mm = obj->base.dev->mm_private;
> +	struct drm_map_list *list;
>  	int ret;
>  
> -	if (obj->base.map_list.map)
> +	list = &obj->base.map_list;
> +	if (list->map)
>  		return 0;
>  
> -	ret = drm_gem_create_mmap_offset(&obj->base);
> -	if (ret != -ENOSPC)
> -		return ret;
> +	/* Set the object up for mmap'ing */
> +	list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);

sizeof(struct drm_local_map) or sizeof(*list->map) instead?

Hmm, it's like this in drm_gem_create_mmap_offset too, either it's a bug
or I'm being clueless.

Other than that, with the same disclaimer as on the previous patch,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> +	if (!list->map)
> +		return -ENOMEM;
> +
> +	list->map->type = _DRM_GEM;
> +	list->map->size = obj->base.size;
> +	list->map->handle = &obj->base;
> +
> +	/* Get a DRM GEM mmap offset allocated... */
> +	list->file_offset_node = kzalloc(sizeof(struct drm_mm_node), GFP_KERNEL);
> +	if (list->file_offset_node == NULL) {
> +		ret = -ENOMEM;
> +		goto out_free_list;
> +	}
>  
>  	/* Badly fragmented mmap space? The only way we can recover
>  	 * space is by destroying unwanted objects. We can't randomly release
> @@ -1528,13 +1543,37 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
>  	 * offsets on purgeable objects by truncating it and marking it purged,
>  	 * which prevents userspace from ever using that object again.
>  	 */
> -	i915_gem_purge(dev_priv, obj->base.size >> PAGE_SHIFT);
> -	ret = drm_gem_create_mmap_offset(&obj->base);
> -	if (ret != -ENOSPC)
> -		return ret;
> +	ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node,
> +				 obj->base.size / PAGE_SIZE, 0);
> +	if (ret) {
> +		i915_gem_purge(dev_priv, obj->base.size >> PAGE_SHIFT);
> +		ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node,
> +					 obj->base.size / PAGE_SIZE, 0);
> +	}
> +	if (ret) {
> +		i915_gem_shrink_all(dev_priv);
> +		ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node,
> +					 obj->base.size / PAGE_SIZE, 0);
> +	}
> +	if (ret) {
> +		kfree(list->file_offset_node);
> +		goto out_free_list;
> +	}
> +
> +	list->hash.key = list->file_offset_node->start;
> +	ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
> +	if (ret)
> +		goto out_free_mm;
>  
> -	i915_gem_shrink_all(dev_priv);
> -	return drm_gem_create_mmap_offset(&obj->base);
> +	return 0;
> +
> +out_free_mm:
> +	drm_mm_put_block(list->file_offset_node);
> +out_free_list:
> +	kfree(list->map);
> +	list->map = NULL;
> +
> +	return ret;
>  }
>  
>  static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
> -- 
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Wed, 12 Dec 2012 12:48:43 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> 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.
> >
> > Fixes i-g-t/gem_tiled_swapping
> >
> > Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c |   59 ++++++++++++++++++++++++++++++++-------
> >  1 file changed, 49 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index d17f52d..3ab97c6 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1512,14 +1512,29 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
> >  static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
> >  {
> >  	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> > +	struct drm_gem_mm *mm = obj->base.dev->mm_private;
> > +	struct drm_map_list *list;
> >  	int ret;
> >  
> > -	if (obj->base.map_list.map)
> > +	list = &obj->base.map_list;
> > +	if (list->map)
> >  		return 0;
> >  
> > -	ret = drm_gem_create_mmap_offset(&obj->base);
> > -	if (ret != -ENOSPC)
> > -		return ret;
> > +	/* Set the object up for mmap'ing */
> > +	list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> 
> sizeof(struct drm_local_map) or sizeof(*list->map) instead?
> 
> Hmm, it's like this in drm_gem_create_mmap_offset too, either it's a bug
> or I'm being clueless.

You're right it's a a bug but fortunately in the safe direction, I'd
vote for sizeof(*list->map) as unequivocal.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/3] drm: Export routines for inserting preallocated nodes into the mm manager

[Daniel Vetter]

On Fri, Dec 07, 2012 at 08:37:06PM +0000, Chris Wilson wrote:
> Required by i915 in order to avoid the allocation in the middle of
> manipulating the drm_mm lists.
> 
> Use a pair of stubs to preserve the existing EXPORT_SYMBOLs for
> backporting; to be removed later.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_mm.c |   42 ++++++++++++++++++++++++++++++------------
>  include/drm/drm_mm.h     |   27 +++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 0761a03..d93dc05 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -184,19 +184,27 @@ EXPORT_SYMBOL(drm_mm_get_block_generic);
>   * -ENOSPC if no suitable free area is available. The preallocated memory node
>   * must be cleared.
>   */
> -int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> -		       unsigned long size, unsigned alignment)
> +int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
> +			       unsigned long size, unsigned alignment,
> +			       unsigned long color, int atomic)
>  {
>  	struct drm_mm_node *hole_node;
>  
> -	hole_node = drm_mm_search_free(mm, size, alignment, false);
> +	hole_node = drm_mm_search_free_generic(mm, size, alignment,
> +					       color, atomic);

The last parameter of search_free_generic is best_match, which isn't used
by any current caller. The only reason it's still there is that I haven't
converted yet all drm_mm.c users to preallocate drm_mm_node's, but as soon
as that's done best_match will die together with search_free_generic as a
public interface.

So what's the atomic doing in here? I've looked through the drm/i915
patches and couldn't see any reason ... Can we just respin the two i915
patches with _generic and atomic = false dropped, or do I miss something
big here?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/3] drm: Export routines for inserting preallocated nodes into the mm manager

[Chris Wilson]

On Tue, 18 Dec 2012 01:28:22 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> The last parameter of search_free_generic is best_match, which isn't used
> by any current caller. The only reason it's still there is that I haven't
> converted yet all drm_mm.c users to preallocate drm_mm_node's, but as soon
> as that's done best_match will die together with search_free_generic as a
> public interface.
> 
> So what's the atomic doing in here? I've looked through the drm/i915
> patches and couldn't see any reason ... Can we just respin the two i915
> patches with _generic and atomic = false dropped, or do I miss something
> big here?

It is scheduled to become a flags parameter for doing more funky
searches. (Allocating preferrentially from unmappable for LLC/snoopable
buffers etc) But the whole idea of the _generic functions is that they
do no currying at all, otherwise we will eventually need a _generic_full.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Daniel Vetter]

On Wed, Dec 12, 2012 at 10:55:17AM +0000, Chris Wilson wrote:
> On Wed, 12 Dec 2012 12:48:43 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> 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.
> > >
> > > Fixes i-g-t/gem_tiled_swapping
> > >
> > > Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c |   59 ++++++++++++++++++++++++++++++++-------
> > >  1 file changed, 49 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index d17f52d..3ab97c6 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -1512,14 +1512,29 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
> > >  static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
> > >  {
> > >  	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> > > +	struct drm_gem_mm *mm = obj->base.dev->mm_private;
> > > +	struct drm_map_list *list;
> > >  	int ret;
> > >  
> > > -	if (obj->base.map_list.map)
> > > +	list = &obj->base.map_list;
> > > +	if (list->map)
> > >  		return 0;
> > >  
> > > -	ret = drm_gem_create_mmap_offset(&obj->base);
> > > -	if (ret != -ENOSPC)
> > > -		return ret;
> > > +	/* Set the object up for mmap'ing */
> > > +	list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> > 
> > sizeof(struct drm_local_map) or sizeof(*list->map) instead?
> > 
> > Hmm, it's like this in drm_gem_create_mmap_offset too, either it's a bug
> > or I'm being clueless.
> 
> You're right it's a a bug but fortunately in the safe direction, I'd
> vote for sizeof(*list->map) as unequivocal.

All applied with bikeshed and pushed to -fixes, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Dave Airlie]

> 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.

Surely just calling drm_gem_create_mmap_offset a few times would work fine.

Like does it really fail that often, and is the extra overhead when it
does going to matter?

Dave.

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

[Daniel Vetter]

On Tue, Dec 18, 2012 at 10:11:14PM +0100, Daniel Vetter wrote:
> On Wed, Dec 12, 2012 at 10:55:17AM +0000, Chris Wilson wrote:
> > On Wed, 12 Dec 2012 12:48:43 +0200, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> 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.
> > > >
> > > > Fixes i-g-t/gem_tiled_swapping
> > > >
> > > > Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_gem.c |   59 ++++++++++++++++++++++++++++++++-------
> > > >  1 file changed, 49 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > index d17f52d..3ab97c6 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -1512,14 +1512,29 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
> > > >  static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
> > > >  {
> > > >  	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> > > > +	struct drm_gem_mm *mm = obj->base.dev->mm_private;
> > > > +	struct drm_map_list *list;
> > > >  	int ret;
> > > >  
> > > > -	if (obj->base.map_list.map)
> > > > +	list = &obj->base.map_list;
> > > > +	if (list->map)
> > > >  		return 0;
> > > >  
> > > > -	ret = drm_gem_create_mmap_offset(&obj->base);
> > > > -	if (ret != -ENOSPC)
> > > > -		return ret;
> > > > +	/* Set the object up for mmap'ing */
> > > > +	list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> > > 
> > > sizeof(struct drm_local_map) or sizeof(*list->map) instead?
> > > 
> > > Hmm, it's like this in drm_gem_create_mmap_offset too, either it's a bug
> > > or I'm being clueless.
> > 
> > You're right it's a a bug but fortunately in the safe direction, I'd
> > vote for sizeof(*list->map) as unequivocal.
> 
> All applied with bikeshed and pushed to -fixes, thanks.

Reconsidering just this patch, this smells too much like we leak our own
shrinker internals still. Can't we just set a dev_priv flag which tells
the shrinker not to attempt lock-stealing while we call down into the
drm mmap functions?

The issue is that people are talking about unifying the mmap lookup stuff
a bit since it's superflously duplicated between ttm and drm. So us
rolling our own special-case is a bit awkward. I've dropped the patch for
now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Chris Wilson]

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