[PATCH 10/87] drm/i915: Purge loose pages if we run out of DMA remap space

[Tvrtko Ursulin]

On 05/01/2017 23:19, Chris Wilson wrote:
> If the DMA remap fails, one cause can be that we have too many objects
> pinned in a small remapping table, such as swiotlb. (DMA remapping does
> not trigger the shrinker by itself on its normal failure paths.)  So try
> purging all other objects (using i915_gem_shrink_all(), sparing our own
> pages as we have yet to assign them to the obj->pages) and try again. If
> there are no pages to reclaim (and consequently no pages to unmap), the
> shrinker will report 0 and we fail with -ENOSPC as before.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 00520f27bea6..f698006fe883 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2361,10 +2361,24 @@ void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv)
>  int i915_gem_gtt_prepare_pages(struct drm_i915_gem_object *obj,
>  			       struct sg_table *pages)
>  {
> -	if (dma_map_sg(&obj->base.dev->pdev->dev,
> -		       pages->sgl, pages->nents,
> -		       PCI_DMA_BIDIRECTIONAL))
> -		return 0;
> +	do {
> +		if (dma_map_sg(&obj->base.dev->pdev->dev,
> +			       pages->sgl, pages->nents,
> +			       PCI_DMA_BIDIRECTIONAL))
> +			return 0;
> +
> +		/* If the DMA remap fails, one cause can be that we have
> +		 * too many objects pinned in a small remapping table,
> +		 * such as swiotlb. Incrementally purge all other objects and
> +		 * try again - if there are no more pages to remove from
> +		 * the DMA remapper, i915_gem_shrink will return 0.
> +		 */
> +		GEM_BUG_ON(obj->mm.pages == pages);

How come you felt like putting this assert here? Especially in the loop, 
but can we even call this with non-null obj->mm.pages?

> +	} while (i915_gem_shrink(to_i915(obj->base.dev),
> +				 obj->base.size >> PAGE_SHIFT,
> +				 I915_SHRINK_BOUND |
> +				 I915_SHRINK_UNBOUND |
> +				 I915_SHRINK_ACTIVE));

I am slightly nervous about looping here, although in the current code I 
don't think there's a chance to keep trickling shrinks indefinitely. 
Once struct_mutex gets reduced it may become a concern though. Also, 
even currently 2nd+ passes would depend on the timing of request 
completion so maybe it would catch something past the 1st pass or maybe 
it wouldn't. I suppose on balance this should be a rare path so can stay 
and make the effort of trying to retire and shrink as much as it can.

>
>  	return -ENOSPC;
>  }
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko