Re: [Intel-gfx] [PATCH 4/9] drm/i915: Limit the number of node allocation retries
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 b...@bwidawsk.net
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@lists.freedesktop.org
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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: Limit the number of node allocation retries
On Wed, May 07, 2014 at 09:49:57AM +0200, Daniel Vetter wrote:
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 b...@bwidawsk.net
So you replace the retry loop with a tailrecursive version in patch 2 and
then add duct-tape later on here? Nope.
-Daniel
I feel like you're implying the retry loop will end, ever. The retry
timeout should probably come first though, I Agree with that much.
---
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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
