Christian König
2022-Sep-01 07:49 UTC
[PATCH v4 00/21] Move all drivers to a common dma-buf locking convention
Hi Dmitry, I've gone over this multiple times now and while it is still possible that we missed something I think that this should land now. The whole topic is just to complicated that we can 100% sure guarantee that there isn't anything wrong with the locking, but lockdep and driver testing should allow us to quickly find remaining issues. Do you have commit rights to drm-misc-next or should I push it? Thanks, Christian. Am 31.08.22 um 17:37 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: > > v4: - Added dma_buf_mmap() to the "locking convention" documentation, > which was missed by accident in v3. > > - Added acks from Christian K?nig, Tomasz Figa and Hans Verkuil that > they gave to couple v3 patches. > > - Dropped the "_unlocked" postfix from function names that don't have > the locked variant, as was requested by Christian K?nig. > > - Factored out the per-driver preparations into separate patches > to ease reviewing of the changes, which is now doable without the > global dma-buf functions renaming. > > - Factored out the dynamic locking convention enforcements into separate > patches which add the final dma_resv_assert_held(dmabuf->resv) to the > dma-buf API functions. > > 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 (21): > dma-buf: Add unlocked variant of vmapping functions > dma-buf: Add unlocked variant of attachment-mapping functions > drm/gem: Take reservation lock for vmap/vunmap operations > drm/prime: Prepare to dynamic dma-buf locking specification > drm/armada: Prepare to dynamic dma-buf locking specification > drm/i915: Prepare to dynamic dma-buf locking specification > drm/omapdrm: Prepare to dynamic dma-buf locking specification > drm/tegra: Prepare to dynamic dma-buf locking specification > drm/etnaviv: Prepare to dynamic dma-buf locking specification > RDMA/umem: Prepare to dynamic dma-buf locking specification > misc: fastrpc: Prepare to dynamic dma-buf locking specification > xen/gntdev: Prepare to dynamic dma-buf locking specification > media: videobuf2: Prepare to dynamic dma-buf locking specification > media: tegra-vde: Prepare to dynamic dma-buf locking specification > dma-buf: Move dma_buf_vmap() to dynamic locking specification > dma-buf: Move dma_buf_attach() to dynamic locking specification > dma-buf: Move dma_buf_map_attachment() to dynamic locking > specification > dma-buf: Move dma_buf_mmap() to dynamic locking specification > dma-buf: Document dynamic locking convention > media: videobuf2: Stop using internal dma-buf lock > dma-buf: Remove obsoleted internal lock > > Documentation/driver-api/dma-buf.rst | 6 + > drivers/dma-buf/dma-buf.c | 211 +++++++++++++++--- > drivers/gpu/drm/armada/armada_gem.c | 8 +- > 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_ttm_helper.c | 9 +- > drivers/gpu/drm/drm_prime.c | 6 +- > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 + > .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 16 +- > drivers/gpu/drm/lima/lima_sched.c | 4 +- > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 4 +- > 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 | 17 +- > drivers/infiniband/core/umem_dmabuf.c | 7 +- > .../common/videobuf2/videobuf2-dma-contig.c | 22 +- > .../media/common/videobuf2/videobuf2-dma-sg.c | 19 +- > .../common/videobuf2/videobuf2-vmalloc.c | 17 +- > .../platform/nvidia/tegra-vde/dmabuf-cache.c | 6 +- > drivers/misc/fastrpc.c | 6 +- > drivers/xen/gntdev-dmabuf.c | 8 +- > include/drm/drm_gem.h | 3 + > include/linux/dma-buf.h | 17 +- > 29 files changed, 318 insertions(+), 155 deletions(-) >