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(-)
Add unlocked variant of dma_buf_vmap/vunmap() that will be utilized by drivers that don't take the reservation lock explicitly.
Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/dma-buf/dma-buf.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 2 ++ 2 files changed, 40 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 1c912255c5d6..5e4459bb1a6f 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1424,6 +1424,28 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map) } EXPORT_SYMBOL_NS_GPL(dma_buf_vmap, DMA_BUF);
+/** + * dma_buf_vmap_unlocked - Create virtual mapping for the buffer object into kernel + * address space. Same restrictions as for vmap and friends apply. + * @dmabuf: [in] buffer to vmap + * @map: [out] returns the vmap pointer + * + * Unlocked version of dma_buf_vmap() + * + * Returns 0 on success, or a negative errno code otherwise. + */ +int dma_buf_vmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map) +{ + int ret; + + dma_resv_lock(dmabuf->resv, NULL); + ret = dma_buf_vmap(dmabuf, map); + dma_resv_unlock(dmabuf->resv); + + return ret; +} +EXPORT_SYMBOL_NS_GPL(dma_buf_vmap_unlocked, DMA_BUF); + /** * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap. * @dmabuf: [in] buffer to vunmap @@ -1448,6 +1470,22 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map) } EXPORT_SYMBOL_NS_GPL(dma_buf_vunmap, DMA_BUF);
+/** + * dma_buf_vunmap_unlocked - Unmap a vmap obtained by dma_buf_vmap. + * @dmabuf: [in] buffer to vunmap + * @map: [in] vmap pointer to vunmap + */ +void dma_buf_vunmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map) +{ + if (WARN_ON(!dmabuf)) + return; + + dma_resv_lock(dmabuf->resv, NULL); + dma_buf_vunmap(dmabuf, map); + dma_resv_unlock(dmabuf->resv); +} +EXPORT_SYMBOL_NS_GPL(dma_buf_vunmap_unlocked, DMA_BUF); + #ifdef CONFIG_DEBUG_FS static int dma_buf_debug_show(struct seq_file *s, void *unused) { diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 71731796c8c3..8daa054dd7fe 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -632,4 +632,6 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long); int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map); void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map); +int dma_buf_vmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map); +void dma_buf_vunmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map); #endif /* __DMA_BUF_H__ */
Add unlocked variant of dma_buf_map/unmap_attachment() that will be used by drivers that don't take the reservation lock explicitly.
Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/dma-buf/dma-buf.c | 53 +++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 6 +++++ 2 files changed, 59 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 5e4459bb1a6f..51fb69048853 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1099,6 +1099,34 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, } EXPORT_SYMBOL_NS_GPL(dma_buf_map_attachment, DMA_BUF);
+/** + * dma_buf_map_attachment_unlocked - Returns the scatterlist table of the attachment; + * mapped into _device_ address space. Is a wrapper for map_dma_buf() of the + * dma_buf_ops. + * @attach: [in] attachment whose scatterlist is to be returned + * @direction: [in] direction of DMA transfer + * + * Unlocked variant of dma_buf_map_attachment(). + */ +struct sg_table * +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach, + enum dma_data_direction direction) +{ + struct sg_table *sg_table; + + might_sleep(); + + if (WARN_ON(!attach || !attach->dmabuf)) + return ERR_PTR(-EINVAL); + + dma_resv_lock(attach->dmabuf->resv, NULL); + sg_table = dma_buf_map_attachment(attach, direction); + dma_resv_unlock(attach->dmabuf->resv); + + return sg_table; +} +EXPORT_SYMBOL_NS_GPL(dma_buf_map_attachment_unlocked, DMA_BUF); + /** * dma_buf_unmap_attachment - unmaps and decreases usecount of the buffer;might * deallocate the scatterlist associated. Is a wrapper for unmap_dma_buf() of @@ -1135,6 +1163,31 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, } EXPORT_SYMBOL_NS_GPL(dma_buf_unmap_attachment, DMA_BUF);
+/** + * dma_buf_unmap_attachment_unlocked - unmaps and decreases usecount of the buffer;might + * deallocate the scatterlist associated. Is a wrapper for unmap_dma_buf() of + * dma_buf_ops. + * @attach: [in] attachment to unmap buffer from + * @sg_table: [in] scatterlist info of the buffer to unmap + * @direction: [in] direction of DMA transfer + * + * Unlocked variant of dma_buf_unmap_attachment(). + */ +void dma_buf_unmap_attachment_unlocked(struct dma_buf_attachment *attach, + struct sg_table *sg_table, + enum dma_data_direction direction) +{ + might_sleep(); + + if (WARN_ON(!attach || !attach->dmabuf || !sg_table)) + return; + + dma_resv_lock(attach->dmabuf->resv, NULL); + dma_buf_unmap_attachment(attach, sg_table, direction); + dma_resv_unlock(attach->dmabuf->resv); +} +EXPORT_SYMBOL_NS_GPL(dma_buf_unmap_attachment_unlocked, DMA_BUF); + /** * dma_buf_move_notify - notify attachments that DMA-buf is moving * diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 8daa054dd7fe..f11b5bbc2f37 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -627,6 +627,12 @@ int dma_buf_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction dir); int dma_buf_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction dir); +struct sg_table * +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach, + enum dma_data_direction direction); +void dma_buf_unmap_attachment_unlocked(struct dma_buf_attachment *attach, + struct sg_table *sg_table, + enum dma_data_direction direction);
int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long);
The new common dma-buf locking convention will require buffer importers to hold the reservation lock around mapping operations. Make DRM GEM core to take the lock around the vmapping operations and update DRM drivers to use the locked functions for the case where DRM core now holds the lock. This patch prepares DRM core and drivers to the common dynamic dma-buf locking convention.
Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- 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/lima/lima_sched.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 ++-- include/drm/drm_gem.h | 3 +++ 11 files changed, 54 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 2b230b4d6942..fbcb1e995384 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -323,7 +323,7 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer, * fd_install step out of the driver backend hooks, to make that * final step optional for internal users. */ - ret = drm_gem_vmap(buffer->gem, map); + ret = drm_gem_vmap_unlocked(buffer->gem, map); if (ret) return ret;
@@ -345,7 +345,7 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) { struct iosys_map *map = &buffer->map;
- drm_gem_vunmap(buffer->gem, map); + drm_gem_vunmap_unlocked(buffer->gem, map); } EXPORT_SYMBOL(drm_client_buffer_vunmap);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ad068865ba20..9c55593d662d 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1156,6 +1156,8 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) { int ret;
+ dma_resv_assert_held(obj->resv); + if (!obj->funcs->vmap) return -EOPNOTSUPP;
@@ -1171,6 +1173,8 @@ EXPORT_SYMBOL(drm_gem_vmap);
void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) { + dma_resv_assert_held(obj->resv); + if (iosys_map_is_null(map)) return;
@@ -1182,6 +1186,26 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) } EXPORT_SYMBOL(drm_gem_vunmap);
+int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) +{ + int ret; + + dma_resv_lock(obj->resv, NULL); + ret = drm_gem_vmap(obj, map); + dma_resv_unlock(obj->resv); + + return ret; +} +EXPORT_SYMBOL(drm_gem_vmap_unlocked); + +void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) +{ + dma_resv_lock(obj->resv, NULL); + drm_gem_vunmap(obj, map); + dma_resv_unlock(obj->resv); +} +EXPORT_SYMBOL(drm_gem_vunmap_unlocked); + /** * drm_gem_lock_reservations - Sets up the ww context and acquires * the lock on an array of GEM objects. diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c index f6901ff97bbb..1e658c448366 100644 --- a/drivers/gpu/drm/drm_gem_dma_helper.c +++ b/drivers/gpu/drm/drm_gem_dma_helper.c @@ -230,7 +230,7 @@ void drm_gem_dma_free(struct drm_gem_dma_object *dma_obj)
if (gem_obj->import_attach) { if (dma_obj->vaddr) - dma_buf_vunmap(gem_obj->import_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(gem_obj->import_attach->dmabuf, &map); drm_prime_gem_destroy(gem_obj, dma_obj->sgt); } else if (dma_obj->vaddr) { if (dma_obj->map_noncoherent) @@ -581,7 +581,7 @@ drm_gem_dma_prime_import_sg_table_vmap(struct drm_device *dev, struct iosys_map map; int ret;
- ret = dma_buf_vmap(attach->dmabuf, &map); + ret = dma_buf_vmap_unlocked(attach->dmabuf, &map); if (ret) { DRM_ERROR("Failed to vmap PRIME buffer\n"); return ERR_PTR(ret); @@ -589,7 +589,7 @@ drm_gem_dma_prime_import_sg_table_vmap(struct drm_device *dev,
obj = drm_gem_dma_prime_import_sg_table(dev, attach, sgt); if (IS_ERR(obj)) { - dma_buf_vunmap(attach->dmabuf, &map); + dma_buf_vunmap_unlocked(attach->dmabuf, &map); return obj; }
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 880a4975507f..e35e224e6303 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -354,7 +354,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, ret = -EINVAL; goto err_drm_gem_vunmap; } - ret = drm_gem_vmap(obj, &map[i]); + ret = drm_gem_vmap_unlocked(obj, &map[i]); if (ret) goto err_drm_gem_vunmap; } @@ -376,7 +376,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, obj = drm_gem_fb_get_obj(fb, i); if (!obj) continue; - drm_gem_vunmap(obj, &map[i]); + drm_gem_vunmap_unlocked(obj, &map[i]); } return ret; } @@ -403,7 +403,7 @@ void drm_gem_fb_vunmap(struct drm_framebuffer *fb, struct iosys_map *map) continue; if (iosys_map_is_null(&map[i])) continue; - drm_gem_vunmap(obj, &map[i]); + drm_gem_vunmap_unlocked(obj, &map[i]); } } EXPORT_SYMBOL(drm_gem_fb_vunmap); diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c index e5fc875990c4..d5962a34c01d 100644 --- a/drivers/gpu/drm/drm_gem_ttm_helper.c +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c @@ -64,13 +64,8 @@ int drm_gem_ttm_vmap(struct drm_gem_object *gem, struct iosys_map *map) { struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem); - int ret; - - dma_resv_lock(gem->resv, NULL); - ret = ttm_bo_vmap(bo, map); - dma_resv_unlock(gem->resv);
- return ret; + return ttm_bo_vmap(bo, map); } EXPORT_SYMBOL(drm_gem_ttm_vmap);
@@ -87,9 +82,7 @@ void drm_gem_ttm_vunmap(struct drm_gem_object *gem, { struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
- dma_resv_lock(gem->resv, NULL); ttm_bo_vunmap(bo, map); - dma_resv_unlock(gem->resv); } EXPORT_SYMBOL(drm_gem_ttm_vunmap);
diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index e82931712d8a..ff003403fbbc 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -371,7 +371,7 @@ static void lima_sched_build_error_task_list(struct lima_sched_task *task) } else { buffer_chunk->size = lima_bo_size(bo);
- ret = drm_gem_shmem_vmap(&bo->base, &map); + ret = drm_gem_vmap_unlocked(&bo->base.base, &map); if (ret) { kvfree(et); goto out; @@ -379,7 +379,7 @@ static void lima_sched_build_error_task_list(struct lima_sched_task *task)
memcpy(buffer_chunk + 1, map.vaddr, buffer_chunk->size);
- drm_gem_shmem_vunmap(&bo->base, &map); + drm_gem_vunmap_unlocked(&bo->base.base, &map); }
buffer_chunk = (void *)(buffer_chunk + 1) + buffer_chunk->size; diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c b/drivers/gpu/drm/panfrost/panfrost_dump.c index 89056a1aac7d..f62a019cc523 100644 --- a/drivers/gpu/drm/panfrost/panfrost_dump.c +++ b/drivers/gpu/drm/panfrost/panfrost_dump.c @@ -209,7 +209,7 @@ void panfrost_core_dump(struct panfrost_job *job) goto dump_header; }
- ret = drm_gem_shmem_vmap(&bo->base, &map); + ret = drm_gem_vmap_unlocked(&bo->base.base, &map); if (ret) { dev_err(pfdev->dev, "Panfrost Dump: couldn't map Buffer Object\n"); iter.hdr->bomap.valid = 0; @@ -236,7 +236,7 @@ void panfrost_core_dump(struct panfrost_job *job) vaddr = map.vaddr; memcpy(iter.data, vaddr, bo->base.base.size);
- drm_gem_shmem_vunmap(&bo->base, &map); + drm_gem_vunmap_unlocked(&bo->base.base, &map);
iter.hdr->bomap.valid = cpu_to_le32(1);
diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c index bc0df93f7f21..ba9b6e2b2636 100644 --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c @@ -106,7 +106,7 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, goto err_close_bo; }
- ret = drm_gem_shmem_vmap(bo, &map); + ret = drm_gem_vmap_unlocked(&bo->base, &map); if (ret) goto err_put_mapping; perfcnt->buf = map.vaddr; @@ -165,7 +165,7 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, return 0;
err_vunmap: - drm_gem_shmem_vunmap(bo, &map); + drm_gem_vunmap_unlocked(&bo->base, &map); err_put_mapping: panfrost_gem_mapping_put(perfcnt->mapping); err_close_bo: @@ -195,7 +195,7 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev, GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
perfcnt->user = NULL; - drm_gem_shmem_vunmap(&perfcnt->mapping->obj->base, &map); + drm_gem_vunmap_unlocked(&perfcnt->mapping->obj->base.base, &map); perfcnt->buf = NULL; panfrost_gem_close(&perfcnt->mapping->obj->base.base, file_priv); panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu); diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index 695d9308d1f0..06a58dad5f5c 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -168,9 +168,16 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct iosys_map *map) bo->map_count++; goto out; } - r = ttm_bo_vmap(&bo->tbo, &bo->map); + + r = __qxl_bo_pin(bo); if (r) return r; + + r = ttm_bo_vmap(&bo->tbo, &bo->map); + if (r) { + __qxl_bo_unpin(bo); + return r; + } bo->map_count = 1;
/* TODO: Remove kptr in favor of map everywhere. */ @@ -192,12 +199,6 @@ int qxl_bo_vmap(struct qxl_bo *bo, struct iosys_map *map) if (r) return r;
- r = __qxl_bo_pin(bo); - if (r) { - qxl_bo_unreserve(bo); - return r; - } - r = qxl_bo_vmap_locked(bo, map); qxl_bo_unreserve(bo); return r; @@ -247,6 +248,7 @@ void qxl_bo_vunmap_locked(struct qxl_bo *bo) return; bo->kptr = NULL; ttm_bo_vunmap(&bo->tbo, &bo->map); + __qxl_bo_unpin(bo); }
int qxl_bo_vunmap(struct qxl_bo *bo) @@ -258,7 +260,6 @@ int qxl_bo_vunmap(struct qxl_bo *bo) return r;
qxl_bo_vunmap_locked(bo); - __qxl_bo_unpin(bo); qxl_bo_unreserve(bo); return 0; } diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c index 142d01415acb..9169c26357d3 100644 --- a/drivers/gpu/drm/qxl/qxl_prime.c +++ b/drivers/gpu/drm/qxl/qxl_prime.c @@ -59,7 +59,7 @@ int qxl_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map) struct qxl_bo *bo = gem_to_qxl_bo(obj); int ret;
- ret = qxl_bo_vmap(bo, map); + ret = qxl_bo_vmap_locked(bo, map); if (ret < 0) return ret;
@@ -71,5 +71,5 @@ void qxl_gem_prime_vunmap(struct drm_gem_object *obj, { struct qxl_bo *bo = gem_to_qxl_bo(obj);
- qxl_bo_vunmap(bo); + qxl_bo_vunmap_locked(bo); } diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 58a18a17c67e..30096f9efdbf 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -420,4 +420,7 @@ void drm_gem_unlock_reservations(struct drm_gem_object **objs, int count, int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, u32 handle, u64 *offset);
+int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map); +void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map); + #endif /* __DRM_GEM_H__ */
Prepare DRM prime core to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/drm_prime.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index eb09e86044c6..20e109a802ae 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -940,7 +940,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
get_dma_buf(dma_buf);
- sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); + sgt = dma_buf_map_attachment_unlocked(attach, DMA_BIDIRECTIONAL); if (IS_ERR(sgt)) { ret = PTR_ERR(sgt); goto fail_detach; @@ -958,7 +958,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, return obj;
fail_unmap: - dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_BIDIRECTIONAL); fail_detach: dma_buf_detach(dma_buf, attach); dma_buf_put(dma_buf); @@ -1056,7 +1056,7 @@ void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg)
attach = obj->import_attach; if (sg) - dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(attach, sg, DMA_BIDIRECTIONAL); dma_buf = attach->dmabuf; dma_buf_detach(attach->dmabuf, attach); /* remove the reference */
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare DRM prime core to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Reviewed-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/drm_prime.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index eb09e86044c6..20e109a802ae 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -940,7 +940,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, get_dma_buf(dma_buf);
- sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
- sgt = dma_buf_map_attachment_unlocked(attach, DMA_BIDIRECTIONAL); if (IS_ERR(sgt)) { ret = PTR_ERR(sgt); goto fail_detach;
@@ -958,7 +958,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, return obj; fail_unmap:
- dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
- dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_BIDIRECTIONAL); fail_detach: dma_buf_detach(dma_buf, attach); dma_buf_put(dma_buf);
@@ -1056,7 +1056,7 @@ void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) attach = obj->import_attach; if (sg)
dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
dma_buf_unmap_attachment_unlocked(attach, sg, DMA_BIDIRECTIONAL);
Prepare Armada driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/armada/armada_gem.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index 5430265ad458..26d10065d534 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -66,8 +66,8 @@ void armada_gem_free_object(struct drm_gem_object *obj) if (dobj->obj.import_attach) { /* We only ever display imported data */ if (dobj->sgt) - dma_buf_unmap_attachment(dobj->obj.import_attach, - dobj->sgt, DMA_TO_DEVICE); + dma_buf_unmap_attachment_unlocked(dobj->obj.import_attach, + dobj->sgt, DMA_TO_DEVICE); drm_prime_gem_destroy(&dobj->obj, NULL); }
@@ -539,8 +539,8 @@ int armada_gem_map_import(struct armada_gem_object *dobj) { int ret;
- dobj->sgt = dma_buf_map_attachment(dobj->obj.import_attach, - DMA_TO_DEVICE); + dobj->sgt = dma_buf_map_attachment_unlocked(dobj->obj.import_attach, + DMA_TO_DEVICE); if (IS_ERR(dobj->sgt)) { ret = PTR_ERR(dobj->sgt); dobj->sgt = NULL;
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare Armada driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/armada/armada_gem.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index 5430265ad458..26d10065d534 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -66,8 +66,8 @@ void armada_gem_free_object(struct drm_gem_object *obj) if (dobj->obj.import_attach) { /* We only ever display imported data */ if (dobj->sgt)
dma_buf_unmap_attachment(dobj->obj.import_attach,dobj->sgt, DMA_TO_DEVICE);
dma_buf_unmap_attachment_unlocked(dobj->obj.import_attach,dobj->sgt, DMA_TO_DEVICE);@@ -539,8 +539,8 @@ int armada_gem_map_import(struct armada_gem_object *dobj) { int ret;
- dobj->sgt = dma_buf_map_attachment(dobj->obj.import_attach,
DMA_TO_DEVICE);
- dobj->sgt = dma_buf_map_attachment_unlocked(dobj->obj.import_attach,
DMA_TO_DEVICE);
Prepare i915 driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions and handling cases where importer now holds the reservation lock.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 ++++++++++++ .../gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 16 ++++++++-------- 3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index f5062d0c6333..07eee1c09aaf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -72,7 +72,7 @@ static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); void *vaddr;
- vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); + vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); if (IS_ERR(vaddr)) return PTR_ERR(vaddr);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 389e9f157ca5..7e2a9b02526c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
+ /* + * dma_buf_unmap_attachment() requires reservation to be + * locked. The imported GEM shouldn't share reservation lock, + * so it's safe to take the lock. + */ + if (obj->base.import_attach) + i915_gem_object_lock(obj, NULL); + __i915_gem_object_pages_fini(obj); + + if (obj->base.import_attach) + i915_gem_object_unlock(obj); + __i915_gem_free_object(obj);
/* But keep the pointer alive for RCU-protected lookups */ diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 62c61af77a42..9e3ed634aa0e 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -213,7 +213,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, goto out_import; }
- st = dma_buf_map_attachment(import_attach, DMA_BIDIRECTIONAL); + st = dma_buf_map_attachment_unlocked(import_attach, DMA_BIDIRECTIONAL); if (IS_ERR(st)) { err = PTR_ERR(st); goto out_detach; @@ -226,7 +226,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, timeout = -ETIME; } err = timeout > 0 ? 0 : timeout; - dma_buf_unmap_attachment(import_attach, st, DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(import_attach, st, DMA_BIDIRECTIONAL); out_detach: dma_buf_detach(dmabuf, import_attach); out_import: @@ -296,7 +296,7 @@ static int igt_dmabuf_import(void *arg) goto out_obj; }
- err = dma_buf_vmap(dmabuf, &map); + err = dma_buf_vmap_unlocked(dmabuf, &map); dma_map = err ? NULL : map.vaddr; if (!dma_map) { pr_err("dma_buf_vmap failed\n"); @@ -337,7 +337,7 @@ static int igt_dmabuf_import(void *arg)
err = 0; out_dma_map: - dma_buf_vunmap(dmabuf, &map); + dma_buf_vunmap_unlocked(dmabuf, &map); out_obj: i915_gem_object_put(obj); out_dmabuf: @@ -358,7 +358,7 @@ static int igt_dmabuf_import_ownership(void *arg) if (IS_ERR(dmabuf)) return PTR_ERR(dmabuf);
- err = dma_buf_vmap(dmabuf, &map); + err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n"); @@ -367,7 +367,7 @@ static int igt_dmabuf_import_ownership(void *arg) }
memset(ptr, 0xc5, PAGE_SIZE); - dma_buf_vunmap(dmabuf, &map); + dma_buf_vunmap_unlocked(dmabuf, &map);
obj = to_intel_bo(i915_gem_prime_import(&i915->drm, dmabuf)); if (IS_ERR(obj)) { @@ -418,7 +418,7 @@ static int igt_dmabuf_export_vmap(void *arg) } i915_gem_object_put(obj);
- err = dma_buf_vmap(dmabuf, &map); + err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n"); @@ -435,7 +435,7 @@ static int igt_dmabuf_export_vmap(void *arg) memset(ptr, 0xc5, dmabuf->size);
err = 0; - dma_buf_vunmap(dmabuf, &map); + dma_buf_vunmap_unlocked(dmabuf, &map); out: dma_buf_put(dmabuf); return err;
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare i915 driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions and handling cases where importer now holds the reservation lock.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com, but it's probably best if somebody from the Intel guys take a look as well.
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 ++++++++++++ .../gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 16 ++++++++-------- 3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index f5062d0c6333..07eee1c09aaf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -72,7 +72,7 @@ static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); void *vaddr;
- vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); if (IS_ERR(vaddr)) return PTR_ERR(vaddr);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 389e9f157ca5..7e2a9b02526c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
/** dma_buf_unmap_attachment() requires reservation to be* locked. The imported GEM shouldn't share reservation lock,* so it's safe to take the lock.*/if (obj->base.import_attach)i915_gem_object_lock(obj, NULL);- __i915_gem_object_pages_fini(obj);
if (obj->base.import_attach)i915_gem_object_unlock(obj);- __i915_gem_free_object(obj);
/* But keep the pointer alive for RCU-protected lookups */ diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 62c61af77a42..9e3ed634aa0e 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -213,7 +213,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, goto out_import; }
- st = dma_buf_map_attachment(import_attach, DMA_BIDIRECTIONAL);
- st = dma_buf_map_attachment_unlocked(import_attach, DMA_BIDIRECTIONAL); if (IS_ERR(st)) { err = PTR_ERR(st); goto out_detach;
@@ -226,7 +226,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, timeout = -ETIME; } err = timeout > 0 ? 0 : timeout;
- dma_buf_unmap_attachment(import_attach, st, DMA_BIDIRECTIONAL);
- dma_buf_unmap_attachment_unlocked(import_attach, st, DMA_BIDIRECTIONAL); out_detach: dma_buf_detach(dmabuf, import_attach); out_import:
@@ -296,7 +296,7 @@ static int igt_dmabuf_import(void *arg) goto out_obj; }
- err = dma_buf_vmap(dmabuf, &map);
- err = dma_buf_vmap_unlocked(dmabuf, &map); dma_map = err ? NULL : map.vaddr; if (!dma_map) { pr_err("dma_buf_vmap failed\n");
@@ -337,7 +337,7 @@ static int igt_dmabuf_import(void *arg) err = 0; out_dma_map:
- dma_buf_vunmap(dmabuf, &map);
- dma_buf_vunmap_unlocked(dmabuf, &map); out_obj: i915_gem_object_put(obj); out_dmabuf:
@@ -358,7 +358,7 @@ static int igt_dmabuf_import_ownership(void *arg) if (IS_ERR(dmabuf)) return PTR_ERR(dmabuf);
- err = dma_buf_vmap(dmabuf, &map);
- err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n");
@@ -367,7 +367,7 @@ static int igt_dmabuf_import_ownership(void *arg) } memset(ptr, 0xc5, PAGE_SIZE);
- dma_buf_vunmap(dmabuf, &map);
- dma_buf_vunmap_unlocked(dmabuf, &map);
obj = to_intel_bo(i915_gem_prime_import(&i915->drm, dmabuf)); if (IS_ERR(obj)) { @@ -418,7 +418,7 @@ static int igt_dmabuf_export_vmap(void *arg) } i915_gem_object_put(obj);
- err = dma_buf_vmap(dmabuf, &map);
- err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n");
@@ -435,7 +435,7 @@ static int igt_dmabuf_export_vmap(void *arg) memset(ptr, 0xc5, dmabuf->size); err = 0;
- dma_buf_vunmap(dmabuf, &map);
- dma_buf_vunmap_unlocked(dmabuf, &map); out: dma_buf_put(dmabuf); return err;
On 9/1/22 09:45, Christian König wrote:
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare i915 driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions and handling cases where importer now holds the reservation lock.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com, but it's probably best if somebody from the Intel guys take a look as well.
+ Chris Wilson, who touched locks of __i915_gem_free_objects() recently
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 ++++++++++++ .../gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 16 ++++++++-------- 3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index f5062d0c6333..07eee1c09aaf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -72,7 +72,7 @@ static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); void *vaddr; - vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); + vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); if (IS_ERR(vaddr)) return PTR_ERR(vaddr); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 389e9f157ca5..7e2a9b02526c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; } + /* + * dma_buf_unmap_attachment() requires reservation to be + * locked. The imported GEM shouldn't share reservation lock, + * so it's safe to take the lock. + */ + if (obj->base.import_attach) + i915_gem_object_lock(obj, NULL);
__i915_gem_object_pages_fini(obj);
+ if (obj->base.import_attach) + i915_gem_object_unlock(obj);
__i915_gem_free_object(obj); /* But keep the pointer alive for RCU-protected lookups */ diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 62c61af77a42..9e3ed634aa0e 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -213,7 +213,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, goto out_import; } - st = dma_buf_map_attachment(import_attach, DMA_BIDIRECTIONAL); + st = dma_buf_map_attachment_unlocked(import_attach, DMA_BIDIRECTIONAL); if (IS_ERR(st)) { err = PTR_ERR(st); goto out_detach; @@ -226,7 +226,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, timeout = -ETIME; } err = timeout > 0 ? 0 : timeout; - dma_buf_unmap_attachment(import_attach, st, DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(import_attach, st, DMA_BIDIRECTIONAL); out_detach: dma_buf_detach(dmabuf, import_attach); out_import: @@ -296,7 +296,7 @@ static int igt_dmabuf_import(void *arg) goto out_obj; } - err = dma_buf_vmap(dmabuf, &map); + err = dma_buf_vmap_unlocked(dmabuf, &map); dma_map = err ? NULL : map.vaddr; if (!dma_map) { pr_err("dma_buf_vmap failed\n"); @@ -337,7 +337,7 @@ static int igt_dmabuf_import(void *arg) err = 0; out_dma_map: - dma_buf_vunmap(dmabuf, &map); + dma_buf_vunmap_unlocked(dmabuf, &map); out_obj: i915_gem_object_put(obj); out_dmabuf: @@ -358,7 +358,7 @@ static int igt_dmabuf_import_ownership(void *arg) if (IS_ERR(dmabuf)) return PTR_ERR(dmabuf); - err = dma_buf_vmap(dmabuf, &map); + err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n"); @@ -367,7 +367,7 @@ static int igt_dmabuf_import_ownership(void *arg) } memset(ptr, 0xc5, PAGE_SIZE); - dma_buf_vunmap(dmabuf, &map); + dma_buf_vunmap_unlocked(dmabuf, &map); obj = to_intel_bo(i915_gem_prime_import(&i915->drm, dmabuf)); if (IS_ERR(obj)) { @@ -418,7 +418,7 @@ static int igt_dmabuf_export_vmap(void *arg) } i915_gem_object_put(obj); - err = dma_buf_vmap(dmabuf, &map); + err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n"); @@ -435,7 +435,7 @@ static int igt_dmabuf_export_vmap(void *arg) memset(ptr, 0xc5, dmabuf->size); err = 0; - dma_buf_vunmap(dmabuf, &map); + dma_buf_vunmap_unlocked(dmabuf, &map); out: dma_buf_put(dmabuf); return err;
-----Original Message----- From: Dmitry Osipenko dmitry.osipenko@collabora.com Sent: Wednesday, August 31, 2022 11:38 AM To: David Airlie airlied@linux.ie; Gerd Hoffmann kraxel@redhat.com; Gurchetan Singh gurchetansingh@chromium.org; Chia-I Wu olvaffe@gmail.com; Daniel Vetter daniel@ffwll.ch; Daniel Almeida daniel.almeida@collabora.com; Gert Wollny gert.wollny@collabora.com; Gustavo Padovan gustavo.padovan@collabora.com; Daniel Stone daniel@fooishbar.org; Tomeu Vizoso tomeu.vizoso@collabora.com; Maarten Lankhorst maarten.lankhorst@linux.intel.com; Maxime Ripard mripard@kernel.org; Thomas Zimmermann tzimmermann@suse.de; Rob Clark robdclark@gmail.com; Sumit Semwal sumit.semwal@linaro.org; Christian König christian.koenig@amd.com; Pan, Xinhui Xinhui.Pan@amd.com; Thierry Reding thierry.reding@gmail.com; Tomasz Figa tfiga@chromium.org; Marek Szyprowski m.szyprowski@samsung.com; Mauro Carvalho Chehab mchehab@kernel.org; Alex Deucher alexander.deucher@amd.com; Jani Nikula jani.nikula@linux.intel.com; Joonas Lahtinen joonas.lahtinen@linux.intel.com; Vivi, Rodrigo rodrigo.vivi@intel.com; Tvrtko Ursulin tvrtko.ursulin@linux.intel.com; Thomas Hellström thomas_os@shipmail.org; Qiang Yu yuq825@gmail.com; Srinivas Kandagatla srinivas.kandagatla@linaro.org; Amol Maheshwari amahesh@qti.qualcomm.com; Jason Gunthorpe jgg@ziepe.ca; Leon Romanovsky leon@kernel.org; Gross, Jurgen jgross@suse.com; Stefano Stabellini sstabellini@kernel.org; Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com; Tomi Valkeinen tomba@kernel.org; Russell King linux@armlinux.org.uk; Lucas Stach l.stach@pengutronix.de; Christian Gmeiner christian.gmeiner@gmail.com Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; Dmitry Osipenko digetx@gmail.com; linux-media@vger.kernel.org; linaro-mm- sig@lists.linaro.org; amd-gfx@lists.freedesktop.org; intel- gfx@lists.freedesktop.org; kernel@collabora.com; virtualization@lists.linux- foundation.org; linux-rdma@vger.kernel.org; linux-arm- msm@vger.kernel.org Subject: [PATCH v4 06/21] drm/i915: Prepare to dynamic dma-buf locking specification
Prepare i915 driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions and handling cases where importer now holds the reservation lock.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 ++++++++++++ .../gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 16 ++++++++-------- 3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index f5062d0c6333..07eee1c09aaf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -72,7 +72,7 @@ static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); void *vaddr;
- vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); if (IS_ERR(vaddr)) return PTR_ERR(vaddr);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 389e9f157ca5..7e2a9b02526c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
/** dma_buf_unmap_attachment() requires reservation to be* locked. The imported GEM shouldn't share reservation lock,* so it's safe to take the lock.*/if (obj->base.import_attach)i915_gem_object_lock(obj, NULL);
There is a lot of stuff going here. Taking the lock may be premature...
__i915_gem_object_pages_fini(obj);
The i915_gem_dmabuf.c:i915_gem_object_put_pages_dmabuf is where unmap_attachment is actually called, would it make more sense to make do the locking there?
Mike
if (obj->base.import_attach)i915_gem_object_unlock(obj);__i915_gem_free_object(obj);
/* But keep the pointer alive for RCU-protected lookups */
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c index 62c61af77a42..9e3ed634aa0e 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c @@ -213,7 +213,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, goto out_import; }
- st = dma_buf_map_attachment(import_attach,
DMA_BIDIRECTIONAL);
- st = dma_buf_map_attachment_unlocked(import_attach,
DMA_BIDIRECTIONAL); if (IS_ERR(st)) { err = PTR_ERR(st); goto out_detach; @@ -226,7 +226,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, timeout = -ETIME; } err = timeout > 0 ? 0 : timeout;
- dma_buf_unmap_attachment(import_attach, st,
DMA_BIDIRECTIONAL);
- dma_buf_unmap_attachment_unlocked(import_attach, st,
DMA_BIDIRECTIONAL); out_detach: dma_buf_detach(dmabuf, import_attach); out_import: @@ -296,7 +296,7 @@ static int igt_dmabuf_import(void *arg) goto out_obj; }
- err = dma_buf_vmap(dmabuf, &map);
- err = dma_buf_vmap_unlocked(dmabuf, &map); dma_map = err ? NULL : map.vaddr; if (!dma_map) { pr_err("dma_buf_vmap failed\n");
@@ -337,7 +337,7 @@ static int igt_dmabuf_import(void *arg)
err = 0; out_dma_map:
- dma_buf_vunmap(dmabuf, &map);
- dma_buf_vunmap_unlocked(dmabuf, &map);
out_obj: i915_gem_object_put(obj); out_dmabuf: @@ -358,7 +358,7 @@ static int igt_dmabuf_import_ownership(void *arg) if (IS_ERR(dmabuf)) return PTR_ERR(dmabuf);
- err = dma_buf_vmap(dmabuf, &map);
- err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n");
@@ -367,7 +367,7 @@ static int igt_dmabuf_import_ownership(void *arg) }
memset(ptr, 0xc5, PAGE_SIZE);
- dma_buf_vunmap(dmabuf, &map);
dma_buf_vunmap_unlocked(dmabuf, &map);
obj = to_intel_bo(i915_gem_prime_import(&i915->drm, dmabuf)); if (IS_ERR(obj)) {
@@ -418,7 +418,7 @@ static int igt_dmabuf_export_vmap(void *arg) } i915_gem_object_put(obj);
- err = dma_buf_vmap(dmabuf, &map);
- err = dma_buf_vmap_unlocked(dmabuf, &map); ptr = err ? NULL : map.vaddr; if (!ptr) { pr_err("dma_buf_vmap failed\n");
@@ -435,7 +435,7 @@ static int igt_dmabuf_export_vmap(void *arg) memset(ptr, 0xc5, dmabuf->size);
err = 0;
- dma_buf_vunmap(dmabuf, &map);
- dma_buf_vunmap_unlocked(dmabuf, &map);
out: dma_buf_put(dmabuf); return err; -- 2.37.2
01.09.2022 17:02, Ruhl, Michael J пишет: ...
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
/** dma_buf_unmap_attachment() requires reservation to be* locked. The imported GEM shouldn't share reservation lock,* so it's safe to take the lock.*/if (obj->base.import_attach)i915_gem_object_lock(obj, NULL);There is a lot of stuff going here. Taking the lock may be premature...
__i915_gem_object_pages_fini(obj);The i915_gem_dmabuf.c:i915_gem_object_put_pages_dmabuf is where unmap_attachment is actually called, would it make more sense to make do the locking there?
The __i915_gem_object_put_pages() is invoked with a held reservation lock, while freeing object is a special time when we know that GEM is unused.
The __i915_gem_free_objects() was taking the lock two weeks ago until the change made Chris Wilson [1] reached linux-next.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?...
I don't think we can take the lock within i915_gem_object_put_pages_dmabuf(), it may/should deadlock other code paths.
02.09.2022 13:31, Dmitry Osipenko пишет:
01.09.2022 17:02, Ruhl, Michael J пишет: ...
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
/** dma_buf_unmap_attachment() requires reservation to be* locked. The imported GEM shouldn't share reservation lock,* so it's safe to take the lock.*/if (obj->base.import_attach)i915_gem_object_lock(obj, NULL);There is a lot of stuff going here. Taking the lock may be premature...
__i915_gem_object_pages_fini(obj);The i915_gem_dmabuf.c:i915_gem_object_put_pages_dmabuf is where unmap_attachment is actually called, would it make more sense to make do the locking there?
The __i915_gem_object_put_pages() is invoked with a held reservation lock, while freeing object is a special time when we know that GEM is unused.
The __i915_gem_free_objects() was taking the lock two weeks ago until the change made Chris Wilson [1] reached linux-next.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?...
I don't think we can take the lock within i915_gem_object_put_pages_dmabuf(), it may/should deadlock other code paths.
On the other hand, we can check whether the GEM's refcount number is zero in i915_gem_object_put_pages_dmabuf() and then take the lock if it's zero.
Also, seems it should be possible just to bail out from i915_gem_object_put_pages_dmabuf() if refcount=0. The further drm_prime_gem_destroy() will take care of unmapping. Perhaps this could be the best option, I'll give it a test.
-----Original Message----- From: Dmitry Osipenko digetx@gmail.com Sent: Friday, September 2, 2022 6:39 AM To: Ruhl, Michael J michael.j.ruhl@intel.com; Dmitry Osipenko dmitry.osipenko@collabora.com; Jani Nikula jani.nikula@linux.intel.com; Joonas Lahtinen joonas.lahtinen@linux.intel.com; Vivi, Rodrigo rodrigo.vivi@intel.com; Tvrtko Ursulin tvrtko.ursulin@linux.intel.com; Thomas Hellström thomas_os@shipmail.org; Christian König christian.koenig@amd.com; Chris Wilson chris@chris-wilson.co.uk Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; linux- media@vger.kernel.org; linaro-mm-sig@lists.linaro.org; amd- gfx@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; kernel@collabora.com; virtualization@lists.linux-foundation.org; linux- rdma@vger.kernel.org; linux-arm-msm@vger.kernel.org; David Airlie airlied@linux.ie; Gerd Hoffmann kraxel@redhat.com; Gurchetan Singh gurchetansingh@chromium.org; Chia-I Wu olvaffe@gmail.com; Daniel Vetter daniel@ffwll.ch; Daniel Almeida daniel.almeida@collabora.com; Gert Wollny gert.wollny@collabora.com; Gustavo Padovan gustavo.padovan@collabora.com; Daniel Stone daniel@fooishbar.org; Tomeu Vizoso tomeu.vizoso@collabora.com; Maarten Lankhorst maarten.lankhorst@linux.intel.com; Maxime Ripard mripard@kernel.org; Thomas Zimmermann tzimmermann@suse.de; Rob Clark robdclark@gmail.com; Sumit Semwal sumit.semwal@linaro.org; Pan, Xinhui Xinhui.Pan@amd.com; Thierry Reding thierry.reding@gmail.com; Tomasz Figa tfiga@chromium.org; Marek Szyprowski m.szyprowski@samsung.com; Mauro Carvalho Chehab mchehab@kernel.org; Alex Deucher alexander.deucher@amd.com; Qiang Yu yuq825@gmail.com; Srinivas Kandagatla srinivas.kandagatla@linaro.org; Amol Maheshwari amahesh@qti.qualcomm.com; Jason Gunthorpe jgg@ziepe.ca; Leon Romanovsky leon@kernel.org; Gross, Jurgen jgross@suse.com; Stefano Stabellini sstabellini@kernel.org; Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com; Tomi Valkeinen tomba@kernel.org; Russell King linux@armlinux.org.uk; Lucas Stach l.stach@pengutronix.de; Christian Gmeiner christian.gmeiner@gmail.com Subject: Re: [PATCH v4 06/21] drm/i915: Prepare to dynamic dma-buf locking specification
02.09.2022 13:31, Dmitry Osipenko пишет:
01.09.2022 17:02, Ruhl, Michael J пишет: ...
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
/** dma_buf_unmap_attachment() requires reservation to be* locked. The imported GEM shouldn't share reservation lock,* so it's safe to take the lock.*/if (obj->base.import_attach)i915_gem_object_lock(obj, NULL);There is a lot of stuff going here. Taking the lock may be premature...
__i915_gem_object_pages_fini(obj);The i915_gem_dmabuf.c:i915_gem_object_put_pages_dmabuf is where unmap_attachment is actually called, would it make more sense to make do the locking there?
The __i915_gem_object_put_pages() is invoked with a held reservation lock, while freeing object is a special time when we know that GEM is unused.
The __i915_gem_free_objects() was taking the lock two weeks ago until the change made Chris Wilson [1] reached linux-next.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
next.git/commit/?id=2826d447fbd60e6a05e53d5f918bceb8c04e315c
I don't think we can take the lock within i915_gem_object_put_pages_dmabuf(), it may/should deadlock other code
paths.
On the other hand, we can check whether the GEM's refcount number is zero in i915_gem_object_put_pages_dmabuf() and then take the lock if it's zero.
Also, seems it should be possible just to bail out from i915_gem_object_put_pages_dmabuf() if refcount=0. The further drm_prime_gem_destroy() will take care of unmapping. Perhaps this could be the best option, I'll give it a test.
i915_gem_object_put_pages() is uses the SG, and the usage for drm_prim_gem_destroy()
from __i915_gem_free_objects() doesn't use the SG because it has been "freed" already, I am not sure if that would work...
Hmm.. with that in mind, maybe moving the base.import_attach check to __i915_gem_object_put_pages with your attach check?
atomic_set(&obj->mm.pages_pin_count, 0); if (obj->base.import) i915_gem_object_lock(obj, NULL);
__i915_gem_object_put_pages(obj);
if (obj->base.import) i915_gem_object_unlock(obj, NULL); GEM_BUG_ON(i915_gem_object_has_pages(obj));
Pretty much one step up from the dmabuf interface, but we are guaranteed to not have any pinned pages?
The other caller of __i915_gem_object_pages_fini is the i915_ttm move_notify which should not conflict (export side, not import side).
Since it appears that not locking during the clean up is desirable, trying to make sure take the lock is taken at the last moment might be the right path?
M
On 9/2/22 19:26, Ruhl, Michael J wrote:
02.09.2022 13:31, Dmitry Osipenko пишет:
01.09.2022 17:02, Ruhl, Michael J пишет: ...
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, continue; }
/** dma_buf_unmap_attachment() requires reservation to be* locked. The imported GEM shouldn't share reservation lock,* so it's safe to take the lock.*/if (obj->base.import_attach)i915_gem_object_lock(obj, NULL);There is a lot of stuff going here. Taking the lock may be premature...
__i915_gem_object_pages_fini(obj);The i915_gem_dmabuf.c:i915_gem_object_put_pages_dmabuf is where unmap_attachment is actually called, would it make more sense to make do the locking there?
The __i915_gem_object_put_pages() is invoked with a held reservation lock, while freeing object is a special time when we know that GEM is unused.
The __i915_gem_free_objects() was taking the lock two weeks ago until the change made Chris Wilson [1] reached linux-next.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
next.git/commit/?id=2826d447fbd60e6a05e53d5f918bceb8c04e315c
I don't think we can take the lock within i915_gem_object_put_pages_dmabuf(), it may/should deadlock other code
paths.
On the other hand, we can check whether the GEM's refcount number is zero in i915_gem_object_put_pages_dmabuf() and then take the lock if it's zero.
Also, seems it should be possible just to bail out from i915_gem_object_put_pages_dmabuf() if refcount=0. The further drm_prime_gem_destroy() will take care of unmapping. Perhaps this could be the best option, I'll give it a test.
i915_gem_object_put_pages() is uses the SG, and the usage for drm_prim_gem_destroy()
from __i915_gem_free_objects() doesn't use the SG because it has been "freed" already, I am not sure if that would work...
Hmm.. with that in mind, maybe moving the base.import_attach check to __i915_gem_object_put_pages with your attach check?
I see you meant __i915_gem_object_pages_fini() here.
atomic_set(&obj->mm.pages_pin_count, 0); if (obj->base.import) i915_gem_object_lock(obj, NULL);
__i915_gem_object_put_pages(obj);
if (obj->base.import) i915_gem_object_unlock(obj, NULL); GEM_BUG_ON(i915_gem_object_has_pages(obj));
Pretty much one step up from the dmabuf interface, but we are guaranteed to not have any pinned pages?
Importer shouldn't hold pages outside of dma-buf API, otherwise it should be a bug.
The other caller of __i915_gem_object_pages_fini is the i915_ttm move_notify which should not conflict (export side, not import side).
Since it appears that not locking during the clean up is desirable, trying to make sure take the lock is taken at the last moment might be the right path?
Reducing the scope of locking usually is preferred more. Yours suggestion works okay, I couldn't spot any problems at least for a non-TTM code paths.
It's indeed a bit not nice that __i915_gem_object_pages_fini() is used by TTM, but should be safe for imported objects. Will be great if anyone from i915 maintainers could ack this variant.
Prepare OMAP DRM driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 393f82e26927..8e194dbc9506 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -125,7 +125,7 @@ struct drm_gem_object *omap_gem_prime_import(struct drm_device *dev,
get_dma_buf(dma_buf);
- sgt = dma_buf_map_attachment(attach, DMA_TO_DEVICE); + sgt = dma_buf_map_attachment_unlocked(attach, DMA_TO_DEVICE); if (IS_ERR(sgt)) { ret = PTR_ERR(sgt); goto fail_detach; @@ -142,7 +142,7 @@ struct drm_gem_object *omap_gem_prime_import(struct drm_device *dev, return obj;
fail_unmap: - dma_buf_unmap_attachment(attach, sgt, DMA_TO_DEVICE); + dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_TO_DEVICE); fail_detach: dma_buf_detach(dma_buf, attach); dma_buf_put(dma_buf);
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare OMAP DRM driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 393f82e26927..8e194dbc9506 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -125,7 +125,7 @@ struct drm_gem_object *omap_gem_prime_import(struct drm_device *dev, get_dma_buf(dma_buf);
- sgt = dma_buf_map_attachment(attach, DMA_TO_DEVICE);
- sgt = dma_buf_map_attachment_unlocked(attach, DMA_TO_DEVICE); if (IS_ERR(sgt)) { ret = PTR_ERR(sgt); goto fail_detach;
@@ -142,7 +142,7 @@ struct drm_gem_object *omap_gem_prime_import(struct drm_device *dev, return obj; fail_unmap:
- dma_buf_unmap_attachment(attach, sgt, DMA_TO_DEVICE);
- dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_TO_DEVICE); fail_detach: dma_buf_detach(dma_buf, attach); dma_buf_put(dma_buf);
Prepare Tegra DRM driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/tegra/gem.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index 81991090adcc..b09b8ab40ae4 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -84,7 +84,7 @@ static struct host1x_bo_mapping *tegra_bo_pin(struct device *dev, struct host1x_ goto free; }
- map->sgt = dma_buf_map_attachment(map->attach, direction); + map->sgt = dma_buf_map_attachment_unlocked(map->attach, direction); if (IS_ERR(map->sgt)) { dma_buf_detach(buf, map->attach); err = PTR_ERR(map->sgt); @@ -160,7 +160,8 @@ static struct host1x_bo_mapping *tegra_bo_pin(struct device *dev, struct host1x_ static void tegra_bo_unpin(struct host1x_bo_mapping *map) { if (map->attach) { - dma_buf_unmap_attachment(map->attach, map->sgt, map->direction); + dma_buf_unmap_attachment_unlocked(map->attach, map->sgt, + map->direction); dma_buf_detach(map->attach->dmabuf, map->attach); } else { dma_unmap_sgtable(map->dev, map->sgt, map->direction, 0); @@ -181,7 +182,7 @@ static void *tegra_bo_mmap(struct host1x_bo *bo) if (obj->vaddr) { return obj->vaddr; } else if (obj->gem.import_attach) { - ret = dma_buf_vmap(obj->gem.import_attach->dmabuf, &map); + ret = dma_buf_vmap_unlocked(obj->gem.import_attach->dmabuf, &map); return ret ? NULL : map.vaddr; } else { return vmap(obj->pages, obj->num_pages, VM_MAP, @@ -197,7 +198,7 @@ static void tegra_bo_munmap(struct host1x_bo *bo, void *addr) if (obj->vaddr) return; else if (obj->gem.import_attach) - dma_buf_vunmap(obj->gem.import_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(obj->gem.import_attach->dmabuf, &map); else vunmap(addr); } @@ -461,7 +462,7 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
get_dma_buf(buf);
- bo->sgt = dma_buf_map_attachment(attach, DMA_TO_DEVICE); + bo->sgt = dma_buf_map_attachment_unlocked(attach, DMA_TO_DEVICE); if (IS_ERR(bo->sgt)) { err = PTR_ERR(bo->sgt); goto detach; @@ -479,7 +480,7 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
detach: if (!IS_ERR_OR_NULL(bo->sgt)) - dma_buf_unmap_attachment(attach, bo->sgt, DMA_TO_DEVICE); + dma_buf_unmap_attachment_unlocked(attach, bo->sgt, DMA_TO_DEVICE);
dma_buf_detach(buf, attach); dma_buf_put(buf); @@ -508,8 +509,8 @@ void tegra_bo_free_object(struct drm_gem_object *gem) tegra_bo_iommu_unmap(tegra, bo);
if (gem->import_attach) { - dma_buf_unmap_attachment(gem->import_attach, bo->sgt, - DMA_TO_DEVICE); + dma_buf_unmap_attachment_unlocked(gem->import_attach, bo->sgt, + DMA_TO_DEVICE); drm_prime_gem_destroy(gem, NULL); } else { tegra_bo_free(gem->dev, bo);
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare Tegra DRM driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/tegra/gem.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index 81991090adcc..b09b8ab40ae4 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -84,7 +84,7 @@ static struct host1x_bo_mapping *tegra_bo_pin(struct device *dev, struct host1x_ goto free; }
map->sgt = dma_buf_map_attachment(map->attach, direction);
map->sgt = dma_buf_map_attachment_unlocked(map->attach, direction);@@ -160,7 +160,8 @@ static struct host1x_bo_mapping *tegra_bo_pin(struct device *dev, struct host1x_ static void tegra_bo_unpin(struct host1x_bo_mapping *map) { if (map->attach) {
dma_buf_unmap_attachment(map->attach, map->sgt, map->direction);
dma_buf_unmap_attachment_unlocked(map->attach, map->sgt,map->direction);@@ -181,7 +182,7 @@ static void *tegra_bo_mmap(struct host1x_bo *bo) if (obj->vaddr) { return obj->vaddr; } else if (obj->gem.import_attach) {
ret = dma_buf_vmap(obj->gem.import_attach->dmabuf, &map);
ret = dma_buf_vmap_unlocked(obj->gem.import_attach->dmabuf, &map);@@ -197,7 +198,7 @@ static void tegra_bo_munmap(struct host1x_bo *bo, void *addr) if (obj->vaddr) return; else if (obj->gem.import_attach)
dma_buf_vunmap(obj->gem.import_attach->dmabuf, &map);
dma_buf_vunmap_unlocked(obj->gem.import_attach->dmabuf, &map);@@ -461,7 +462,7 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm, get_dma_buf(buf);
- bo->sgt = dma_buf_map_attachment(attach, DMA_TO_DEVICE);
- bo->sgt = dma_buf_map_attachment_unlocked(attach, DMA_TO_DEVICE); if (IS_ERR(bo->sgt)) { err = PTR_ERR(bo->sgt); goto detach;
@@ -479,7 +480,7 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm, detach: if (!IS_ERR_OR_NULL(bo->sgt))
dma_buf_unmap_attachment(attach, bo->sgt, DMA_TO_DEVICE);
dma_buf_unmap_attachment_unlocked(attach, bo->sgt, DMA_TO_DEVICE);dma_buf_detach(buf, attach); dma_buf_put(buf); @@ -508,8 +509,8 @@ void tegra_bo_free_object(struct drm_gem_object *gem) tegra_bo_iommu_unmap(tegra, bo); if (gem->import_attach) {
dma_buf_unmap_attachment(gem->import_attach, bo->sgt,DMA_TO_DEVICE);
dma_buf_unmap_attachment_unlocked(gem->import_attach, bo->sgt,DMA_TO_DEVICE);
Prepare Etnaviv driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 3fa2da149639..7031db145a77 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -65,7 +65,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj) struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);
if (etnaviv_obj->vaddr) - dma_buf_vunmap(etnaviv_obj->base.import_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);
/* Don't drop the pages for imported dmabuf, as they are not * ours, just free the array we allocated:
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare Etnaviv driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Interesting, where is the matching vmap()?
Anyway, this patch is Acked-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 3fa2da149639..7031db145a77 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -65,7 +65,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj) struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr); if (etnaviv_obj->vaddr)
dma_buf_vunmap(etnaviv_obj->base.import_attach->dmabuf, &map);
dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);/* Don't drop the pages for imported dmabuf, as they are not * ours, just free the array we allocated:
On 9/1/22 09:50, Christian König wrote:
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare Etnaviv driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Interesting, where is the matching vmap()?
Anyway, this patch is Acked-by: Christian König christian.koenig@amd.com
Etnaviv maps GEM only once and then unmaps it when GEM is destroyed. The dma-buf vmapping should happen under the reservation lock, hence only the release function needs to be changed to the unlocked variant.
Lucas/Christian(Gmeiner), could you please check that I haven't missed anything?
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 3fa2da149639..7031db145a77 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -65,7 +65,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj) struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr); if (etnaviv_obj->vaddr) - dma_buf_vunmap(etnaviv_obj->base.import_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map); /* Don't drop the pages for imported dmabuf, as they are not * ours, just free the array we allocated:
To whoom it may belong: Deepest sorrow to inform you that my husband Helmut passed by on July 21. This access will be cleared in a view days. Irene -------- Ursprüngliche Nachricht --------Von: Christian König christian.koenig@amd.com Datum: 01.09.22 08:50 (GMT+01:00) An: Dmitry Osipenko dmitry.osipenko@collabora.com, David Airlie airlied@linux.ie, Gerd Hoffmann kraxel@redhat.com, Gurchetan Singh gurchetansingh@chromium.org, Chia-I Wu olvaffe@gmail.com, Daniel Vetter daniel@ffwll.ch, Daniel Almeida daniel.almeida@collabora.com, Gert Wollny gert.wollny@collabora.com, Gustavo Padovan gustavo.padovan@collabora.com, Daniel Stone daniel@fooishbar.org, Tomeu Vizoso tomeu.vizoso@collabora.com, Maarten Lankhorst maarten.lankhorst@linux.intel.com, Maxime Ripard mripard@kernel.org, Thomas Zimmermann tzimmermann@suse.de, Rob Clark robdclark@gmail.com, Sumit Semwal sumit.semwal@linaro.org, "Pan, Xinhui" Xinhui.Pan@amd.com, Thierry Reding thierry.reding@gmail.com, Tomasz Figa tfiga@chromium.org, Marek Szyprowski m.szyprowski@samsung.com, Mauro Carvalho Chehab mchehab@kernel.org, Alex Deucher alexander.deucher@amd.com, Jani Nikula jani.nikula@linux.intel.com, Joonas Lahtinen joonas.lahtinen@linux.intel.com, Rodrigo Vivi rodrigo.vivi@intel.com, Tvrtko Ursulin tvrtko.ursulin@linux.intel.com, Thomas Hellström thomas_os@shipmail.org, Qiang Yu yuq825@gmail.com, Srinivas Kandagatla srinivas.kandagatla@linaro.org, Amol Maheshwari amahesh@qti.qualcomm.com, Jason Gunthorpe jgg@ziepe.ca, Leon Romanovsky leon@kernel.org, Juergen Gross jgross@suse.com, Stefano Stabellini sstabellini@kernel.org, Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com, Tomi Valkeinen tomba@kernel.org, Russell King linux@armlinux.org.uk, Lucas Stach l.stach@pengutronix.de, Christian Gmeiner christian.gmeiner@gmail.com Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Dmitry Osipenko digetx@gmail.com, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, kernel@collabora.com, virtualization@lists.linux-foundation.org, linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org Betreff: Re: [PATCH v4 09/21] drm/etnaviv: Prepare to dynamic dma-buf locking specification Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:> Prepare Etnaviv driver to the common dynamic dma-buf locking convention> by starting to use the unlocked versions of dma-buf API functions.>> Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.comInteresting, where is the matching vmap()?Anyway, this patch is Acked-by: Christian König christian.koenig@amd.com> ---> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +-> 1 file changed, 1 insertion(+), 1 deletion(-)>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c> index 3fa2da149639..7031db145a77 100644> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c> @@ -65,7 +65,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj)> struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);> > if (etnaviv_obj->vaddr)> - dma_buf_vunmap(etnaviv_obj->base.import_attach->dmabuf, &map);> + dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);> > /* Don't drop the pages for imported dmabuf, as they are not> * ours, just free the array we allocated:
Prepare InfiniBand drivers to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/infiniband/core/umem_dmabuf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/core/umem_dmabuf.c b/drivers/infiniband/core/umem_dmabuf.c index 04c04e6d24c3..43b26bc12288 100644 --- a/drivers/infiniband/core/umem_dmabuf.c +++ b/drivers/infiniband/core/umem_dmabuf.c @@ -26,7 +26,8 @@ int ib_umem_dmabuf_map_pages(struct ib_umem_dmabuf *umem_dmabuf) if (umem_dmabuf->sgt) goto wait_fence;
- sgt = dma_buf_map_attachment(umem_dmabuf->attach, DMA_BIDIRECTIONAL); + sgt = dma_buf_map_attachment_unlocked(umem_dmabuf->attach, + DMA_BIDIRECTIONAL); if (IS_ERR(sgt)) return PTR_ERR(sgt);
@@ -102,8 +103,8 @@ void ib_umem_dmabuf_unmap_pages(struct ib_umem_dmabuf *umem_dmabuf) umem_dmabuf->last_sg_trim = 0; }
- dma_buf_unmap_attachment(umem_dmabuf->attach, umem_dmabuf->sgt, - DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(umem_dmabuf->attach, umem_dmabuf->sgt, + DMA_BIDIRECTIONAL);
umem_dmabuf->sgt = NULL; }
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare InfiniBand drivers to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/infiniband/core/umem_dmabuf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/core/umem_dmabuf.c b/drivers/infiniband/core/umem_dmabuf.c index 04c04e6d24c3..43b26bc12288 100644 --- a/drivers/infiniband/core/umem_dmabuf.c +++ b/drivers/infiniband/core/umem_dmabuf.c @@ -26,7 +26,8 @@ int ib_umem_dmabuf_map_pages(struct ib_umem_dmabuf *umem_dmabuf) if (umem_dmabuf->sgt) goto wait_fence;
- sgt = dma_buf_map_attachment(umem_dmabuf->attach, DMA_BIDIRECTIONAL);
- sgt = dma_buf_map_attachment_unlocked(umem_dmabuf->attach,
DMA_BIDIRECTIONAL);@@ -102,8 +103,8 @@ void ib_umem_dmabuf_unmap_pages(struct ib_umem_dmabuf *umem_dmabuf) umem_dmabuf->last_sg_trim = 0; }
- dma_buf_unmap_attachment(umem_dmabuf->attach, umem_dmabuf->sgt,
DMA_BIDIRECTIONAL);
- dma_buf_unmap_attachment_unlocked(umem_dmabuf->attach, umem_dmabuf->sgt,
DMA_BIDIRECTIONAL);umem_dmabuf->sgt = NULL; }
Prepare fastrpc to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/misc/fastrpc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 93ebd174d848..6fcfb2e9f7a7 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -310,8 +310,8 @@ static void fastrpc_free_map(struct kref *ref) return; } } - dma_buf_unmap_attachment(map->attach, map->table, - DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(map->attach, map->table, + DMA_BIDIRECTIONAL); dma_buf_detach(map->buf, map->attach); dma_buf_put(map->buf); } @@ -726,7 +726,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd, goto attach_err; }
- map->table = dma_buf_map_attachment(map->attach, DMA_BIDIRECTIONAL); + map->table = dma_buf_map_attachment_unlocked(map->attach, DMA_BIDIRECTIONAL); if (IS_ERR(map->table)) { err = PTR_ERR(map->table); goto map_err;
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare fastrpc to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/misc/fastrpc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 93ebd174d848..6fcfb2e9f7a7 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -310,8 +310,8 @@ static void fastrpc_free_map(struct kref *ref) return; } }
dma_buf_unmap_attachment(map->attach, map->table,DMA_BIDIRECTIONAL);
dma_buf_unmap_attachment_unlocked(map->attach, map->table,DMA_BIDIRECTIONAL);@@ -726,7 +726,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd, goto attach_err; }
- map->table = dma_buf_map_attachment(map->attach, DMA_BIDIRECTIONAL);
- map->table = dma_buf_map_attachment_unlocked(map->attach, DMA_BIDIRECTIONAL); if (IS_ERR(map->table)) { err = PTR_ERR(map->table); goto map_err;
On 31/08/2022 16:37, Dmitry Osipenko wrote:
Prepare fastrpc to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
LGTM,
Incase you plan to take it via another tree.
Acked-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
--srini
drivers/misc/fastrpc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 93ebd174d848..6fcfb2e9f7a7 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -310,8 +310,8 @@ static void fastrpc_free_map(struct kref *ref) return; } }
dma_buf_unmap_attachment(map->attach, map->table,DMA_BIDIRECTIONAL);
dma_buf_unmap_attachment_unlocked(map->attach, map->table,DMA_BIDIRECTIONAL);@@ -726,7 +726,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd, goto attach_err; }
- map->table = dma_buf_map_attachment(map->attach, DMA_BIDIRECTIONAL);
- map->table = dma_buf_map_attachment_unlocked(map->attach, DMA_BIDIRECTIONAL); if (IS_ERR(map->table)) { err = PTR_ERR(map->table); goto map_err;
On 31/08/2022 16:37, Dmitry Osipenko wrote:
Prepare fastrpc to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
LGTM,
Incase you plan to take it via another tree.
Acked-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org
--srini
drivers/misc/fastrpc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 93ebd174d848..6fcfb2e9f7a7 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -310,8 +310,8 @@ static void fastrpc_free_map(struct kref *ref) return; } }
dma_buf_unmap_attachment(map->attach, map->table,DMA_BIDIRECTIONAL);
dma_buf_unmap_attachment_unlocked(map->attach, map->table,DMA_BIDIRECTIONAL);@@ -726,7 +726,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd, goto attach_err; }
- map->table = dma_buf_map_attachment(map->attach, DMA_BIDIRECTIONAL);
- map->table = dma_buf_map_attachment_unlocked(map->attach, DMA_BIDIRECTIONAL); if (IS_ERR(map->table)) { err = PTR_ERR(map->table); goto map_err;
Prepare gntdev driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/xen/gntdev-dmabuf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index 940e5e9e8a54..4440e626b797 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -600,7 +600,7 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev,
gntdev_dmabuf->u.imp.attach = attach;
- sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); + sgt = dma_buf_map_attachment_unlocked(attach, DMA_BIDIRECTIONAL); if (IS_ERR(sgt)) { ret = ERR_CAST(sgt); goto fail_detach; @@ -658,7 +658,7 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev, fail_end_access: dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs, count); fail_unmap: - dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_BIDIRECTIONAL); fail_detach: dma_buf_detach(dma_buf, attach); fail_free_obj: @@ -708,8 +708,8 @@ static int dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32 fd) attach = gntdev_dmabuf->u.imp.attach;
if (gntdev_dmabuf->u.imp.sgt) - dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt, - DMA_BIDIRECTIONAL); + dma_buf_unmap_attachment_unlocked(attach, gntdev_dmabuf->u.imp.sgt, + DMA_BIDIRECTIONAL); dma_buf = attach->dmabuf; dma_buf_detach(attach->dmabuf, attach); dma_buf_put(dma_buf);
On 31.08.22 17:37, Dmitry Osipenko wrote:
Prepare gntdev driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Juergen Gross jgross@suse.com
Juergen
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare gntdev driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/xen/gntdev-dmabuf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index 940e5e9e8a54..4440e626b797 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -600,7 +600,7 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev, gntdev_dmabuf->u.imp.attach = attach;
- sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
- sgt = dma_buf_map_attachment_unlocked(attach, DMA_BIDIRECTIONAL); if (IS_ERR(sgt)) { ret = ERR_CAST(sgt); goto fail_detach;
@@ -658,7 +658,7 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev, fail_end_access: dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs, count); fail_unmap:
- dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
- dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_BIDIRECTIONAL); fail_detach: dma_buf_detach(dma_buf, attach); fail_free_obj:
@@ -708,8 +708,8 @@ static int dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32 fd) attach = gntdev_dmabuf->u.imp.attach; if (gntdev_dmabuf->u.imp.sgt)
dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt,DMA_BIDIRECTIONAL);
dma_buf_unmap_attachment_unlocked(attach, gntdev_dmabuf->u.imp.sgt,DMA_BIDIRECTIONAL);
Prepare V4L2 memory allocators to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Acked-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/media/common/videobuf2/videobuf2-dma-contig.c | 11 ++++++----- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 8 ++++---- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index 678b359717c4..79f4d8301fbb 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -101,7 +101,7 @@ static void *vb2_dc_vaddr(struct vb2_buffer *vb, void *buf_priv) if (buf->db_attach) { struct iosys_map map;
- if (!dma_buf_vmap(buf->db_attach->dmabuf, &map)) + if (!dma_buf_vmap_unlocked(buf->db_attach->dmabuf, &map)) buf->vaddr = map.vaddr;
return buf->vaddr; @@ -711,7 +711,7 @@ static int vb2_dc_map_dmabuf(void *mem_priv) }
/* get the associated scatterlist for this buffer */ - sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); + sgt = dma_buf_map_attachment_unlocked(buf->db_attach, buf->dma_dir); if (IS_ERR(sgt)) { pr_err("Error getting dmabuf scatterlist\n"); return -EINVAL; @@ -722,7 +722,8 @@ static int vb2_dc_map_dmabuf(void *mem_priv) if (contig_size < buf->size) { pr_err("contiguous chunk is too small %lu/%lu\n", contig_size, buf->size); - dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir); + dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, + buf->dma_dir); return -EFAULT; }
@@ -750,10 +751,10 @@ static void vb2_dc_unmap_dmabuf(void *mem_priv) }
if (buf->vaddr) { - dma_buf_vunmap(buf->db_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(buf->db_attach->dmabuf, &map); buf->vaddr = NULL; } - dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir); + dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, buf->dma_dir);
buf->dma_addr = 0; buf->dma_sgt = NULL; diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index fa69158a65b1..36ecdea8d707 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -309,7 +309,7 @@ static void *vb2_dma_sg_vaddr(struct vb2_buffer *vb, void *buf_priv)
if (!buf->vaddr) { if (buf->db_attach) { - ret = dma_buf_vmap(buf->db_attach->dmabuf, &map); + ret = dma_buf_vmap_unlocked(buf->db_attach->dmabuf, &map); buf->vaddr = ret ? NULL : map.vaddr; } else { buf->vaddr = vm_map_ram(buf->pages, buf->num_pages, -1); @@ -565,7 +565,7 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv) }
/* get the associated scatterlist for this buffer */ - sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); + sgt = dma_buf_map_attachment_unlocked(buf->db_attach, buf->dma_dir); if (IS_ERR(sgt)) { pr_err("Error getting dmabuf scatterlist\n"); return -EINVAL; @@ -594,10 +594,10 @@ static void vb2_dma_sg_unmap_dmabuf(void *mem_priv) }
if (buf->vaddr) { - dma_buf_vunmap(buf->db_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(buf->db_attach->dmabuf, &map); buf->vaddr = NULL; } - dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir); + dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, buf->dma_dir);
buf->dma_sgt = NULL; } diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index 948152f1596b..7831bf545874 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -376,7 +376,7 @@ static int vb2_vmalloc_map_dmabuf(void *mem_priv) struct iosys_map map; int ret;
- ret = dma_buf_vmap(buf->dbuf, &map); + ret = dma_buf_vmap_unlocked(buf->dbuf, &map); if (ret) return -EFAULT; buf->vaddr = map.vaddr; @@ -389,7 +389,7 @@ static void vb2_vmalloc_unmap_dmabuf(void *mem_priv) struct vb2_vmalloc_buf *buf = mem_priv; struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
- dma_buf_vunmap(buf->dbuf, &map); + dma_buf_vunmap_unlocked(buf->dbuf, &map); buf->vaddr = NULL; }
@@ -399,7 +399,7 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv) struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
if (buf->vaddr) - dma_buf_vunmap(buf->dbuf, &map); + dma_buf_vunmap_unlocked(buf->dbuf, &map);
kfree(buf); }
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare V4L2 memory allocators to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Acked-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/media/common/videobuf2/videobuf2-dma-contig.c | 11 ++++++----- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 8 ++++---- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index 678b359717c4..79f4d8301fbb 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -101,7 +101,7 @@ static void *vb2_dc_vaddr(struct vb2_buffer *vb, void *buf_priv) if (buf->db_attach) { struct iosys_map map;
if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
if (!dma_buf_vmap_unlocked(buf->db_attach->dmabuf, &map)) buf->vaddr = map.vaddr;return buf->vaddr; @@ -711,7 +711,7 @@ static int vb2_dc_map_dmabuf(void *mem_priv) } /* get the associated scatterlist for this buffer */
- sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
- sgt = dma_buf_map_attachment_unlocked(buf->db_attach, buf->dma_dir); if (IS_ERR(sgt)) { pr_err("Error getting dmabuf scatterlist\n"); return -EINVAL;
@@ -722,7 +722,8 @@ static int vb2_dc_map_dmabuf(void *mem_priv) if (contig_size < buf->size) { pr_err("contiguous chunk is too small %lu/%lu\n", contig_size, buf->size);
dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt,buf->dma_dir);@@ -750,10 +751,10 @@ static void vb2_dc_unmap_dmabuf(void *mem_priv) } if (buf->vaddr) {
dma_buf_vunmap(buf->db_attach->dmabuf, &map);
dma_buf_vunmap_unlocked(buf->db_attach->dmabuf, &map);
- dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
- dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, buf->dma_dir);
buf->dma_addr = 0; buf->dma_sgt = NULL; diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index fa69158a65b1..36ecdea8d707 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -309,7 +309,7 @@ static void *vb2_dma_sg_vaddr(struct vb2_buffer *vb, void *buf_priv) if (!buf->vaddr) { if (buf->db_attach) {
ret = dma_buf_vmap(buf->db_attach->dmabuf, &map);
ret = dma_buf_vmap_unlocked(buf->db_attach->dmabuf, &map); buf->vaddr = ret ? NULL : map.vaddr;@@ -565,7 +565,7 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv) } /* get the associated scatterlist for this buffer */
- sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
- sgt = dma_buf_map_attachment_unlocked(buf->db_attach, buf->dma_dir); if (IS_ERR(sgt)) { pr_err("Error getting dmabuf scatterlist\n"); return -EINVAL;
@@ -594,10 +594,10 @@ static void vb2_dma_sg_unmap_dmabuf(void *mem_priv) } if (buf->vaddr) {
dma_buf_vunmap(buf->db_attach->dmabuf, &map);
dma_buf_vunmap_unlocked(buf->db_attach->dmabuf, &map);
- dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
- dma_buf_unmap_attachment_unlocked(buf->db_attach, sgt, buf->dma_dir);
buf->dma_sgt = NULL; } diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index 948152f1596b..7831bf545874 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -376,7 +376,7 @@ static int vb2_vmalloc_map_dmabuf(void *mem_priv) struct iosys_map map; int ret;
- ret = dma_buf_vmap(buf->dbuf, &map);
- ret = dma_buf_vmap_unlocked(buf->dbuf, &map); if (ret) return -EFAULT; buf->vaddr = map.vaddr;
@@ -389,7 +389,7 @@ static void vb2_vmalloc_unmap_dmabuf(void *mem_priv) struct vb2_vmalloc_buf *buf = mem_priv; struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
- dma_buf_vunmap(buf->dbuf, &map);
- dma_buf_vunmap_unlocked(buf->dbuf, &map); buf->vaddr = NULL; }
@@ -399,7 +399,7 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv) struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr); if (buf->vaddr)
dma_buf_vunmap(buf->dbuf, &map);
dma_buf_vunmap_unlocked(buf->dbuf, &map);kfree(buf); }
Prepare Tegra video decoder driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c b/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c index 69c346148070..1c5b94989aec 100644 --- a/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c +++ b/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c @@ -38,7 +38,7 @@ static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry) if (entry->vde->domain) tegra_vde_iommu_unmap(entry->vde, entry->iova);
- dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir); + dma_buf_unmap_attachment_unlocked(entry->a, entry->sgt, entry->dma_dir); dma_buf_detach(dmabuf, entry->a); dma_buf_put(dmabuf);
@@ -102,7 +102,7 @@ int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde, goto err_unlock; }
- sgt = dma_buf_map_attachment(attachment, dma_dir); + sgt = dma_buf_map_attachment_unlocked(attachment, dma_dir); if (IS_ERR(sgt)) { dev_err(dev, "Failed to get dmabufs sg_table\n"); err = PTR_ERR(sgt); @@ -152,7 +152,7 @@ int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde, err_free: kfree(entry); err_unmap: - dma_buf_unmap_attachment(attachment, sgt, dma_dir); + dma_buf_unmap_attachment_unlocked(attachment, sgt, dma_dir); err_detach: dma_buf_detach(dmabuf, attachment); err_unlock:
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Prepare Tegra video decoder driver to the common dynamic dma-buf locking convention by starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c b/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c index 69c346148070..1c5b94989aec 100644 --- a/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c +++ b/drivers/media/platform/nvidia/tegra-vde/dmabuf-cache.c @@ -38,7 +38,7 @@ static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry) if (entry->vde->domain) tegra_vde_iommu_unmap(entry->vde, entry->iova);
- dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
- dma_buf_unmap_attachment_unlocked(entry->a, entry->sgt, entry->dma_dir); dma_buf_detach(dmabuf, entry->a); dma_buf_put(dmabuf);
@@ -102,7 +102,7 @@ int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde, goto err_unlock; }
- sgt = dma_buf_map_attachment(attachment, dma_dir);
- sgt = dma_buf_map_attachment_unlocked(attachment, dma_dir); if (IS_ERR(sgt)) { dev_err(dev, "Failed to get dmabufs sg_table\n"); err = PTR_ERR(sgt);
@@ -152,7 +152,7 @@ int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde, err_free: kfree(entry); err_unmap:
- dma_buf_unmap_attachment(attachment, sgt, dma_dir);
- dma_buf_unmap_attachment_unlocked(attachment, sgt, dma_dir); err_detach: dma_buf_detach(dmabuf, attachment); err_unlock:
Move dma_buf_vmap/vunmap_unlocked() functions to the dynamic locking specification by asserting that the reservation lock is held.
Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/dma-buf/dma-buf.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 51fb69048853..ceea4839c641 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1449,6 +1449,8 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map) if (WARN_ON(!dmabuf)) return -EINVAL;
+ dma_resv_assert_held(dmabuf->resv); + if (!dmabuf->ops->vmap) return -EINVAL;
@@ -1509,6 +1511,8 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map) if (WARN_ON(!dmabuf)) return;
+ dma_resv_assert_held(dmabuf->resv); + BUG_ON(iosys_map_is_null(&dmabuf->vmap_ptr)); BUG_ON(dmabuf->vmapping_counter == 0); BUG_ON(!iosys_map_is_equal(&dmabuf->vmap_ptr, map));
Move dma-buf attachment API functions to the dynamic locking specification by taking the reservation lock around the mapping operations. The strict locking convention prevents deadlock situations for dma-buf importers and exporters.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/dma-buf/dma-buf.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index ceea4839c641..073942bf5ae9 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -858,8 +858,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dma_buf_is_dynamic(dmabuf)) { struct sg_table *sgt;
+ dma_resv_lock(attach->dmabuf->resv, NULL); if (dma_buf_is_dynamic(attach->dmabuf)) { - dma_resv_lock(attach->dmabuf->resv, NULL); ret = dmabuf->ops->pin(attach); if (ret) goto err_unlock; @@ -872,8 +872,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, ret = PTR_ERR(sgt); goto err_unpin; } - if (dma_buf_is_dynamic(attach->dmabuf)) - dma_resv_unlock(attach->dmabuf->resv); + dma_resv_unlock(attach->dmabuf->resv); attach->sgt = sgt; attach->dir = DMA_BIDIRECTIONAL; } @@ -889,8 +888,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dmabuf->ops->unpin(attach);
err_unlock: - if (dma_buf_is_dynamic(attach->dmabuf)) - dma_resv_unlock(attach->dmabuf->resv); + dma_resv_unlock(attach->dmabuf->resv);
dma_buf_detach(dmabuf, attach); return ERR_PTR(ret); @@ -936,21 +934,19 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) if (WARN_ON(!dmabuf || !attach)) return;
+ dma_resv_lock(attach->dmabuf->resv, NULL); + if (attach->sgt) { - if (dma_buf_is_dynamic(attach->dmabuf)) - dma_resv_lock(attach->dmabuf->resv, NULL);
__unmap_dma_buf(attach, attach->sgt, attach->dir);
- if (dma_buf_is_dynamic(attach->dmabuf)) { + if (dma_buf_is_dynamic(attach->dmabuf)) dmabuf->ops->unpin(attach); - dma_resv_unlock(attach->dmabuf->resv); - } } - - dma_resv_lock(dmabuf->resv, NULL); list_del(&attach->node); + dma_resv_unlock(dmabuf->resv); + if (dmabuf->ops->detach) dmabuf->ops->detach(dmabuf, attach);
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Move dma-buf attachment API functions to the dynamic locking specification by taking the reservation lock around the mapping operations. The strict locking convention prevents deadlock situations for dma-buf importers and exporters.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Reviewed-by: Christian König christian.koenig@amd.com
drivers/dma-buf/dma-buf.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index ceea4839c641..073942bf5ae9 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -858,8 +858,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dma_buf_is_dynamic(dmabuf)) { struct sg_table *sgt;
dma_resv_lock(attach->dmabuf->resv, NULL);
dma_resv_lock(attach->dmabuf->resv, NULL); ret = dmabuf->ops->pin(attach); if (ret) goto err_unlock;@@ -872,8 +872,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, ret = PTR_ERR(sgt); goto err_unpin; }
if (dma_buf_is_dynamic(attach->dmabuf))dma_resv_unlock(attach->dmabuf->resv);
dma_resv_unlock(attach->dmabuf->resv);@@ -889,8 +888,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, dmabuf->ops->unpin(attach); err_unlock:
- if (dma_buf_is_dynamic(attach->dmabuf))
dma_resv_unlock(attach->dmabuf->resv);
- dma_resv_unlock(attach->dmabuf->resv);
dma_buf_detach(dmabuf, attach); return ERR_PTR(ret); @@ -936,21 +934,19 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) if (WARN_ON(!dmabuf || !attach)) return;
- dma_resv_lock(attach->dmabuf->resv, NULL);
- if (attach->sgt) {
if (dma_buf_is_dynamic(attach->dmabuf))dma_resv_lock(attach->dmabuf->resv, NULL);__unmap_dma_buf(attach, attach->sgt, attach->dir);
if (dma_buf_is_dynamic(attach->dmabuf)) {
if (dma_buf_is_dynamic(attach->dmabuf)) dmabuf->ops->unpin(attach);
dma_resv_unlock(attach->dmabuf->resv);}- dma_resv_lock(dmabuf->resv, NULL); list_del(&attach->node);
- dma_resv_unlock(dmabuf->resv);
- if (dmabuf->ops->detach) dmabuf->ops->detach(dmabuf, attach);
Move dma-buf attachment mapping functions to the dynamic locking specification by asserting that the reservation lock is held.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/dma-buf/dma-buf.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 073942bf5ae9..8e928fe6e8df 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1037,8 +1037,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, if (WARN_ON(!attach || !attach->dmabuf)) return ERR_PTR(-EINVAL);
- if (dma_buf_attachment_is_dynamic(attach)) - dma_resv_assert_held(attach->dmabuf->resv); + dma_resv_assert_held(attach->dmabuf->resv);
if (attach->sgt) { /* @@ -1053,7 +1052,6 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, }
if (dma_buf_is_dynamic(attach->dmabuf)) { - dma_resv_assert_held(attach->dmabuf->resv); if (!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) { r = attach->dmabuf->ops->pin(attach); if (r) @@ -1142,15 +1140,11 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, if (WARN_ON(!attach || !attach->dmabuf || !sg_table)) return;
- if (dma_buf_attachment_is_dynamic(attach)) - dma_resv_assert_held(attach->dmabuf->resv); + dma_resv_assert_held(attach->dmabuf->resv);
if (attach->sgt == sg_table) return;
- if (dma_buf_is_dynamic(attach->dmabuf)) - dma_resv_assert_held(attach->dmabuf->resv); - __unmap_dma_buf(attach, sg_table, direction);
if (dma_buf_is_dynamic(attach->dmabuf) &&
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Move dma-buf attachment mapping functions to the dynamic locking specification by asserting that the reservation lock is held.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Reviewed-by: Christian König christian.koenig@amd.com
drivers/dma-buf/dma-buf.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 073942bf5ae9..8e928fe6e8df 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1037,8 +1037,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, if (WARN_ON(!attach || !attach->dmabuf)) return ERR_PTR(-EINVAL);
- if (dma_buf_attachment_is_dynamic(attach))
dma_resv_assert_held(attach->dmabuf->resv);
- dma_resv_assert_held(attach->dmabuf->resv);
if (attach->sgt) { /* @@ -1053,7 +1052,6 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, } if (dma_buf_is_dynamic(attach->dmabuf)) {
dma_resv_assert_held(attach->dmabuf->resv);@@ -1142,15 +1140,11 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, if (WARN_ON(!attach || !attach->dmabuf || !sg_table)) return;
- if (dma_buf_attachment_is_dynamic(attach))
dma_resv_assert_held(attach->dmabuf->resv);
- dma_resv_assert_held(attach->dmabuf->resv);
if (attach->sgt == sg_table) return;
- if (dma_buf_is_dynamic(attach->dmabuf))
dma_resv_assert_held(attach->dmabuf->resv);- __unmap_dma_buf(attach, sg_table, direction);
if (dma_buf_is_dynamic(attach->dmabuf) &&
Move dma_buf_mmap() function to the dynamic locking specification by taking the reservation lock. Neither of the today's drivers take the reservation lock within the mmap() callback, hence it's safe to enforce the locking.
Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/dma-buf/dma-buf.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 8e928fe6e8df..d9130486cb8f 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1389,6 +1389,8 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_end_cpu_access, DMA_BUF); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { + int ret; + if (WARN_ON(!dmabuf || !vma)) return -EINVAL;
@@ -1409,7 +1411,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, vma_set_file(vma, dmabuf->file); vma->vm_pgoff = pgoff;
- return dmabuf->ops->mmap(dmabuf, vma); + dma_resv_lock(dmabuf->resv, NULL); + ret = dmabuf->ops->mmap(dmabuf, vma); + dma_resv_unlock(dmabuf->resv); + + return ret; } EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, DMA_BUF);
Add documentation for the dynamic locking convention. The documentation tells dma-buf API users when they should take the reservation lock and when not.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- Documentation/driver-api/dma-buf.rst | 6 +++ drivers/dma-buf/dma-buf.c | 64 ++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+)
diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index 36a76cbe9095..622b8156d212 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -119,6 +119,12 @@ DMA Buffer ioctls
.. kernel-doc:: include/uapi/linux/dma-buf.h
+DMA-BUF locking convention +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: drivers/dma-buf/dma-buf.c + :doc: locking convention + Kernel Functions and Structures Reference ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d9130486cb8f..97ce884fad76 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -794,6 +794,70 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, return sg_table; }
+/** + * DOC: locking convention + * + * In order to avoid deadlock situations between dma-buf exports and importers, + * all dma-buf API users must follow the common dma-buf locking convention. + * + * Convention for importers + * + * 1. Importers must hold the dma-buf reservation lock when calling these + * functions: + * + * - dma_buf_pin() + * - dma_buf_unpin() + * - dma_buf_map_attachment() + * - dma_buf_unmap_attachment() + * - dma_buf_vmap() + * - dma_buf_vunmap() + * + * 2. Importers must not hold the dma-buf reservation lock when calling these + * functions: + * + * - dma_buf_attach() + * - dma_buf_dynamic_attach() + * - dma_buf_detach() + * - dma_buf_export( + * - dma_buf_fd() + * - dma_buf_get() + * - dma_buf_put() + * - dma_buf_mmap() + * - dma_buf_begin_cpu_access() + * - dma_buf_end_cpu_access() + * - dma_buf_map_attachment_unlocked() + * - dma_buf_unmap_attachment_unlocked() + * - dma_buf_vmap_unlocked() + * - dma_buf_vunmap_unlocked() + * + * Convention for exporters + * + * 1. These &dma_buf_ops callbacks are invoked with unlocked dma-buf + * reservation and exporter can take the lock: + * + * - &dma_buf_ops.attach() + * - &dma_buf_ops.detach() + * - &dma_buf_ops.release() + * - &dma_buf_ops.begin_cpu_access() + * - &dma_buf_ops.end_cpu_access() + * + * 2. These &dma_buf_ops callbacks are invoked with locked dma-buf + * reservation and exporter can't take the lock: + * + * - &dma_buf_ops.pin() + * - &dma_buf_ops.unpin() + * - &dma_buf_ops.map_dma_buf() + * - &dma_buf_ops.unmap_dma_buf() + * - &dma_buf_ops.mmap() + * - &dma_buf_ops.vmap() + * - &dma_buf_ops.vunmap() + * + * 3. Exporters must hold the dma-buf reservation lock when calling these + * functions: + * + * - dma_buf_move_notify() + */ + /** * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list * @dmabuf: [in] buffer to attach device to.
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
Add documentation for the dynamic locking convention. The documentation tells dma-buf API users when they should take the reservation lock and when not.
Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Reviewed-by: Christian König christian.koenig@amd.com
Documentation/driver-api/dma-buf.rst | 6 +++ drivers/dma-buf/dma-buf.c | 64 ++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+)
diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index 36a76cbe9095..622b8156d212 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -119,6 +119,12 @@ DMA Buffer ioctls .. kernel-doc:: include/uapi/linux/dma-buf.h +DMA-BUF locking convention +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.. kernel-doc:: drivers/dma-buf/dma-buf.c
- :doc: locking convention
- Kernel Functions and Structures Reference
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d9130486cb8f..97ce884fad76 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -794,6 +794,70 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, return sg_table; } +/**
- DOC: locking convention
- In order to avoid deadlock situations between dma-buf exports and importers,
- all dma-buf API users must follow the common dma-buf locking convention.
- Convention for importers
- Importers must hold the dma-buf reservation lock when calling these
- functions:
- dma_buf_pin()
- dma_buf_unpin()
- dma_buf_map_attachment()
- dma_buf_unmap_attachment()
- dma_buf_vmap()
- dma_buf_vunmap()
- Importers must not hold the dma-buf reservation lock when calling these
- functions:
- dma_buf_attach()
- dma_buf_dynamic_attach()
- dma_buf_detach()
- dma_buf_export(
- dma_buf_fd()
- dma_buf_get()
- dma_buf_put()
- dma_buf_mmap()
- dma_buf_begin_cpu_access()
- dma_buf_end_cpu_access()
- dma_buf_map_attachment_unlocked()
- dma_buf_unmap_attachment_unlocked()
- dma_buf_vmap_unlocked()
- dma_buf_vunmap_unlocked()
- Convention for exporters
- These &dma_buf_ops callbacks are invoked with unlocked dma-buf
- reservation and exporter can take the lock:
- &dma_buf_ops.attach()
- &dma_buf_ops.detach()
- &dma_buf_ops.release()
- &dma_buf_ops.begin_cpu_access()
- &dma_buf_ops.end_cpu_access()
- These &dma_buf_ops callbacks are invoked with locked dma-buf
- reservation and exporter can't take the lock:
- &dma_buf_ops.pin()
- &dma_buf_ops.unpin()
- &dma_buf_ops.map_dma_buf()
- &dma_buf_ops.unmap_dma_buf()
- &dma_buf_ops.mmap()
- &dma_buf_ops.vmap()
- &dma_buf_ops.vunmap()
- Exporters must hold the dma-buf reservation lock when calling these
- functions:
- dma_buf_move_notify()- */
- /**
- dma_buf_dynamic_attach - Add the device to dma_buf's attachments list
- @dmabuf: [in] buffer to attach device to.
All drivers that use dma-bufs have been moved to the updated locking specification and now dma-buf reservation is guaranteed to be locked by importers during the mapping operations. There is no need to take the internal dma-buf lock anymore. Remove locking from the videobuf2 memory allocators.
Acked-by: Tomasz Figa tfiga@chromium.org Acked-by: Hans Verkuil hverkuil-cisco@xs4all.nl Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com --- drivers/media/common/videobuf2/videobuf2-dma-contig.c | 11 +---------- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 11 +---------- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 11 +---------- 3 files changed, 3 insertions(+), 30 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index 79f4d8301fbb..555bd40fa472 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -382,18 +382,12 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir) { struct vb2_dc_attachment *attach = db_attach->priv; - /* stealing dmabuf mutex to serialize map/unmap operations */ - struct mutex *lock = &db_attach->dmabuf->lock; struct sg_table *sgt;
- mutex_lock(lock); - sgt = &attach->sgt; /* return previously mapped sg table */ - if (attach->dma_dir == dma_dir) { - mutex_unlock(lock); + if (attach->dma_dir == dma_dir) return sgt; - }
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { @@ -409,14 +403,11 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, DMA_ATTR_SKIP_CPU_SYNC)) { pr_err("failed to map scatterlist\n"); - mutex_unlock(lock); return ERR_PTR(-EIO); }
attach->dma_dir = dma_dir;
- mutex_unlock(lock); - return sgt; }
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index 36ecdea8d707..36981a5b5c53 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -424,18 +424,12 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map( struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir) { struct vb2_dma_sg_attachment *attach = db_attach->priv; - /* stealing dmabuf mutex to serialize map/unmap operations */ - struct mutex *lock = &db_attach->dmabuf->lock; struct sg_table *sgt;
- mutex_lock(lock); - sgt = &attach->sgt; /* return previously mapped sg table */ - if (attach->dma_dir == dma_dir) { - mutex_unlock(lock); + if (attach->dma_dir == dma_dir) return sgt; - }
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { @@ -446,14 +440,11 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map( /* mapping to the client with new direction */ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n"); - mutex_unlock(lock); return ERR_PTR(-EIO); }
attach->dma_dir = dma_dir;
- mutex_unlock(lock); - return sgt; }
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index 7831bf545874..41db707e43a4 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -267,18 +267,12 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map( struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir) { struct vb2_vmalloc_attachment *attach = db_attach->priv; - /* stealing dmabuf mutex to serialize map/unmap operations */ - struct mutex *lock = &db_attach->dmabuf->lock; struct sg_table *sgt;
- mutex_lock(lock); - sgt = &attach->sgt; /* return previously mapped sg table */ - if (attach->dma_dir == dma_dir) { - mutex_unlock(lock); + if (attach->dma_dir == dma_dir) return sgt; - }
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { @@ -289,14 +283,11 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map( /* mapping to the client with new direction */ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n"); - mutex_unlock(lock); return ERR_PTR(-EIO); }
attach->dma_dir = dma_dir;
- mutex_unlock(lock); - return sgt; }
Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
All drivers that use dma-bufs have been moved to the updated locking specification and now dma-buf reservation is guaranteed to be locked by importers during the mapping operations. There is no need to take the internal dma-buf lock anymore. Remove locking from the videobuf2 memory allocators.
Acked-by: Tomasz Figa tfiga@chromium.org Acked-by: Hans Verkuil hverkuil-cisco@xs4all.nl Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.com
Acked-by: Christian König christian.koenig@amd.com
drivers/media/common/videobuf2/videobuf2-dma-contig.c | 11 +---------- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 11 +---------- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 11 +---------- 3 files changed, 3 insertions(+), 30 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index 79f4d8301fbb..555bd40fa472 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -382,18 +382,12 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir) { struct vb2_dc_attachment *attach = db_attach->priv;
- /* stealing dmabuf mutex to serialize map/unmap operations */
- struct mutex *lock = &db_attach->dmabuf->lock; struct sg_table *sgt;
- mutex_lock(lock);
- sgt = &attach->sgt; /* return previously mapped sg table */
- if (attach->dma_dir == dma_dir) {
mutex_unlock(lock);
- if (attach->dma_dir == dma_dir) return sgt;
- }
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { @@ -409,14 +403,11 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, DMA_ATTR_SKIP_CPU_SYNC)) { pr_err("failed to map scatterlist\n");
mutex_unlock(lock);attach->dma_dir = dma_dir;
- mutex_unlock(lock);
- return sgt; }
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index 36ecdea8d707..36981a5b5c53 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -424,18 +424,12 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map( struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir) { struct vb2_dma_sg_attachment *attach = db_attach->priv;
- /* stealing dmabuf mutex to serialize map/unmap operations */
- struct mutex *lock = &db_attach->dmabuf->lock; struct sg_table *sgt;
- mutex_lock(lock);
- sgt = &attach->sgt; /* return previously mapped sg table */
- if (attach->dma_dir == dma_dir) {
mutex_unlock(lock);
- if (attach->dma_dir == dma_dir) return sgt;
- }
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { @@ -446,14 +440,11 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map( /* mapping to the client with new direction */ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n");
mutex_unlock(lock);attach->dma_dir = dma_dir;
- mutex_unlock(lock);
- return sgt; }
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index 7831bf545874..41db707e43a4 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -267,18 +267,12 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map( struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir) { struct vb2_vmalloc_attachment *attach = db_attach->priv;
- /* stealing dmabuf mutex to serialize map/unmap operations */
- struct mutex *lock = &db_attach->dmabuf->lock; struct sg_table *sgt;
- mutex_lock(lock);
- sgt = &attach->sgt; /* return previously mapped sg table */
- if (attach->dma_dir == dma_dir) {
mutex_unlock(lock);
- if (attach->dma_dir == dma_dir) return sgt;
- }
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { @@ -289,14 +283,11 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map( /* mapping to the client with new direction */ if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n");
mutex_unlock(lock);attach->dma_dir = dma_dir;
- mutex_unlock(lock);
- return sgt; }
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.
Acked-by: Christian König christian.koenig@amd.com Signed-off-by: Dmitry Osipenko dmitry.osipenko@collabora.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 97ce884fad76..772fdd9eeed8 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); @@ -1502,7 +1501,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, 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);
@@ -1514,28 +1513,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);
@@ -1577,13 +1573,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 f11b5bbc2f37..6fa8d4e29719 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: *
Am 31.08.22 um 17:37 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.
Acked-by: Christian König christian.koenig@amd.com 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 97ce884fad76..772fdd9eeed8 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); @@ -1502,7 +1501,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, 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); @@ -1514,28 +1513,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);
@@ -1577,13 +1573,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 f11b5bbc2f37..6fa8d4e29719 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:
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(-)
Hello Christian,
On 9/1/22 10:49, Christian König wrote:
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.
Thank you very much for the review!
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.
At least the most popular drivers should be okay. If anyone will find issue introduced by this series, then indeed shouldn't be a problem to fix it later on.
Do you have commit rights to drm-misc-next or should I push it?
I got the drm-misc commit right two weeks ago, haven't pushed anything there yet. Please let me try to do the push. I'll let you know if any kind of help will be needed from you.
I'll wait for more acks/r-bs and then do the push.
linaro-mm-sig@lists.linaro.org
-
cco -
Christian König -
Christian König -
Dmitry Osipenko -
Dmitry Osipenko -
Juergen Gross -
Ruhl, Michael J -
Srinivas Kandagatla