This patchset makes dma-buf exporters responisble for taking care of
the reservation lock. I also included patch that moves drm-shmem to use
reservation lock, to let CI test the whole set. I'm going to take all
the patches via the drm-misc tree, please give an ack.
Previous policy stated that dma-buf core takes the lock around mmap()
callback. Which meant that both importers and exporters shouldn't touch
the reservation lock in the mmap() code path. This worked well until
Intel-CI found a deadlock problem in a case of self-imported dma-buf [1].
The problem happens when userpace mmaps a self-imported dma-buf, i.e.
mmaps the dma-buf FD. DRM core treats self-imported dma-bufs as own GEMs
[2]. There is no way to differentiate a prime GEM from a normal GEM for
drm-shmem in drm_gem_shmem_mmap(), which resulted in a deadlock problem
for drm-shmem mmap() code path once it's switched to use reservation lock.
It was difficult to fix the drm-shmem problem without adjusting dma-buf
locking policy. In parctice not much changed from importers perspective
because previosly dma-buf was taking the lock in between of importers
and exporters. Now this lock is shifted down to exporters.
[1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114671v2/shard-snb5/igt@prime_vgem@[email protected]
[2] https://elixir.bootlin.com/linux/v6.3-rc4/source/drivers/gpu/drm/drm_prime.c#L924
Changelog:
v4: - Added dma_resv_assert_held() to drm_gem_shmem_get_pages(), making
assert consistent with the put() variant. Suggested by Emil Velikov.
v3: - Added r-b from Hans Verkuil to the videobuf2 patch.
- The v2 fastrpc patch was already applied, not including it anymore.
Though, the cover-letter says that I'd want to apply all the patches
via the drm-misc tree to keep the proper ordering of the changes.
- Previously Intel's CI gave a flake failure to v2, want to re-test
it again.
v2: - Added ack from Christian König to the DRM patch.
- Dropped "fixes" tag from the patches, like was requested by
Christian König. The patches don't actually need a backport
and merely improve the locking policy.
- Dropped "reverts" from the patch titles to prevent them from
auto-backporting by the stable bot based on the title.
- Added r-b from Emil Velikov and placed the drm_WARN in the
drm-shmem patch like he suggested in a comment to v1.
- Corrected drm-shmem patch dma_resv_lock(obj->resv) inconsistently
used with dma_resv_unlock(shmem->base.resv). Now shmem->base.resv
variant is used for all locks/unlocks.
Dmitry Osipenko (6):
media: videobuf2: Don't assert held reservation lock for dma-buf
mmapping
dma-buf/heaps: Don't assert held reservation lock for dma-buf mmapping
udmabuf: Don't assert held reservation lock for dma-buf mmapping
drm: Don't assert held reservation lock for dma-buf mmapping
dma-buf: Change locking policy for mmap()
drm/shmem-helper: Switch to reservation lock
drivers/dma-buf/dma-buf.c | 17 +-
drivers/dma-buf/heaps/cma_heap.c | 3 -
drivers/dma-buf/heaps/system_heap.c | 3 -
drivers/dma-buf/udmabuf.c | 2 -
drivers/gpu/drm/drm_gem_shmem_helper.c | 210 ++++++++----------
drivers/gpu/drm/drm_prime.c | 2 -
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 -
drivers/gpu/drm/lima/lima_gem.c | 8 +-
drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 2 -
drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +-
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 6 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 19 +-
drivers/gpu/drm/tegra/gem.c | 2 -
.../common/videobuf2/videobuf2-dma-contig.c | 3 -
.../media/common/videobuf2/videobuf2-dma-sg.c | 3 -
.../common/videobuf2/videobuf2-vmalloc.c | 3 -
include/drm/drm_gem_shmem_helper.h | 14 +-
17 files changed, 119 insertions(+), 187 deletions(-)
--
2.40.1
Don't assert held dma-buf reservation lock on memory mapping of exported
buffer.
We're going to change dma-buf mmap() locking policy such that exporters
will have to handle the lock. The previous locking policy caused deadlock
problem for DRM drivers in a case of self-imported dma-bufs once these
drivers are moved to use reservation lock universally. The problem is
solved by moving the lock down to exporters. This patch prepares udmabuf
for the locking policy update.
Reviewed-by: Emil Velikov <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/dma-buf/udmabuf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 740d6e426ee9..277f1afa9552 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -52,8 +52,6 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma)
{
struct udmabuf *ubuf = buf->priv;
- dma_resv_assert_held(buf->resv);
-
if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0)
return -EINVAL;
--
2.40.1
On 5/30/23 01:39, Dmitry Osipenko wrote:
> This patchset makes dma-buf exporters responisble for taking care of
> the reservation lock. I also included patch that moves drm-shmem to use
> reservation lock, to let CI test the whole set. I'm going to take all
> the patches via the drm-misc tree, please give an ack.
>
> Previous policy stated that dma-buf core takes the lock around mmap()
> callback. Which meant that both importers and exporters shouldn't touch
> the reservation lock in the mmap() code path. This worked well until
> Intel-CI found a deadlock problem in a case of self-imported dma-buf [1].
>
> The problem happens when userpace mmaps a self-imported dma-buf, i.e.
> mmaps the dma-buf FD. DRM core treats self-imported dma-bufs as own GEMs
> [2]. There is no way to differentiate a prime GEM from a normal GEM for
> drm-shmem in drm_gem_shmem_mmap(), which resulted in a deadlock problem
> for drm-shmem mmap() code path once it's switched to use reservation lock.
>
> It was difficult to fix the drm-shmem problem without adjusting dma-buf
> locking policy. In parctice not much changed from importers perspective
> because previosly dma-buf was taking the lock in between of importers
> and exporters. Now this lock is shifted down to exporters.
>
> [1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114671v2/shard-snb5/igt@prime_vgem@[email protected]
> [2] https://elixir.bootlin.com/linux/v6.3-rc4/source/drivers/gpu/drm/drm_prime.c#L924
Applied to misc-next
--
Best regards,
Dmitry