On Fri, 07 Dec 2012, Chris Wilson <ch...@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.kuopp...@linux.intel.com>
> Signed-off-by: Chris Wilson <ch...@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.nik...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to