[PATCH v6 00/21] Move all drivers to a common dma-buf locking convention

Dmitry Osipenko posted 21 patches 1 year, 6 months ago
There is a newer version of this series
Documentation/driver-api/dma-buf.rst          |   6 +
drivers/dma-buf/dma-buf.c                     | 214 +++++++++++++++---
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    |  14 ++
.../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, 323 insertions(+), 155 deletions(-)
[PATCH v6 00/21] Move all drivers to a common dma-buf locking convention
Posted by Dmitry Osipenko 1 year, 6 months ago
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:

v6: - Added r-b from Michael Ruhl to the i915 patch.

    - Added acks from Sumit Semwal and updated commit message of the
      "Move dma_buf_vmap() to dynamic locking specification" patch like
      was suggested by Sumit.

    - Added "!dmabuf" check to dma_buf_vmap_unlocked() to match the locked
      variant of the function, for consistency.

v5: - Added acks and r-bs that were given to v4.

    - Changed i915 preparation patch like was suggested by Michael Ruhl.
      The scope of reservation locking is smaller now.

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                     | 214 +++++++++++++++---
 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    |  14 ++
 .../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, 323 insertions(+), 155 deletions(-)

-- 
2.37.3

Re: [PATCH v6 00/21] Move all drivers to a common dma-buf locking convention
Posted by Dmitry Osipenko 1 year, 5 months ago
On 9/28/22 22:15, Dmitry Osipenko wrote:
> 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.

All the non-drm patches have been acked by the respective maintainers.
I'm now feeling comfortable to take this series into drm-misc-next and
going to do it later this week.

If anyone have more comments to add, then please do it now. It won't be
possible to drop out patches from drm-misc once they will be merged. All
further changes will have to be made on top of the applied patches.

Thanks to all who reviewed this patchset!

-- 
Best regards,
Dmitry