On Fri, 07 Dec 2012, Chris Wilson <chris at 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 at linux.intel.com>
> Signed-off-by: Chris Wilson <chris at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
