| Message ID | 20220824102248.91964-1-dmitry.osipenko@collabora.com |
|---|---|
| Headers | show |
| Series | Move all drivers to a common dma-buf locking convention | expand |
Am 24.08.22 um 12:22 schrieb Dmitry Osipenko: > The internal dma-buf lock isn't needed anymore because the updated > locking specification claims that dma-buf reservation must be locked > by importers, and thus, the internal data is already protected by the > reservation lock. Remove the obsoleted internal lock. > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-buf.c | 14 ++++---------- > include/linux/dma-buf.h | 9 --------- > 2 files changed, 4 insertions(+), 19 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 696d132b02f4..a0406254f0ae 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -656,7 +656,6 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) > > dmabuf->file = file; > > - mutex_init(&dmabuf->lock); > INIT_LIST_HEAD(&dmabuf->attachments); > > mutex_lock(&db_list.lock); > @@ -1503,7 +1502,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_mmap_unlocked, DMA_BUF); > int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map) > { > struct iosys_map ptr; > - int ret = 0; > + int ret; > > iosys_map_clear(map); > > @@ -1515,28 +1514,25 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map) > if (!dmabuf->ops->vmap) > return -EINVAL; > > - mutex_lock(&dmabuf->lock); > if (dmabuf->vmapping_counter) { > dmabuf->vmapping_counter++; > BUG_ON(iosys_map_is_null(&dmabuf->vmap_ptr)); > *map = dmabuf->vmap_ptr; > - goto out_unlock; > + return 0; > } > > BUG_ON(iosys_map_is_set(&dmabuf->vmap_ptr)); > > ret = dmabuf->ops->vmap(dmabuf, &ptr); > if (WARN_ON_ONCE(ret)) > - goto out_unlock; > + return ret; > > dmabuf->vmap_ptr = ptr; > dmabuf->vmapping_counter = 1; > > *map = dmabuf->vmap_ptr; > > -out_unlock: > - mutex_unlock(&dmabuf->lock); > - return ret; > + return 0; > } > EXPORT_SYMBOL_NS_GPL(dma_buf_vmap, DMA_BUF); > > @@ -1578,13 +1574,11 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map) > BUG_ON(dmabuf->vmapping_counter == 0); > BUG_ON(!iosys_map_is_equal(&dmabuf->vmap_ptr, map)); > > - mutex_lock(&dmabuf->lock); > if (--dmabuf->vmapping_counter == 0) { > if (dmabuf->ops->vunmap) > dmabuf->ops->vunmap(dmabuf, map); > iosys_map_clear(&dmabuf->vmap_ptr); > } > - mutex_unlock(&dmabuf->lock); > } > EXPORT_SYMBOL_NS_GPL(dma_buf_vunmap, DMA_BUF); > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index d48d534dc55c..aed6695bbb50 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -326,15 +326,6 @@ struct dma_buf { > /** @ops: dma_buf_ops associated with this buffer object. */ > const struct dma_buf_ops *ops; > > - /** > - * @lock: > - * > - * Used internally to serialize list manipulation, attach/detach and > - * vmap/unmap. Note that in many cases this is superseeded by > - * dma_resv_lock() on @resv. > - */ > - struct mutex lock; > - > /** > * @vmapping_counter: > *
Reviewed-by: Christian König <christian.koenig@amd.com> to patches #2-#4 Am 24.08.22 um 12:22 schrieb Dmitry Osipenko: > Hello, > > This series moves all drivers to a dynamic dma-buf locking specification. > From now on all dma-buf importers are made responsible for holding > dma-buf's reservation lock around all operations performed over dma-bufs > in accordance to the locking specification. This allows us to utilize > reservation lock more broadly around kernel without fearing of a potential > deadlocks. > > This patchset passes all i915 selftests. It was also tested using VirtIO, > Panfrost, Lima, Tegra, udmabuf, AMDGPU and Nouveau drivers. I tested cases > of display+GPU, display+V4L and GPU+V4L dma-buf sharing (where appropriate), > which covers majority of kernel drivers since rest of the drivers share > same or similar code paths. > > Changelog: > > v3: - Factored out dma_buf_mmap_unlocked() and attachment functions > into aseparate patches, like was suggested by Christian König. > > - Corrected and factored out dma-buf locking documentation into > a separate patch, like was suggested by Christian König. > > - Intel driver dropped the reservation locking fews days ago from > its BO-release code path, but we need that locking for the imported > GEMs because in the end that code path unmaps the imported GEM. > So I added back the locking needed by the imported GEMs, updating > the "dma-buf attachment locking specification" patch appropriately. > > - Tested Nouveau+Intel dma-buf import/export combo. > > - Tested udmabuf import to i915/Nouveau/AMDGPU. > > - Fixed few places in Etnaviv, Panfrost and Lima drivers that I missed > to switch to locked dma-buf vmapping in the drm/gem: Take reservation > lock for vmap/vunmap operations" patch. In a result invalidated the > Christian's r-b that he gave to v2. > > - Added locked dma-buf vmap/vunmap functions that are needed for fixing > vmappping of Etnaviv, Panfrost and Lima drivers mentioned above. > I actually had this change stashed for the drm-shmem shrinker patchset, > but then realized that it's already needed by the dma-buf patches. > Also improved my tests to better cover these code paths. > > v2: - Changed locking specification to avoid problems with a cross-driver > ww locking, like was suggested by Christian König. Now the attach/detach > callbacks are invoked without the held lock and exporter should take the > lock. > > - Added "locking convention" documentation that explains which dma-buf > functions and callbacks are locked/unlocked for importers and exporters, > which was requested by Christian König. > > - Added ack from Tomasz Figa to the V4L patches that he gave to v1. > > Dmitry Osipenko (9): > dma-buf: Add _unlocked postfix to function names > dma-buf: Add locked variant of dma_buf_vmap/vunmap() > drm/gem: Take reservation lock for vmap/vunmap operations > dma-buf: Move dma_buf_vmap/vunmap_unlocked() to dynamic locking > specification > dma-buf: Move dma_buf_mmap_unlocked() to dynamic locking specification > dma-buf: Move dma-buf attachment to dynamic locking specification > dma-buf: Document dynamic locking convention > media: videobuf2: Stop using internal dma-buf lock > dma-buf: Remove internal lock > > Documentation/driver-api/dma-buf.rst | 6 + > drivers/dma-buf/dma-buf.c | 276 ++++++++++++++---- > drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +- > drivers/gpu/drm/armada/armada_gem.c | 14 +- > drivers/gpu/drm/drm_client.c | 4 +- > drivers/gpu/drm/drm_gem.c | 24 ++ > drivers/gpu/drm/drm_gem_dma_helper.c | 6 +- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > drivers/gpu/drm/drm_gem_ttm_helper.c | 9 +- > drivers/gpu/drm/drm_prime.c | 12 +- > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 +- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 6 +- > drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 + > .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 20 +- > drivers/gpu/drm/lima/lima_sched.c | 4 +- > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 8 +- > drivers/gpu/drm/panfrost/panfrost_dump.c | 4 +- > drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 6 +- > drivers/gpu/drm/qxl/qxl_object.c | 17 +- > drivers/gpu/drm/qxl/qxl_prime.c | 4 +- > drivers/gpu/drm/tegra/gem.c | 27 +- > drivers/infiniband/core/umem_dmabuf.c | 11 +- > .../common/videobuf2/videobuf2-dma-contig.c | 26 +- > .../media/common/videobuf2/videobuf2-dma-sg.c | 23 +- > .../common/videobuf2/videobuf2-vmalloc.c | 17 +- > .../platform/nvidia/tegra-vde/dmabuf-cache.c | 12 +- > drivers/misc/fastrpc.c | 12 +- > drivers/xen/gntdev-dmabuf.c | 14 +- > include/drm/drm_gem.h | 3 + > include/linux/dma-buf.h | 57 ++-- > 32 files changed, 410 insertions(+), 242 deletions(-) >