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

[Jani Nikula <jani.nikula@linux.intel.com>]

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