Re: [PATCH v5 5/7] drm/i915: move locking in i915_gem_unmap_dma_buf()

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 29/02/16 11:13, Dave Gordon wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> The struct_mutex doesn't need to be (and therefore shouldn't be)
> held around the various non-i915 operations in this function. We
> only need the struct_mutex when we get to the unpin_pages() call.
>
> Extracted from Chris Wilson's patch:
>      drm/i915: Refactor duplicate object vmap functions
> in preparation for the reimplementation of the same.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_dmabuf.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index aee4149..68e21d1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -95,14 +95,12 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
>   {
>   	struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
>
> -	mutex_lock(&obj->base.dev->struct_mutex);
> -
>   	dma_unmap_sg(attachment->dev, sg->sgl, sg->nents, dir);
>   	sg_free_table(sg);
>   	kfree(sg);
>
> +	mutex_lock(&obj->base.dev->struct_mutex);
>   	i915_gem_object_unpin_pages(obj);
> -
>   	mutex_unlock(&obj->base.dev->struct_mutex);
>   }

Looks OK under my superficial understanding of dmabuf. But then 
i915_gem_map_dma_buf holds the mutex for too long as well. Anyway,

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

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx