diff mbox

drm/i915: Guard pages being reaped by OOM whilst binding-to-GTT

Message ID 1352451530-10767-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Nov. 9, 2012, 8:58 a.m. UTC
In the circumstances that the shrinker is allowed to steal the mutex
in order to reap pages, we need to be careful to prevent it operating on
the current object and shooting ourselves in the foot.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Nov. 20, 2012, 9:48 a.m. UTC | #1
On Fri, Nov 09, 2012 at 08:58:50AM +0000, Chris Wilson wrote:
> In the circumstances that the shrinker is allowed to steal the mutex
> in order to reap pages, we need to be careful to prevent it operating on
> the current object and shooting ourselves in the foot.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Ok, I this is definitely the major one that I've thought of, but I think
I've found another place that need a similar treatment:

i915_gem_fault: In between bind_to_gtt and vm_insert_pfn we have a call to
get_fence, which might potentially call kmalloc due to a pending request.
I know, it's highly unlikely that this will ever matter, but I think if we
braket this with a pin/unpin, it will nicely serve as a reminder where to
reserve/unreserve and object, in case we ever switch to a per-object
locking scheme.

Otherwise I can't poke holes into this any more ...

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 159dca5..f225583 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3091,6 +3091,8 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
>  	if (ret)
>  		return ret;
>  
> +	i915_gem_object_pin_pages(obj);
> +
>   search_free:
>  	if (map_and_fenceable)
>  		free_space =
> @@ -3121,14 +3123,17 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
>  					       obj->cache_level,
>  					       map_and_fenceable,
>  					       nonblocking);
> -		if (ret)
> +		if (ret) {
> +			i915_gem_object_unpin_pages(obj);
>  			return ret;
> +		}
>  
>  		goto search_free;
>  	}
>  	if (WARN_ON(!i915_gem_valid_gtt_space(dev,
>  					      obj->gtt_space,
>  					      obj->cache_level))) {
> +		i915_gem_object_unpin_pages(obj);
>  		drm_mm_put_block(obj->gtt_space);
>  		obj->gtt_space = NULL;
>  		return -EINVAL;
> @@ -3137,6 +3142,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
>  
>  	ret = i915_gem_gtt_prepare_object(obj);
>  	if (ret) {
> +		i915_gem_object_unpin_pages(obj);
>  		drm_mm_put_block(obj->gtt_space);
>  		obj->gtt_space = NULL;
>  		return ret;
> @@ -3159,6 +3165,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
>  
>  	obj->map_and_fenceable = mappable && fenceable;
>  
> +	i915_gem_object_unpin_pages(obj);
>  	trace_i915_gem_object_bind(obj, map_and_fenceable);
>  	i915_gem_verify_gtt(dev);
>  	return 0;
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 159dca5..f225583 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3091,6 +3091,8 @@  i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 	if (ret)
 		return ret;
 
+	i915_gem_object_pin_pages(obj);
+
  search_free:
 	if (map_and_fenceable)
 		free_space =
@@ -3121,14 +3123,17 @@  i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 					       obj->cache_level,
 					       map_and_fenceable,
 					       nonblocking);
-		if (ret)
+		if (ret) {
+			i915_gem_object_unpin_pages(obj);
 			return ret;
+		}
 
 		goto search_free;
 	}
 	if (WARN_ON(!i915_gem_valid_gtt_space(dev,
 					      obj->gtt_space,
 					      obj->cache_level))) {
+		i915_gem_object_unpin_pages(obj);
 		drm_mm_put_block(obj->gtt_space);
 		obj->gtt_space = NULL;
 		return -EINVAL;
@@ -3137,6 +3142,7 @@  i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 	ret = i915_gem_gtt_prepare_object(obj);
 	if (ret) {
+		i915_gem_object_unpin_pages(obj);
 		drm_mm_put_block(obj->gtt_space);
 		obj->gtt_space = NULL;
 		return ret;
@@ -3159,6 +3165,7 @@  i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 	obj->map_and_fenceable = mappable && fenceable;
 
+	i915_gem_object_unpin_pages(obj);
 	trace_i915_gem_object_bind(obj, map_and_fenceable);
 	i915_gem_verify_gtt(dev);
 	return 0;