On Tue, May 06, 2014 at 10:21:33PM -0700, Ben Widawsky wrote:
> AFAICT, it's impossible to actually infinitely retry the allocation in
> our current code. However, a small oversight on my part, slight bug, or
> future bug, could easily change that.
>
> To avoid a potential breakage, a simple retry variable of 16 bits will
> help us prevent infinitely running.
>
> Retry is limited to 100 as a mostly random number. Some consideration
> about stack usage was taken into account. As an example, if we allowed
> 256 retries on a 32b arch (and my memory serves that all arguments are
> passed on the stack for such architectures), thats 33 bytes * 256
> retries + (fudge for return address and such)... it's way more than we
> want to use already. 64b architectures might be slightly better, since
> 6? of the first args will get passed through registers, but it's still
> bad.
>
> If anything, we might want to do way less than 100, like 3.
>
> Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx>
So you replace the retry loop with a tailrecursive version in patch 2 and
then add duct-tape later on here? Nope.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b6965a2..de98b26 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3216,6 +3216,7 @@ static void i915_gem_verify_gtt(struct drm_device *dev)
> #endif
> }
>
> +#define MAX_VMA_FIND_RETRY 100
> static int
> i915_gem_find_vm_space(struct i915_address_space *vm,
> struct drm_mm_node *node,
> @@ -3224,7 +3225,8 @@ i915_gem_find_vm_space(struct i915_address_space *vm,
> unsigned long color,
> unsigned long start,
> unsigned long end,
> - uint32_t flags)
> + uint32_t flags,
> + uint8_t retry)
> {
> int ret;
> ret = drm_mm_insert_node_in_range_generic(&vm->mm, node,
> @@ -3232,7 +3234,7 @@ i915_gem_find_vm_space(struct i915_address_space *vm,
> start, end,
> DRM_MM_SEARCH_DEFAULT,
> DRM_MM_CREATE_DEFAULT);
> - if (ret) {
> + if (ret && (retry < MAX_VMA_FIND_RETRY)) {
> if (WARN_ON(ret != -ENOSPC))
> return ret;
>
> @@ -3241,7 +3243,8 @@ i915_gem_find_vm_space(struct i915_address_space *vm,
> if (ret == 0)
> return i915_gem_find_vm_space(vm, node,
> size, align, color,
> - start, end, flags);
> + start, end, flags,
> + retry++);
> }
>
> return ret;
> @@ -3306,8 +3309,9 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
> if (IS_ERR(vma))
> goto err_unpin;
>
> - ret = i915_gem_find_vm_space(vm, &vma->node, size, alignment,
> - obj->cache_level, 0, gtt_max, flags);
> + ret = i915_gem_find_vm_space(vm, &vma->node,
> + size, alignment, obj->cache_level,
> + 0, gtt_max, flags, 1);
> if (ret)
> goto err_free_vma;
>
> --
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
