Skip to content

Commit

Permalink
drm/prime: remove cargo-cult locking from map_sg helper
Browse files Browse the repository at this point in the history
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.

Since I've created this patch cma prime support for dma_buf was added.
drm_gem_cma_prime_get_sg_table only calls kzalloc and the creates&maps
the sg table with dma_get_sgtable. It doesn't touch any gem object
state otherwise. So the cma helpers also look safe.

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.

v2: Add analysis for recently added cma helper prime code.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
  • Loading branch information
Daniel Vetter authored and Dave Airlie committed Aug 19, 2013
1 parent d0ed8d2 commit 01ce605
Showing 1 changed file with 0 additions and 3 deletions.
3 changes: 0 additions & 3 deletions drivers/gpu/drm/drm_prime.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
}

Expand Down

0 comments on commit 01ce605

Please sign in to comment.