Op 10-07-13 13:54, Daniel Vetter schreef:
> I've checked both implementations (radeon/nouveau) and they both grab
> the page array from ttm simply by dereferencing it and then wrapping
> it up with drm_prime_pages_to_sg in the callback and map it with
> dma_map_sg (in the helper).
>
> Only the grabbing of the underlying page array is anything we need to
> be concerned about, and either those pages are pinned independently,
> or we're screwed no matter what.
>
> And indeed, nouveau/radeon pin the backing storage in their
> attach/detach functions.
>
> The only thing we might claim it does is prevent concurrent mapping of
> dma_buf attachments. But a) that's not allowed and b) the current code
> is racy already since it checks whether the sg mapping exists _before_
> grabbing the lock.
>
> So the dev->struct_mutex locking here does absolutely nothing useful,
> but only distracts. Remove it.
>
> This should also help Maarten's work to eventually pin the backing
> storage more dynamically by preventing locking inversions around
> dev->struct_mutex.
This pleases me, but I think it's not thorough enough.
if (prime_attach->dir == dir)
return prime_attach->sgt;
^ That check must go too. I don't think recursive map_dma_buf is valid.
and unmap_dma_buf should set prime_attach->dir = DMA_NONE; again.
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_prime.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 85e450e..64a99b3 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -167,8 +167,6 @@ static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
> if (WARN_ON(prime_attach->dir != DMA_NONE))
> return ERR_PTR(-EBUSY);
>
> - mutex_lock(&obj->dev->struct_mutex);
> -
> sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
>
> if (!IS_ERR(sgt)) {
> @@ -182,7 +180,6 @@ static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
> }
> }
>
> - mutex_unlock(&obj->dev->struct_mutex);
> return sgt;
> }
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
