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(-)
