Add dmabuf read/write via io_uring
Subsystems (9)
v6 cover Add dmabuf read/write via io_uring
The patch set allows to register a dmabuf to an io_uring instance for a specified file and use it with io_uring read / write requests. The infrastructure is not tied to io_uring and there could be more users in the future. A similar idea was attempted some years ago by Keith [1], from where I borrowed a number of changes. Later it was brough back up to life by Tushar and Vishal.
It's an opt-in feature for files, and they need to implement a new file operation to use it. Only NVMe block devices are supported in this series. The user API is built on top of io_uring's "registered buffers", where a dmabuf is registered in a special way, but after it can be used as any other "registered buffer" with IORING_OP_{READ,WRITE}_FIXED requests. It's created via a new file operation and the resulted map is then passed through the I/O stack in a new iterator type. There is some additional infrastructure to glue it together, count requests, manage lifetime and implement invalidation.
Tushar, William, Phil did a lot of testing and experimentation on various devices with previous versions of the patch set, and I've received lots of help from Anuj, Kanchan and Nitesh with investigations, testing, and patching. Earlier benchmarks by Anuj for IOMMU optimisations with udmabuf showed:
STRICT: before = 570 KIOPS, after = 5.01 MIOPS
LAZY: before = 1.93 MIOPS, after = 5.01 MIOPS
PASSTHROUGH: before = 5.01 MIOPS, after = 5.01 MIOPS
# Patch set structure:
- Patches 1-2 introduce internal API and infrastructure mediating io_uring and target subsystem / devices
- Patches 3-5: block layer support + prep patches
- Patches 6-8 implement NVMe support
- Patches 9-13 add io_uring support and uapi.
There are some liburing tests that can serve as an example:
git: https://github.com/isilence/liburing.git rw-dmabuf-tests-v5
url: https://github.com/isilence/liburing/tree/rw-dmabuf-tests-v5
The patches are based on Jens' for-next. Also available as a branch:
git: https://github.com/isilence/linux.git rw-dmabuf-v6
url: https://github.com/isilence/linux/tree/rw-dmabuf-v6
[1] https://lore.kernel.org/io-uring/[email protected]/
v6: - remove fences and wait on invalidation synchronously
- report all map requests are detached earlier, outside of wq
- handle nvme removal
- gate nvme_free_descriptors() on iod->nr_descriptors
- fix error handling in nvme_rq_setup_dmabuf_map()
- fix iov_iter_alignment()
- relax the 1G io_uring registered dma-buf limit
- fix type overflow issue in bio split
- allocate struct dma_buf_io_ctx inside the dma-buf-io code
- harden block checks against dma-buf + buffered io
v5: - Reject dma-buf with buffered IO for raw bdev
- Add lim->max_segments bio splitting
- Add bio_iov_iter_set() helper
- Fix io_uring uapi validation
- Other minor changes
NVMe:
- Rename nvme_pci_sgl_set_data() to nvme_pci_dma_iter_set_sgl(), split into its own prep patch.
- Convert segment walk loops to do-while.
- Drop adjacent segment coalescing logic.
- Drop first_dma/first_len from nvme_pci_dmabuf_sgl_nents()
- Remove entries > NVME_MAX_SEGS bailout; moving to the block layer.
- Factor SGL vs PRP decision making into a helper.
v4: - https://lore.kernel.org/all/[email protected]/
- Add sgl support from Anuj
- Move it under drivers/dma-buf/ and rename
- Fix mis-sized allocations
- Fix io_uring re-import mishandling
- Drop map before io_uring "task work"
- Move blk-mq callback to block_device_operations
- Convert bio flag to REQ_OP*
- Other small changes
v3: https://lore.kernel.org/io-uring/[email protected]/
- Rework io_uring registration
- Move token/map infrastructure code out of blk-mq
- Simplify callbacks: remove a separate blk-mq table, which was mostly just forwarding calls (to nvme).
- Don't skip dma sync depending on request direction
- Fix a couple of hangs
- Rename s/dma/dmabuf/
- Other small changes
v2: - Don't pass raw dma addresses, wrap it into a driver specific object
- Split into two objects: token and map
- Implement move_notify
Anuj Gupta (2):
nvme-pci: rename nvme_pci_sgl_set_data to nvme_pci_dma_iter_set_sgl
nvme-pci: add SGL support for the dmabuf path
Pavel Begunkov (11):
dma-buf: introduce initial file I/O infrastructure
iov_iter: add iterator type for dmabuf maps
block: always adjust bi_offset on bio_advance_iter
block: introduce dma map backed bio type
block: add dma-buf support for raw bdev
nvme-pci: implement dma-buf backed requests io_uring/rsrc: introduce buf registration structure io_uring/rsrc: extend buffer update io_uring/rsrc: add uncloneable regbuf flag io_uring/rsrc: add regbuf import flags io_uring/rsrc: add dmabuf backed registered buffers
block/bio.c | 15 +- block/blk-merge.c | 45 +++ block/fops.c | 24 +- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/dma-buf-io.c | 219 +++++++++++++++ drivers/md/dm-io-rewind.c | 6 +- drivers/nvme/host/core.c | 12 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/pci.c | 484 ++++++++++++++++++++++++++++++++- include/linux/bio.h | 21 +- include/linux/blk-mq.h | 7 + include/linux/blk_types.h | 14 +- include/linux/blkdev.h | 2 + include/linux/bvec.h | 3 +- include/linux/dma-buf-io.h | 113 ++++++++ include/linux/fs.h | 2 + include/linux/io_uring_types.h | 5 + include/linux/uio.h | 11 + include/uapi/linux/io_uring.h | 31 ++- io_uring/io_uring.c | 3 +- io_uring/rsrc.c | 274 ++++++++++++++++--- io_uring/rsrc.h | 45 ++- io_uring/rw.c | 6 +- lib/iov_iter.c | 29 +- 24 files changed, 1295 insertions(+), 80 deletions(-) create mode 100644 drivers/dma-buf/dma-buf-io.c create mode 100644 include/linux/dma-buf-io.h
Signature · 1 line
2.54.0
PATCH v6 1/13 dma-buf: introduce initial file I/O infrastructure
The goal is to be able to natively use dma-buf in the read-write / IO path. This patch adds basic building blocks serving as a glue and API between drivers and upper layer subsystems providing the uAPI. Later patches implement it for NVMe raw block devices and expose it to the user space via io_uring.
There are two main objects. struct dma_buf_io_ctx and struct dma_buf_io_map. The ctx is used during initial registration and serves as an interface between the upper layer user like io_uring and to the importer subsystem / driver. The map represents the actual dma map established for the target device[s] with dma_buf_map_attachment() and stored in a device specific format. The context is created via a new file operation ->init_dma_buf_io_ctx.
The ctx-map separation exists to support map invalidation (see dma_buf_io_invalidate_mappings()). A ctx can create multiple maps during its lifetime, but there can only be no more than one (active) map attached to it. Invalidation drops the active map if present, and the next map will only be attempted to be created once there is a new request that wants to use the dma-buf IO ctx.
The primary task of the dma_buf_io_map object is to count requests using it and to wait for their completion when we want to destroy the DMA map.
[un]mapping and any work with dma addresses is delegated to the importer driver via an ops table stored in the ctx, see struct dma_buf_io_ops. Only the target driver / subsystem knows about devices it wants to use the dma-buf with, especially in case of multi-device filesystems or stacking in the future.
--- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/dma-buf-io.c | 219 +++++++++++++++++++++++++++++++++++ include/linux/dma-buf-io.h | 113 ++++++++++++++++++ include/linux/fs.h | 2 + 4 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/dma-buf-io.c create mode 100644 include/linux/dma-buf-io.h
drivers/dma-buf/Makefile +1 -1
@@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ - dma-fence-unwrap.o dma-resv.o dma-buf-mapping.o + dma-fence-unwrap.o dma-resv.o dma-buf-mapping.o dma-buf-io.o obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o obj-$(CONFIG_DMABUF_HEAPS) += heaps/ obj-$(CONFIG_SYNC_FILE) += sync_file.o
drivers/dma-buf/dma-buf-io.c new file +219 -0
219 lines not on this page · raw message
include/linux/dma-buf-io.h new file +113 -0
113 lines not on this page · raw message
include/linux/fs.h +2 -0
@@ -1914,6 +1914,7 @@ struct dir_context {#define COPY_FILE_SPLICE (1 << 0)struct io_uring_cmd ; +struct dma_buf_io_ctx ;struct offset_ctx ;struct file_operations {
@@ -1960,6 +1961,7 @@ struct file_operations {int (*uring_cmd_iopoll)(struct io_uring_cmd *,struct io_comp_batch *,unsigned int poll_flags);int (*mmap_prepare)(struct vm_area_desc *); +int (*init_dma_buf_io_ctx)(struct file *,struct dma_buf_io_ctx *); } __randomize_layout;/* Supports async buffered reads */
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider: - [Critical] dma-buf: TOCTOU race allows a stale map to escape concurrent invalidation - [High] dma-buf…
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] dma-buf: TOCTOU race allows a stale map to escape concurrent invalidation
- [High] dma-buf: Use-After-Free of ctx in waitqueue due to race condition
- [High] dma-buf: GFP_KERNEL allocation performed while holding dmabuf->resv ww_mutex
- [Medium] dma-buf: Hardcoded kfree(map) restricts driver structure embedding
- [Medium] dma-buf: Memory leak of ctx and dmabuf reference on incomplete dev_ops error path
- [Medium] dma-buf: Memory leak of map structure and DMA mappings on min_seg_shift validation error --
commit b9251fd88ee105b166b558434c10ce6e3b106691
Author: Pavel Begunkov <[email protected]>
Subject: dma-buf: introduce initial file I/O infrastructure
This commit adds basic building blocks serving as a glue and API between drivers and upper layer subsystems providing the uAPI. It introduces struct dma_buf_io_ctx and struct dma_buf_io_map, allowing dma-buf to be natively used in the read-write / IO path.
Quoted 5 lines
drivers/dma-buf/dma-buf-io.c new file +0
-0
[ ... ]
drivers/dma-buf/dma-buf-io.c static void dma_buf_io_map_release_work(struct work_struct *work) · line 33
Quoted 13 lines
drivers/dma-buf/dma-buf-io.c new file +0
-0@@ -22,17 +22,17 @@ static void dma_buf_io_map_release_work(struct work_struct *work) { struct dma_buf_io_map *map = container_of(work, struct dma_buf_io_map, release_work);struct dma_buf_io_ctx *ctx = map->ctx;struct dma_buf *dmabuf = ctx->dmabuf;dma_resv_lock (dmabuf->resv,NULL ); ctx->dev_ops->unmap (ctx, map);dma_resv_unlock (dmabuf->resv);percpu_ref_exit (&map->refs);kfree (map);
[Severity: Medium]
Since drivers allocate this map object (often embedding it in a larger structure), does this unconditional kfree(map) restrict where drivers can embed the structure?
If a driver needs to use vmalloc or embed the map at a non-zero offset, this will cause a panic. Could a .free callback be provided for safe lifecycle management?
drivers/dma-buf/dma-buf-io.c static void dma_buf_io_map_release_work(struct work_struct *work) · line 35
Quoted 3 lines
drivers/dma-buf/dma-buf-io.c new file +0
-0atomic_dec (&ctx->all_maps);wake_up (&ctx->maps_wq);
[Severity: High]
Can this sequence lead to a use-after-free on ctx?
If atomic_dec(&ctx->all_maps) drops the count to 0, a concurrent thread blocked in dma_buf_io_wait_maps() can wake up, return, and free the ctx via dma_buf_io_ctx_release():
Thread 1 (waiter):dma_buf_io_ctx_release()
dma_buf_io_wait_maps()
wait_event() unblocks since atomic_read(&ctx->all_maps) == 0.
dma_buf_io_put_ctx() is called and frees ctx.
Thread 2 (worker here):
Proceeds to call wake_up(&ctx->maps_wq) on the now-freed ctx memory.
Quoted 1 line
drivers/dma-buf/dma-buf-io.c new file +0
-0}
[ ... ]
drivers/dma-buf/dma-buf-io.c line 79
Quoted 13 lines
drivers/dma-buf/dma-buf-io.c new file +13
-0+int dma_buf_io_init_map (struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map, +struct sg_table *sgt) +{ +unsigned seg_shift = ~0U ; +struct scatterlist *sg; +unsigned long tmp; +int ret; + +for_each_sgtable_dma_sg (sgt, sg, tmp) + seg_shift =min (seg_shift,__ffs (sg_dma_len (sg))); + + ret =percpu_ref_init (&map->refs, dma_buf_io_map_refs_release,0 , +GFP_KERNEL );
[Severity: High]
Does this GFP_KERNEL allocation violate DMA-buf locking rules?
dma_buf_io_create_map() calls ctx->dev_ops->map(ctx) while holding the dmabuf->resv ww_mutex. If the driver then calls dma_buf_io_init_map() under this lock, the GFP_KERNEL allocation could deadlock if memory reclaim attempts to acquire dma_resv locks.
Quoted 37 lines
drivers/dma-buf/dma-buf-io.c new file +37
-0+if (ret) +return ret; + map->min_seg_shift = seg_shift; + map->ctx = ctx; +return 0 ; +} +EXPORT_SYMBOL_NS_GPL (dma_buf_io_init_map,"DMA_BUF" ); + +struct dma_buf_io_map *dma_buf_io_create_map (struct dma_buf_io_ctx *ctx) +{ +struct dma_buf *dmabuf = ctx->dmabuf; +struct dma_buf_io_map *map; +long ret; + +guard (mutex)(&ctx->map_create_mutex); + + scoped_guard(mutex, &ctx->map_mutex) { +if (ctx->maps_killed) +return ERR_PTR (-ENOENT ); +/* recheck under the lock in case it has already been re-created */ + map =__dma_buf_io_get_map (ctx); +if (map) +return map; + } + +dma_buf_io_wait_active_maps (ctx); + + ret =dma_resv_lock_interruptible (dmabuf->resv,NULL ); +if (ret) +return ERR_PTR (ret); + + ret =dma_resv_wait_timeout (dmabuf->resv,DMA_RESV_USAGE_KERNEL , + true,MAX_SCHEDULE_TIMEOUT ); +if (ret <=0 ) { +if (!ret) + ret = -EAGAIN ; +dma_resv_unlock (dmabuf->resv);
drivers/dma-buf/dma-buf-io.c line 126
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+return ERR_PTR (ret); + } + + map = ctx->dev_ops->map (ctx); +dma_resv_unlock (dmabuf->resv); + +if (IS_ERR (map)) +return map; +if (WARN_ON_ONCE (!map->min_seg_shift)) +return ERR_PTR (-EFAULT );
[Severity: Medium]
If this validation fails and we return -EFAULT, do we leak the map structure and the underlying device DMA mappings?
It looks like we are bypassing ctx->dev_ops->unmap() and the corresponding map structure freeing here.
[Severity: Critical]
Does this sequence create a TOCTOU race with dma_buf_io_invalidate_mappings()?
The map is created under dmabuf->resv lock, but we unlock it before updating ctx->map and ctx->active_maps.
If an exporter triggers an invalidation:
1. dma_buf_io_create_map() calls ctx->dev_ops->map(ctx).
2. It then calls dma_resv_unlock(dmabuf->resv).
3. The exporter acquires the dmabuf->resv lock and calls dma_buf_io_invalidate_mappings().
4. dma_buf_io_kill_maps() and dma_buf_io_wait_active_maps() observe no active maps and finish the invalidation.
5. dma_buf_io_create_map() resumes, increments active_maps, and publishes the now-stale map to ctx->map.
This allows the stale physical memory mapping to escape to the caller.
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ +atomic_inc (&ctx->active_maps); +atomic_inc (&ctx->all_maps); +/* get a reference for the caller */ +percpu_ref_get (&map->refs); + + scoped_guard(mutex, &ctx->map_mutex) +rcu_assign_pointer (ctx->map, map); +return map; +}
[ ... ]
Quoted 27 lines
drivers/dma-buf/dma-buf-io.c new file +27
-0+int dma_buf_io_ctx_create (struct file *file, +struct dma_buf *dmabuf, +enum dma_data_direction dir, +struct dma_buf_io_ctx **out_ctx) +{ +struct dma_buf_io_ctx *ctx; +int ret; + +if (!file->f_op->init_dma_buf_io_ctx) +return -EOPNOTSUPP ; + + ctx =kmalloc_obj (*ctx); +if (!ctx) +return -ENOMEM ; + +memset (ctx,0 ,sizeof (*ctx)); + ctx->dir = dir; + ctx->dmabuf = dmabuf; +get_dma_buf (dmabuf); +mutex_init (&ctx->map_mutex); +mutex_init (&ctx->map_create_mutex); +atomic_set (&ctx->active_maps,0 ); +atomic_set (&ctx->all_maps,0 ); +init_waitqueue_head (&ctx->maps_wq); + + ret = file->f_op->init_dma_buf_io_ctx (file, ctx); +if (ret) {
drivers/dma-buf/dma-buf-io.c line 206
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+kfree (ctx); +dma_buf_put (dmabuf); +return ret; + } + +if (WARN_ON_ONCE (!ctx->dev_ops || + !ctx->dev_ops->map || + !ctx->dev_ops->unmap || + !ctx->dev_ops->release)) +return -EINVAL ;
[Severity: Medium]
If this check fails, does it leak the allocated ctx and the dmabuf reference?
The previous error path handles cleanup via kfree(ctx) and dma_buf_put(dmabuf), but this one bails out without cleaning up.
Quoted 4 lines
drivers/dma-buf/dma-buf-io.c new file +4
-0+ + *out_ctx = ctx; +return 0 ; +}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
On 9/21/26 14:53, [email protected] wrote:
...>> }
Quoted 18 lines
[ ... ]
drivers/dma-buf/dma-buf-io.c line 79
Quoted 13 lines
drivers/dma-buf/dma-buf-io.c new file +13
-0+int dma_buf_io_init_map (struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map, +struct sg_table *sgt) +{ +unsigned seg_shift = ~0U ; +struct scatterlist *sg; +unsigned long tmp; +int ret; + +for_each_sgtable_dma_sg (sgt, sg, tmp) + seg_shift =min (seg_shift,__ffs (sg_dma_len (sg))); + + ret =percpu_ref_init (&map->refs, dma_buf_io_map_refs_release,0 , +GFP_KERNEL );[Severity: High]
Does thisGFP_KERNELallocation violate DMA-buf locking rules?
Quoted 4 lines
dma_buf_io_create_map()callsctx->dev_ops->map(ctx)while holding thedmabuf->resvww_mutex. If the driver then callsdma_buf_io_init_map()under this lock, theGFP_KERNELallocation could deadlock if memory reclaim attempts to acquiredma_resvlocks.
Most of these look trivial, going to fix them. This one, however, doesn't sound true AFAIU, I haven't heard about any blank ban on GFP_KERNEL allocation under resv lock.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Quoted 20 lines · Pavel Begunkov
On 9/21/26 14:53, [email protected] wrote:
...>> }Quoted 18 lines
[ ... ]
drivers/dma-buf/dma-buf-io.c line 79
Quoted 13 lines
drivers/dma-buf/dma-buf-io.c new file +13
-0+int dma_buf_io_init_map (struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map, +struct sg_table *sgt) +{ +unsigned seg_shift = ~0U ; +struct scatterlist *sg; +unsigned long tmp; +int ret; + +for_each_sgtable_dma_sg (sgt, sg, tmp) + seg_shift =min (seg_shift,__ffs (sg_dma_len (sg))); + + ret =percpu_ref_init (&map->refs, dma_buf_io_map_refs_release,0 , +GFP_KERNEL );[Severity: High]
Does thisGFP_KERNELallocation violate DMA-buf locking rules?
Quoted 9 lines
Quoted 4 lines
dma_buf_io_create_map()callsctx->dev_ops->map(ctx)while holding thedmabuf->resvww_mutex. If the driver then callsdma_buf_io_init_map()under this lock, theGFP_KERNELallocation could deadlock if memory reclaim attempts to acquiredma_resvlocks.Most of these look trivial, going to fix them. This one, however, doesn't sound true AFAIU, I haven't heard about any blank ban on
GFP_KERNELallocation under resv lock.
I can confirm this definitely not correct - see dma_resv_lockdep it acquires dma_resv then fs_reclaim_acquire(GFP_KERNEL). Shrinker enter direct reclaim and try to take dma-resv locks, but not block on the lock.
Matt
Quoted 3 lines
Signature · 1 line
Pavel Begunkov
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Quoted 31 lines · Matthew Brost
Quoted 20 lines · Pavel Begunkov
On 9/21/26 14:53, [email protected] wrote:
...>> }Quoted 18 lines
[ ... ]
drivers/dma-buf/dma-buf-io.c line 79
Quoted 13 lines
drivers/dma-buf/dma-buf-io.c new file +13
-0+int dma_buf_io_init_map (struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map, +struct sg_table *sgt) +{ +unsigned seg_shift = ~0U ; +struct scatterlist *sg; +unsigned long tmp; +int ret; + +for_each_sgtable_dma_sg (sgt, sg, tmp) + seg_shift =min (seg_shift,__ffs (sg_dma_len (sg))); + + ret =percpu_ref_init (&map->refs, dma_buf_io_map_refs_release,0 , +GFP_KERNEL );[Severity: High]
Does thisGFP_KERNELallocation violate DMA-buf locking rules?Quoted 9 lines
Quoted 4 lines
dma_buf_io_create_map()callsctx->dev_ops->map(ctx)while holding thedmabuf->resvww_mutex. If the driver then callsdma_buf_io_init_map()under this lock, theGFP_KERNELallocation could deadlock if memory reclaim attempts to acquiredma_resvlocks.Most of these look trivial, going to fix them. This one, however, doesn't sound true AFAIU, I haven't heard about any blank ban on
GFP_KERNELallocation under resv lock.
Quoted 4 lines
I can confirm this definitely not correct - see
dma_resv_lockdepit acquiresdma_resvthenfs_reclaim_acquire(GFP_KERNEL). Shrinker enter direct reclaim and try to take dma-resv locks, but not block on the lock.
Just to double check, I believe by "this" you mean that the sashiko report is incorrect. I'm going to leave it as is, but let me know if I'm wrong.
Signature · 1 line
Pavel Begunkov
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
In reply to Pavel BegunkovQuoted 169 lines · Pavel Begunkov
The goal is to be able to natively use dma-buf in the read-write / IO path. This patch adds basic building blocks serving as a glue and API between drivers and upper layer subsystems providing the uAPI. Later patches implement it for NVMe raw block devices and expose it to the user space via
io_uring.There are two main objects. struct
dma_buf_io_ctxand structdma_buf_io_map. The ctx is used during initial registration and serves as an interface between the upper layer user likeio_uringand to the importer subsystem / driver. The map represents the actual dma map established for the target device[s] withdma_buf_map_attachment()and stored in a device specific format. The context is created via a new file operation ->init_dma_buf_io_ctx.The ctx-map separation exists to support map invalidation (see
dma_buf_io_invalidate_mappings()). A ctx can create multiple maps during its lifetime, but there can only be no more than one (active) map attached to it. Invalidation drops the active map if present, and the next map will only be attempted to be created once there is a new request that wants to use the dma-buf IO ctx.The primary task of the
dma_buf_io_mapobject is to count requests using it and to wait for their completion when we want to destroy the DMA map.[un]mapping and any work with dma addresses is delegated to the importer driver via an ops table stored in the ctx, see struct
dma_buf_io_ops. Only the target driver / subsystem knows about devices it wants to use the dma-buf with, especially in case of multi-device filesystems or stacking in the future.Signed-off-by Pavel Begunkov <[email protected]>--- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/dma-buf-io.c | 219 +++++++++++++++++++++++++++++++++++ include/linux/dma-buf-io.h | 113 ++++++++++++++++++ include/linux/fs.h | 2 + 4 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/dma-buf-io.c create mode 100644 include/linux/dma-buf-io.hdrivers/dma-buf/Makefile +1
-1@@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ - dma-fence-unwrap.o dma-resv.o dma-buf-mapping.o + dma-fence-unwrap.o dma-resv.o dma-buf-mapping.o dma-buf-io.o obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o obj-$(CONFIG_DMABUF_HEAPS) += heaps/ obj-$(CONFIG_SYNC_FILE) += sync_file.odrivers/dma-buf/dma-buf-io.c new file +110
-0@@ -0,0 +1,219 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Common infrastructure for supporing dma-buf in the I/O path. +* +* Copyright (C) 2026 Pavel Begunkov <[email protected]> +*/ +#include <linux/dma-buf-io.h> +#include <linux/dma-resv.h> + +static void dma_buf_io_put_ctx (struct dma_buf_io_ctx *ctx) +{ +might_sleep (); + +if (WARN_ON_ONCE (rcu_dereference_protected (ctx->map, true))) +return ; + + ctx->dev_ops->release (ctx); + +dma_buf_put (ctx->dmabuf); +mutex_destroy (&ctx->map_mutex); +mutex_destroy (&ctx->map_create_mutex); +kfree (ctx); +} + +static void dma_buf_io_map_release_work (struct work_struct *work) +{ + struct dma_buf_io_map *map = container_of(work, struct dma_buf_io_map, + release_work); +struct dma_buf_io_ctx *ctx = map->ctx; +struct dma_buf *dmabuf = ctx->dmabuf; + +dma_resv_lock (dmabuf->resv,NULL ); + ctx->dev_ops->unmap (ctx, map); +dma_resv_unlock (dmabuf->resv); + +percpu_ref_exit (&map->refs); +kfree (map); + +atomic_dec (&ctx->all_maps); +wake_up (&ctx->maps_wq); +} + +static void dma_buf_io_map_refs_release (struct percpu_ref *ref) +{ + struct dma_buf_io_map *map = container_of(ref, struct dma_buf_io_map, refs); +struct dma_buf_io_ctx *ctx = map->ctx; + +/* There are no more requests using the map. */ +atomic_dec (&ctx->active_maps); +wake_up (&ctx->maps_wq); + +/* might sleep, use a worker */ +INIT_WORK (&map->release_work, dma_buf_io_map_release_work); +queue_work (system_percpu_wq, &map->release_work); +} + +static void dma_buf_io_wait_active_maps (struct dma_buf_io_ctx *ctx) +{ +wait_event (ctx->maps_wq,atomic_read (&ctx->active_maps) ==0 ); +} + +static void dma_buf_io_wait_maps (struct dma_buf_io_ctx *ctx) +{ +wait_event (ctx->maps_wq,atomic_read (&ctx->all_maps) ==0 ); +} + +int dma_buf_io_init_map (struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map, +struct sg_table *sgt) +{ +unsigned seg_shift = ~0U ; +struct scatterlist *sg; +unsigned long tmp; +int ret; + +for_each_sgtable_dma_sg (sgt, sg, tmp) + seg_shift =min (seg_shift,__ffs (sg_dma_len (sg))); + + ret =percpu_ref_init (&map->refs, dma_buf_io_map_refs_release,0 , +GFP_KERNEL ); +if (ret) +return ret; + map->min_seg_shift = seg_shift; + map->ctx = ctx; +return 0 ; +} +EXPORT_SYMBOL_NS_GPL (dma_buf_io_init_map,"DMA_BUF" ); + +struct dma_buf_io_map *dma_buf_io_create_map (struct dma_buf_io_ctx *ctx) +{ +struct dma_buf *dmabuf = ctx->dmabuf; +struct dma_buf_io_map *map; +long ret; + +guard (mutex)(&ctx->map_create_mutex); + + scoped_guard(mutex, &ctx->map_mutex) { +if (ctx->maps_killed) +return ERR_PTR (-ENOENT ); +/* recheck under the lock in case it has already been re-created */ + map =__dma_buf_io_get_map (ctx); +if (map) +return map; + } + +dma_buf_io_wait_active_maps (ctx); + + ret =dma_resv_lock_interruptible (dmabuf->resv,NULL ); +if (ret) +return ERR_PTR (ret); +
drivers/dma-buf/dma-buf-io.c line 120
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ ret =dma_resv_wait_timeout (dmabuf->resv,DMA_RESV_USAGE_KERNEL , + true,MAX_SCHEDULE_TIMEOUT ); +if (ret <=0 ) { +if (!ret) + ret = -EAGAIN ; +dma_resv_unlock (dmabuf->resv); +return ERR_PTR (ret); + } + + map = ctx->dev_ops->map (ctx);
I'm playing around this code now.
I think you need the dma_resv_wait_timeout after the 'map'?
If a device doesn't support p2p ->map() will typically trigger an async migrate to system memory and data will be moving but the map is valid - Xe 100% does this, I checked AMDGPU and fairly confident it has the same async behavior.
Matt
Quoted 241 lines
drivers/dma-buf/dma-buf-io.c new file +99
-0+dma_resv_unlock (dmabuf->resv); + +if (IS_ERR (map)) +return map; +if (WARN_ON_ONCE (!map->min_seg_shift)) +return ERR_PTR (-EFAULT ); + +atomic_inc (&ctx->active_maps); +atomic_inc (&ctx->all_maps); +/* get a reference for the caller */ +percpu_ref_get (&map->refs); + + scoped_guard(mutex, &ctx->map_mutex) +rcu_assign_pointer (ctx->map, map); +return map; +} + +static void dma_buf_io_kill_maps (struct dma_buf_io_ctx *ctx,bool final) +{ +struct dma_buf_io_map *map; + + scoped_guard(mutex, &ctx->map_mutex) { +if (final) + ctx->maps_killed = true; + + map =rcu_dereference_protected (ctx->map, +lockdep_is_held (&ctx->map_mutex)); +if (!map) +return ; +rcu_assign_pointer (ctx->map,NULL ); +percpu_ref_kill (&map->refs); + } +} + +void dma_buf_io_invalidate_mappings (struct dma_buf_io_ctx *ctx) +{ +dma_buf_io_kill_maps (ctx, false); +dma_buf_io_wait_active_maps (ctx); +} +EXPORT_SYMBOL_NS_GPL (dma_buf_io_invalidate_mappings,"DMA_BUF" ); + +void dma_buf_io_ctx_release (struct dma_buf_io_ctx *ctx) +{ +/* Remove and wait for the last map, there should be no new ones. */ +dma_buf_io_kill_maps (ctx, true); +dma_buf_io_wait_maps (ctx); +dma_buf_io_put_ctx (ctx); +} + +int dma_buf_io_ctx_create (struct file *file, +struct dma_buf *dmabuf, +enum dma_data_direction dir, +struct dma_buf_io_ctx **out_ctx) +{ +struct dma_buf_io_ctx *ctx; +int ret; + +if (!file->f_op->init_dma_buf_io_ctx) +return -EOPNOTSUPP ; + + ctx =kmalloc_obj (*ctx); +if (!ctx) +return -ENOMEM ; + +memset (ctx,0 ,sizeof (*ctx)); + ctx->dir = dir; + ctx->dmabuf = dmabuf; +get_dma_buf (dmabuf); +mutex_init (&ctx->map_mutex); +mutex_init (&ctx->map_create_mutex); +atomic_set (&ctx->active_maps,0 ); +atomic_set (&ctx->all_maps,0 ); +init_waitqueue_head (&ctx->maps_wq); + + ret = file->f_op->init_dma_buf_io_ctx (file, ctx); +if (ret) { +kfree (ctx); +dma_buf_put (dmabuf); +return ret; + } + +if (WARN_ON_ONCE (!ctx->dev_ops || + !ctx->dev_ops->map || + !ctx->dev_ops->unmap || + !ctx->dev_ops->release)) +return -EINVAL ; + + *out_ctx = ctx; +return 0 ; +} + +void dma_buf_io_detach (struct dma_buf_io_ctx *ctx) +{ +guard (mutex)(&ctx->map_create_mutex); + +dma_buf_io_kill_maps (ctx, true); +dma_buf_io_wait_maps (ctx); +} +EXPORT_SYMBOL_NS_GPL (dma_buf_io_detach,"DMA_BUF" );include/linux/dma-buf-io.h new file +113
-0@@ -0,0 +1,113 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __DMA_BUF_IO_H__ +#define __DMA_BUF_IO_H__ + +#include <linux/dma-buf.h> + +struct dma_buf_io_ctx ; +struct dma_buf_io_map ; + +struct dma_buf_io_ops { +/* +* Create a new map for the given ctx. Called with the reservation +* lock held. +*/ +struct dma_buf_io_map *(*map)(struct dma_buf_io_ctx *ctx); + +/* +* Clean up device specific parts of the @map. Called with the +* reservation lock held. +*/ +void (*unmap)(struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map); + +/* +* The user tries to destroy the ctx. Release all device specific +* parts of the token. +*/ +void (*release)(struct dma_buf_io_ctx *); +}; + +struct dma_buf_io_map { +/* +* Counts attached requests and other users. Device specific unmapping +* is deferred until all refs are dropped. +*/ +struct percpu_ref refs; +/* +* Shift for the minimum segment size of the mapping. +*/ +unsigned min_seg_shift; + +struct work_struct release_work; +struct dma_buf_io_ctx *ctx; +}; + +struct dma_buf_io_ctx { +struct dma_buf_io_map __rcu *map; +struct dma_buf *dmabuf; +enum dma_data_direction dir; + +/* synchronises map reassignment */ +struct mutex map_mutex; +struct mutex map_create_mutex; +/* maps that may still be in use. */ +atomic_t active_maps; +atomic_t all_maps; +struct wait_queue_head maps_wq; +bool maps_killed; + +void *dev_priv; +const struct dma_buf_io_ops *dev_ops; +}; + +int dma_buf_io_ctx_create (struct file *file, +struct dma_buf *dmabuf, +enum dma_data_direction dir, +struct dma_buf_io_ctx **ctx); +void dma_buf_io_ctx_release (struct dma_buf_io_ctx *ctx); + +struct dma_buf_io_map *dma_buf_io_create_map (struct dma_buf_io_ctx *ctx); + +static inline struct dma_buf_io_map * +__dma_buf_io_get_map (struct dma_buf_io_ctx *ctx) +{ +struct dma_buf_io_map *map; + +guard (rcu)(); + + map =rcu_dereference (ctx->map); +if (unlikely (!map || !percpu_ref_tryget_live_rcu (&map->refs))) +return NULL ; + +return map; +} + +static inline struct dma_buf_io_map * +dma_buf_io_get_map (struct dma_buf_io_ctx *ctx,bool nowait) +{ +struct dma_buf_io_map *map; + + map =__dma_buf_io_get_map (ctx); +if (likely (map)) +return map; + +if (nowait) +return ERR_PTR (-EAGAIN ); +return dma_buf_io_create_map (ctx); +} + +static inline void dma_buf_io_map_drop (struct dma_buf_io_map *map) +{ +percpu_ref_put (&map->refs); +} + +/* +* Device API +*/ + +void dma_buf_io_invalidate_mappings (struct dma_buf_io_ctx *ctx); +int dma_buf_io_init_map (struct dma_buf_io_ctx *ctx,struct dma_buf_io_map *map, +struct sg_table *sgt); +void dma_buf_io_detach (struct dma_buf_io_ctx *ctx); + +#endif include/linux/fs.h +2
-0@@ -1914,6 +1914,7 @@ struct dir_context {#define COPY_FILE_SPLICE (1 << 0)struct io_uring_cmd ; +struct dma_buf_io_ctx ;struct offset_ctx ;struct file_operations {@@ -1960,6 +1961,7 @@ struct file_operations {int (*uring_cmd_iopoll)(struct io_uring_cmd *,struct io_comp_batch *,unsigned int poll_flags);int (*mmap_prepare)(struct vm_area_desc *); +int (*init_dma_buf_io_ctx)(struct file *,struct dma_buf_io_ctx *); } __randomize_layout;/* Supports async buffered reads */ Signature · 1 line
2.54.0
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Quoted 1 line · Matthew Brost
On Mon, Sep 21, 2026 at 02:38:45PM +0100, Pavel Begunkov wrote:
...>> +struct dma_buf_io_map *dma_buf_io_create_map(struct dma_buf_io_ctx *ctx)
Quoted 35 lines
Quoted 22 lines
drivers/dma-buf/dma-buf-io.c new file +22
-0+{ +struct dma_buf *dmabuf = ctx->dmabuf; +struct dma_buf_io_map *map; +long ret; + +guard (mutex)(&ctx->map_create_mutex); + + scoped_guard(mutex, &ctx->map_mutex) { +if (ctx->maps_killed) +return ERR_PTR (-ENOENT ); +/* recheck under the lock in case it has already been re-created */ + map =__dma_buf_io_get_map (ctx); +if (map) +return map; + } + +dma_buf_io_wait_active_maps (ctx); + + ret =dma_resv_lock_interruptible (dmabuf->resv,NULL ); +if (ret) +return ERR_PTR (ret); +drivers/dma-buf/dma-buf-io.c line 120
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ ret =dma_resv_wait_timeout (dmabuf->resv,DMA_RESV_USAGE_KERNEL , + true,MAX_SCHEDULE_TIMEOUT ); +if (ret <=0 ) { +if (!ret) + ret = -EAGAIN ; +dma_resv_unlock (dmabuf->resv); +return ERR_PTR (ret); + } + + map = ctx->dev_ops->map (ctx);I'm playing around this code now.
Quoted 6 lines
I think you need the
dma_resv_wait_timeoutafter the 'map'?If a device doesn't support p2p ->map() will typically trigger an async migrate to system memory and data will be moving but the map is valid - Xe 100% does this, I checked AMDGPU and fairly confident it has the same async behavior.
There is a wait right before because I read somewhere in dma-buf comments that I need to do that, sounds a bit odd if I need to wait on fences before and after. I can add it, just curious how come that other dma_buf_map_attachment() callers don't need to do that. Or maybe they wait somewhere else?
Signature · 1 line
Pavel Begunkov
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Quoted 45 lines · Pavel Begunkov
Quoted 1 line · Matthew Brost
On Mon, Sep 21, 2026 at 02:38:45PM +0100, Pavel Begunkov wrote:
...>> +struct
dma_buf_io_map*dma_buf_io_create_map(structdma_buf_io_ctx*ctx)Quoted 35 lines
Quoted 22 lines
drivers/dma-buf/dma-buf-io.c new file +22
-0+{ + struct dma_buf *dmabuf = ctx->dmabuf; + struct dma_buf_io_map *map; + long ret; + + guard(mutex)(&ctx->map_create_mutex); + + scoped_guard(mutex, &ctx->map_mutex) { + if (ctx->maps_killed) + return ERR_PTR(-ENOENT); + /* recheck under the lock in case it has already been re-created */ + map = __dma_buf_io_get_map(ctx); + if (map) + return map; + } + + dma_buf_io_wait_active_maps(ctx); + + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL); + if (ret) + return ERR_PTR(ret); +drivers/dma-buf/dma-buf-io.c line 120
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL, + true, MAX_SCHEDULE_TIMEOUT); + if (ret <= 0) { + if (!ret) + ret = -EAGAIN; + dma_resv_unlock(dmabuf->resv); + return ERR_PTR(ret); + } + + map = ctx->dev_ops->map(ctx);I'm playing around this code now.
Quoted 6 lines
I think you need the
dma_resv_wait_timeoutafter the 'map'?If a device doesn't support p2p ->map() will typically trigger an async migrate to system memory and data will be moving but the map is valid - Xe 100% does this, I checked AMDGPU and fairly confident it has the same async behavior.
Quoted 5 lines
There is a wait right before because I read somewhere in dma-buf comments that I need to do that, sounds a bit odd if I need to wait on fences before and after. I can add it, just curious how come that other
dma_buf_map_attachment()callers don't need to do that. Or maybe they wait somewhere else?
I can't find it, so maybe it was the comment below and I mixed sth up back then. I'll move it after ->map().
* Note that for non-dynamic exporters the driver must guarantee that
* that the memory is available for use and cleared of any old data by
* the time this function returns. Drivers which pipeline their buffer
* moves internally must wait for all moves and clears to complete.
* Dynamic exporters do not need to follow this rule: For non-dynamic
* importers the buffer is already pinned through @pin, which has the
* same requirements. Dynamic importers otoh are required to obey the
* dma_resv fences.
*
Signature · 1 line
Pavel Begunkov
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Quoted 46 lines · Pavel Begunkov
Quoted 45 lines · Pavel Begunkov
Quoted 1 line · Matthew Brost
On Mon, Sep 21, 2026 at 02:38:45PM +0100, Pavel Begunkov wrote:
...>> +struct
dma_buf_io_map*dma_buf_io_create_map(structdma_buf_io_ctx*ctx)Quoted 41 lines
Quoted 22 lines
drivers/dma-buf/dma-buf-io.c new file +22
-0+{ + struct dma_buf *dmabuf = ctx->dmabuf; + struct dma_buf_io_map *map; + long ret; + + guard(mutex)(&ctx->map_create_mutex); + + scoped_guard(mutex, &ctx->map_mutex) { + if (ctx->maps_killed) + return ERR_PTR(-ENOENT); + /* recheck under the lock in case it has already been re-created */ + map = __dma_buf_io_get_map(ctx); + if (map) + return map; + } + + dma_buf_io_wait_active_maps(ctx); + + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL); + if (ret) + return ERR_PTR(ret); +drivers/dma-buf/dma-buf-io.c line 120
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL, + true, MAX_SCHEDULE_TIMEOUT); + if (ret <= 0) { + if (!ret) + ret = -EAGAIN; + dma_resv_unlock(dmabuf->resv); + return ERR_PTR(ret); + } + + map = ctx->dev_ops->map(ctx);I'm playing around this code now.
I think you need the
dma_resv_wait_timeoutafter the 'map'?If a device doesn't support p2p ->map() will typically trigger an async migrate to system memory and data will be moving but the map is valid - Xe 100% does this, I checked AMDGPU and fairly confident it has the same async behavior.
Quoted 5 lines
Quoted 5 lines
There is a wait right before because I read somewhere in dma-buf comments that I need to do that, sounds a bit odd if I need to wait on fences before and after. I can add it, just curious how come that other
dma_buf_map_attachment()callers don't need to do that. Or maybe they wait somewhere else?
In GPU drivers, when we receive a foreign object and map it in Xe or AMDGPU, this logic sits deep in the stack. However, the top-level call is typically ttm_bo_validate(), which in both drivers eventually resolves to the TTM ->move() vfunc. That path calls dma_buf_map_attachment(), which in turn invokes the dma-buf's ->map() callback.
That ->map() callback can trigger a asynchronous move on a different device, where the top-level call is again ttm_bo_validate(). We then end up back in the TTM ->move() vfunc, where kernel fences are installed. I realize that's a lot of layers, but the call chain can end up looking like this.
Now we have a mapping and an object with kernel fences attached. Before the object can be used by either the exec IOCTL (batch buffer submission), the VM bind IOCTL (mapping into the GPU address space), or a CPU page fault (not relevant for dma-bufs since they cannot be CPU-mapped, but a useful example of a normal BO move), we wait on those kernel fences.
In the case of the exec IOCTL or VM bind IOCTL, the kernel fences are added as dependencies to a drm_sched job, delaying its execution until the fences signal. That is where the wait occurs. In the case of a CPU page fault, the kernel waits directly on the fences before installing the CPU page mappings.
So the TL;DR is if you want to immediately hand back a valid mapping, the kernel fences must be waited on before returning after ->map() call.
Matt
Quoted 18 lines
I can't find it, so maybe it was the comment below and I mixed sth up back then. I'll move it after ->map().
* Note that for non-dynamic exporters the driver must guarantee that
* that the memory is available for use and cleared of any old data by
* the time this function returns. Drivers which pipeline their buffer
* moves internally must wait for all moves and clears to complete.
* Dynamic exporters do not need to follow this rule: For non-dynamic
* importers the buffer is already pinned through @pin, which has the
* same requirements. Dynamic importers otoh are required to obey the
*dma_resvfences.
*Signature · 1 line
Pavel Begunkov
Re: [PATCH v6 01/13] dma-buf: introduce initial file I/O infrastructure
Quoted 73 lines · Matthew Brost
Quoted 46 lines · Pavel Begunkov
Quoted 45 lines · Pavel Begunkov
Quoted 1 line · Matthew Brost
On Mon, Sep 21, 2026 at 02:38:45PM +0100, Pavel Begunkov wrote:
...>> +struct
dma_buf_io_map*dma_buf_io_create_map(structdma_buf_io_ctx*ctx)Quoted 41 lines
Quoted 22 lines
drivers/dma-buf/dma-buf-io.c new file +22
-0+{ + struct dma_buf *dmabuf = ctx->dmabuf; + struct dma_buf_io_map *map; + long ret; + + guard(mutex)(&ctx->map_create_mutex); + + scoped_guard(mutex, &ctx->map_mutex) { + if (ctx->maps_killed) + return ERR_PTR(-ENOENT); + /* recheck under the lock in case it has already been re-created */ + map = __dma_buf_io_get_map(ctx); + if (map) + return map; + } + + dma_buf_io_wait_active_maps(ctx); + + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL); + if (ret) + return ERR_PTR(ret); +drivers/dma-buf/dma-buf-io.c line 120
Quoted 10 lines
drivers/dma-buf/dma-buf-io.c new file +10
-0+ ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL, + true, MAX_SCHEDULE_TIMEOUT); + if (ret <= 0) { + if (!ret) + ret = -EAGAIN; + dma_resv_unlock(dmabuf->resv); + return ERR_PTR(ret); + } + + map = ctx->dev_ops->map(ctx);I'm playing around this code now.
I think you need the
dma_resv_wait_timeoutafter the 'map'?If a device doesn't support p2p ->map() will typically trigger an async migrate to system memory and data will be moving but the map is valid - Xe 100% does this, I checked AMDGPU and fairly confident it has the same async behavior.
Quoted 5 lines
Quoted 5 lines
There is a wait right before because I read somewhere in dma-buf comments that I need to do that, sounds a bit odd if I need to wait on fences before and after. I can add it, just curious how come that other
dma_buf_map_attachment()callers don't need to do that. Or maybe they wait somewhere else?In GPU drivers, when we receive a foreign object and map it in Xe or AMDGPU, this logic sits deep in the stack. However, the top-level call is typically
ttm_bo_validate(), which in both drivers eventually resolves to the TTM ->move() vfunc. That path callsdma_buf_map_attachment(), which in turn invokes the dma-buf's ->map() callback.That ->map() callback can trigger a asynchronous move on a different device, where the top-level call is again
ttm_bo_validate(). We then end up back in the TTM ->move() vfunc, where kernel fences are installed. I realize that's a lot of layers, but the call chain can end up looking like this.Now we have a mapping and an object with kernel fences attached. Before the object can be used by either the exec IOCTL (batch buffer submission), the VM bind IOCTL (mapping into the GPU address space), or a CPU page fault (not relevant for dma-bufs since they cannot be CPU-mapped, but a useful example of a normal BO move), we wait on those kernel fences.
Quoted 8 lines
In the case of the exec IOCTL or VM bind IOCTL, the kernel fences are added as dependencies to a
drm_schedjob, delaying its execution until the fences signal. That is where the wait occurs. In the case of a CPU page fault, the kernel waits directly on the fences before installing the CPU page mappings.So the TL;DR is if you want to immediately hand back a valid mapping, the kernel fences must be waited on before returning after ->map() call.
Thanks for the overview! I'll add the wait after ->map in v8
Quoted 13 lines
Quoted 13 lines
I can't find it, so maybe it was the comment below and I mixed sth up back then. I'll move it after ->map().
* Note that for non-dynamic exporters the driver must guarantee that
* that the memory is available for use and cleared of any old data by
* the time this function returns. Drivers which pipeline their buffer
* moves internally must wait for all moves and clears to complete.
* Dynamic exporters do not need to follow this rule: For non-dynamic
* importers the buffer is already pinned through @pin, which has the
* same requirements. Dynamic importers otoh are required to obey the
*dma_resvfences.
*
Signature · 1 line
Pavel Begunkov
PATCH v6 2/13 iov_iter: add iterator type for dmabuf maps
In reply to Pavel BegunkovIntroduce a new iterator type for dmabuf maps. The map in an opaque object with internals and format specific to the subsystem / driver, and only it can use that subsystem / driver for issuing IO. The task of the middle layers is to pass the map / iterator further down, maybe doing basic splitting and length checking. The iterator can only be used by operations of the file the associated map was created for.
--- include/linux/uio.h | 11 +++++++++++ lib/iov_iter.c | 29 +++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-)
include/linux/uio.h +11 -0
@@ -12,6 +12,7 @@struct page ;struct folio_queue ; +struct dma_buf_io_map ;typedef unsigned int __bitwise iov_iter_extraction_t ;
@@ -29,6 +30,7 @@ enum iter_type {ITER_FOLIOQ ,ITER_XARRAY ,ITER_DISCARD , + ITER_DMABUF_MAP, };#define ITER_SOURCE 1 // == WRITE
@@ -71,6 +73,7 @@ struct iov_iter {const struct folio_queue *folioq;struct xarray *xarray;void __user *ubuf; +struct dma_buf_io_map *dmabuf_map; };size_t count; };
@@ -155,6 +158,11 @@ static inline bool iov_iter_is_xarray(const struct iov_iter *i)return iov_iter_type (i) ==ITER_XARRAY ; } +static inline bool iov_iter_is_dmabuf_map (const struct iov_iter *i) +{ +return iov_iter_type (i) ==ITER_DMABUF_MAP ; +} +static inline unsigned char iov_iter_rw (const struct iov_iter *i) {return i->data_source ?WRITE :READ ;
@@ -300,6 +308,9 @@ void iov_iter_folio_queue(struct iov_iter *i, unsigned int direction, unsigned int first_slot, unsigned int offset, size_t count);void iov_iter_xarray (struct iov_iter *i,unsigned int direction,struct xarray *xarray,loff_t start,size_t count); +void iov_iter_dmabuf_map (struct iov_iter *i,unsigned int direction, +struct dma_buf_io_map *map, +loff_t off,size_t count);ssize_t iov_iter_get_pages2 (struct iov_iter *i,struct page **pages,size_t maxsize,unsigned maxpages,size_t *start); ssize_t iov_iter_get_pages_alloc2(struct iov_iter *i, struct page ***pages,
lib/iov_iter.c +23 -6
@@ -575,7 +575,8 @@ void iov_iter_advance(struct iov_iter *i, size_t size) {if (unlikely (i->count < size)) size = i->count; -if (likely (iter_is_ubuf (i)) ||unlikely (iov_iter_is_xarray (i))) { +if (likely (iter_is_ubuf (i)) ||unlikely (iov_iter_is_xarray (i)) || +unlikely (iov_iter_is_dmabuf_map (i))) { i->iov_offset += size; i->count -= size; }else if (likely (iter_is_iovec (i) ||iov_iter_is_kvec (i))) {
@@ -631,7 +632,8 @@ void iov_iter_revert(struct iov_iter *i, size_t unroll)return ; } unroll -= i->iov_offset; -if (iov_iter_is_xarray (i) ||iter_is_ubuf (i)) { +if (iov_iter_is_xarray (i) ||iter_is_ubuf (i) || +iov_iter_is_dmabuf_map (i)) {BUG ();/* We should never go beyond the start of the specified * range since we might then be straying into pages that * aren't pinned.
@@ -775,6 +777,20 @@ void iov_iter_xarray(struct iov_iter *i, unsigned int direction, }EXPORT_SYMBOL (iov_iter_xarray); +void iov_iter_dmabuf_map (struct iov_iter *i,unsigned int direction, +struct dma_buf_io_map *map, +loff_t off,size_t count) +{ +WARN_ON (direction & ~(READ |WRITE )); + *i = (struct iov_iter ){ + .iter_type =ITER_DMABUF_MAP , + .data_source = direction, + .dmabuf_map = map, + .count = count, + .iov_offset = off, + }; +} +/** * iov_iter_discard - Initialise an I/O iterator that discards data * @i: The iterator to initialise.
@@ -856,7 +872,7 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)return iov_iter_alignment_bvec (i);/* With both xarray and folioq types, we're dealing with whole folios. */ -if (iov_iter_is_folioq (i)) +if (iov_iter_is_folioq (i) ||iov_iter_is_dmabuf_map (i))return i->iov_offset | i->count;if (iov_iter_is_xarray (i))return (i->xarray_start + i->iov_offset) | i->count;
@@ -872,7 +888,7 @@ unsigned long iov_iter_gap_alignment(const struct iov_iter *i)size_t size = i->count;unsigned k; -if (iter_is_ubuf (i)) +if (iter_is_ubuf (i) ||iov_iter_is_dmabuf_map (i))return 0 ;if (WARN_ON (!iter_is_iovec (i)))
@@ -1469,11 +1485,12 @@ EXPORT_SYMBOL_GPL(import_ubuf);void iov_iter_restore (struct iov_iter *i,struct iov_iter_state *state) {if (WARN_ON_ONCE (!iov_iter_is_bvec (i) && !iter_is_iovec (i) && - !iter_is_ubuf (i)) && !iov_iter_is_kvec (i)) + !iter_is_ubuf (i) && !iov_iter_is_kvec (i) && + !iov_iter_is_dmabuf_map (i)))return ; i->iov_offset = state->iov_offset; i->count = state->count; -if (iter_is_ubuf (i)) +if (iter_is_ubuf (i) ||iov_iter_is_dmabuf_map (i))return ;/* * For the *vec iters, nr_segs + iov is constant - if we increment
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v6 02/13] iov_iter: add iterator type for dmabuf maps
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] iov_iter: silent fallback to iterate_discard() for dmabuf maps - [Medium] iov_iter: bounds check…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] iov_iter: silent fallback to iterate_discard() for dmabuf maps
- [Medium] iov_iter: bounds check bypass in iov_iter_revert() for dmabuf maps --
commit 8e47b333dea94c1451175bac8e284adfbbf175cc
Author: Pavel Begunkov <[email protected]>
Subject: iov_iter: add iterator type for dmabuf maps
Introduce a new ITER_DMABUF_MAP iterator type for dmabuf maps. The patch adds the new iterator type, updates struct iov_iter to hold dmabuf maps, and wires up basic iterator operations like advance, revert, and alignment.
Quoted 4 lines
include/linux/uio.h +0
-0
[ ... ]
include/linux/uio.h enum iter_type { · line 34
Quoted 6 lines
include/linux/uio.h +1
-0@@ -29,6 +30,7 @@ enum iter_type {ITER_FOLIOQ ,ITER_XARRAY ,ITER_DISCARD , + ITER_DMABUF_MAP, };
[Severity: Medium]
With the addition of ITER_DMABUF_MAP here, do iterate_and_advance2() and iterate_and_advance_kernel() in include/linux/iov_iter.h also need to be updated?
It appears they currently lack a type check for ITER_DMABUF_MAP, which might cause operations like copy_to_iter() or copy_from_iter() to silently fall through to iterate_discard(). If any middle layer attempts to read or write using this iterator type, the operation would drop data and return success without reporting an error.
[ ... ]
Quoted 4 lines
lib/iov_iter.c +0
-0
[ ... ]
Quoted 7 lines
lib/iov_iter.c +3
-0@@ -775,6 +777,20 @@ void iov_iter_xarray(struct iov_iter *i, unsigned int direction, }EXPORT_SYMBOL (iov_iter_xarray); +void iov_iter_dmabuf_map (struct iov_iter *i,unsigned int direction, +struct dma_buf_io_map *map, +loff_t off,size_t count)
lib/iov_iter.c void iov_iter_xarray(struct iov_iter *i, unsigned int direction, · line 792
Quoted 10 lines
lib/iov_iter.c +10
-0+{ +WARN_ON (direction & ~(READ |WRITE )); + *i = (struct iov_iter ){ + .iter_type =ITER_DMABUF_MAP , + .data_source = direction, + .dmabuf_map = map, + .count = count, + .iov_offset = off, + }; +}
[Severity: Medium]
Since iov_offset is initialized here to the absolute starting offset (off) rather than a relative zero, does this cause an issue with the tracking logic in iov_iter_revert()?
In iov_iter_revert(), the unroll size is checked against i->iov_offset like this:
if (unroll <= i->iov_offset) {
i->iov_offset -= unroll;
return;
}Because iov_offset starts at the absolute offset rather than 0, couldn't a caller successfully revert an ITER_DMABUF_MAP iterator beyond its initial starting offset? This would bypass the BUG() safety net intended to catch invalid unrolls, potentially leading to an out-of-bounds DMA access if the reverted iterator is used again.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH v6 02/13] iov_iter: add iterator type for dmabuf maps
On 9/21/26 14:53, [email protected] wrote:
...>> +void iov_iter_dmabuf_map(struct iov_iter *i, unsigned int direction,
Quoted 9 lines
Quoted 9 lines
lib/iov_iter.c +9
-0+struct dma_buf_io_map *map, + loff_t off, size_t count) +{ + WARN_ON(direction & ~(READ | WRITE)); + *i = (struct iov_iter){ + .iter_type = ITER_DMABUF_MAP, + .data_source = direction, + .dmabuf_map = map, + .count = count,
Quoted 8 lines
lib/iov_iter.c void iov_iter_xarray(struct iov_iter *i, unsigned int direction, · line 792
Quoted 3 lines
lib/iov_iter.c +3
-0+ .iov_offset = off, + }; +}[Severity: Medium]
Sinceiov_offsetis initialized here to the absolute starting offset (off) rather than a relative zero, does this cause an issue with the tracking logic iniov_iter_revert()?
It is relative to the buffer. No clue why it thinks it's absolute, but this unroll looks fine.
Quoted 13 lines
In
iov_iter_revert(), the unroll size is checked againsti->iov_offsetlike this:if (unroll <= i->iov_offset) { i->iov_offset -= unroll; return; }Because
iov_offsetstarts at the absolute offset rather than 0, couldn't a caller successfully revert anITER_DMABUF_MAPiterator beyond its initial starting offset? This would bypass theBUG()safety net intended to catch invalid unrolls, potentially leading to an out-of-bounds DMA access if the reverted iterator is used again.
Signature · 1 line
Pavel Begunkov
PATCH v6 3/13 block: always adjust bi_offset on bio_advance_iter
In reply to Pavel BegunkovExtend bio_no_advance_iter() iteration to also increment the offset. It's not currently needed because the currently listed request types don't have a buffer, but will be useful once we add bios backed by dma-buf, which don't have a bvec but work with a single range.
--- drivers/md/dm-io-rewind.c | 6 ++++-- include/linux/bio.h | 12 ++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-)
drivers/md/dm-io-rewind.c +4 -2
@@ -113,10 +113,12 @@ static inline void dm_bio_rewind_iter(const struct bio *bio, iter->bi_sector -= bytes >>9 ;/* No advance means no rewind */ -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size += bytes; -else + iter->bi_offset -= bytes; + }else {dm_bvec_iter_rewind (bio->bi_io_vec, iter, bytes); + } }/**
include/linux/bio.h +8 -4
@@ -113,11 +113,13 @@ static inline void bio_advance_iter(const struct bio *bio, { iter->bi_sector += bytes >>9 ; -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size -= bytes; -else + iter->bi_offset += bytes; + }else {bvec_iter_advance (bio->bi_io_vec, iter, bytes);/* TODO: It is reasonable to complete bio with error here. */ + } }/* @bytes should be less or equal to bvec[i->bi_idx].bv_len */
@@ -127,10 +129,12 @@ static inline void bio_advance_iter_single(const struct bio *bio, { iter->bi_sector += bytes >>9 ; -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size -= bytes; -else + iter->bi_offset += bytes; + }else {bvec_iter_advance_single (bio->bi_io_vec, iter, bytes); + } }void __bio_advance (struct bio *,unsigned bytes);
Signature · 1 line
2.54.0
PATCH v6 4/13 block: introduce dma map backed bio type
In reply to Pavel BegunkovPremapped buffers don't require a generic bio_vec since these have already been dma mapped. Repurpose the bi_io_vec space to strore dmabuf maps as they are mutually exclusive.
--- block/bio.c | 15 +++++++++++-- block/blk-merge.c | 45 +++++++++++++++++++++++++++++++++++++++ block/fops.c | 2 +- include/linux/bio.h | 9 ++++---- include/linux/blk-mq.h | 7 ++++++ include/linux/blk_types.h | 14 +++++++++++- include/linux/bvec.h | 3 ++- 7 files changed, 86 insertions(+), 9 deletions(-)
block/bio.c +13 -2
@@ -881,7 +881,11 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp) bio->bi_write_stream = bio_src->bi_write_stream; bio->bi_bvec_gap_bit = bio_src->bi_bvec_gap_bit; bio->bi_iter = bio_src->bi_iter; - bio->bi_io_vec = bio_src->bi_io_vec; + +if (op_is_dmabuf (bio->bi_opf)) + bio->bi_dmabuf_map = bio_src->bi_dmabuf_map; +else + bio->bi_io_vec = bio_src->bi_io_vec;if (bio->bi_bdev) { if (bio->bi_bdev == bio_src->bi_bdev &&
@@ -1204,16 +1208,23 @@ EXPORT_SYMBOL_GPL(__bio_release_pages);bool bio_iov_iter_set (struct bio *bio,const struct iov_iter *iter) { -if (!iov_iter_is_bvec (iter)) +if (!iov_iter_is_bvec (iter) && !iov_iter_is_dmabuf_map (iter))return false;WARN_ON_ONCE (bio->bi_max_vecs); +static_assert (offsetof(struct bio , bi_io_vec) == + offsetof(struct bio , bi_dmabuf_map)); +static_assert (offsetof(struct iov_iter , bvec) == + offsetof(struct iov_iter , dmabuf_map)); + bio->bi_io_vec = (struct bio_vec *)iter->bvec; bio->bi_iter.bi_idx =0 ; bio->bi_iter.bi_offset = iter->iov_offset; bio->bi_iter.bi_size =iov_iter_count (iter);bio_set_flag (bio,BIO_CLONED ); +if (iov_iter_is_dmabuf_map (iter)) + bio->bi_opf |=REQ_NOMERGE |REQ_DMABUF ;return true; }
block/blk-merge.c +45 -0
69 lines not on this page · raw message
block/fops.c +1 -1
@@ -362,7 +362,7 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,* Users don't rely on the iterator being in any particular * state for async I/O returning -EIOCBQUEUED, hence we can * avoid expensive iov_iter_advance(). Bypass -* bio_iov_iter_get_pages() and set the bvec directly. +* bio_iov_iter_get_pages() and set the bvec/dmabuf directly. */ if (!bio_iov_iter_set (bio, iter)) { ret =blkdev_iov_iter_get_pages (bio, iter, bdev);
include/linux/bio.h +5 -4
@@ -80,7 +80,8 @@ static inline bool bio_no_advance_iter(const struct bio *bio) {return bio_op (bio) ==REQ_OP_DISCARD ||bio_op (bio) ==REQ_OP_SECURE_ERASE || -bio_op (bio) ==REQ_OP_WRITE_ZEROES ; +bio_op (bio) ==REQ_OP_WRITE_ZEROES || +op_is_dmabuf (bio->bi_opf); }static inline void *bio_data (struct bio *bio)
@@ -438,12 +439,12 @@ static inline void bio_wouldblock_error(struct bio *bio)/* * Calculate number of bvec segments that should be allocated to fit data -* pointed by @iter. If @iter is backed by bvec it's going to be reused -* instead of allocating a new one. +* pointed by @iter. If @iter is backed by a bvec or a dmabuf, the bvec array / +* the dma map are going to be reused, and so no extra allocation is required. */ static inline int bio_iov_vecs_to_alloc (struct iov_iter *iter,int max_segs) { -if (iov_iter_is_bvec (iter)) +if (iov_iter_is_bvec (iter) ||iov_iter_is_dmabuf_map (iter))return 0 ;return iov_iter_npages (iter, max_segs); }
include/linux/blk-mq.h +7 -0
@@ -1017,6 +1017,13 @@ static inline void *blk_mq_rq_to_pdu(struct request *rq)return rq +1 ; } +static inline bool blk_mq_rq_is_dmabuf (struct request *rq) +{ +if (!IS_ENABLED (CONFIG_DMA_SHARED_BUFFER )) +return false; +return rq->bio &&op_is_dmabuf (rq->bio->bi_opf); +} +static inline struct blk_mq_hw_ctx *queue_hctx (struct request_queue *q,int id) {struct blk_mq_hw_ctx *hctx;
include/linux/blk_types.h +13 -1
@@ -233,7 +233,12 @@ struct bio {atomic_t __bi_remaining;/* The actual vec list, preserved by bio_reset() */ -struct bio_vec *bi_io_vec; +union { +struct bio_vec *bi_io_vec; +/* Driver specific dma map, valid IFF REQ_DMABUF is set */ +struct dma_buf_io_map *bi_dmabuf_map; + }; +struct bvec_iter bi_iter;union {
@@ -402,6 +407,7 @@ enum req_flag_bits { __REQ_DRV,/* for driver use */ __REQ_FS_PRIVATE,/* for file system (submitter) use */ __REQ_ATOMIC,/* for atomic write operations */ + __REQ_DMABUF, /* Using premmaped dma buffers *//* * Command specific flags, keep last: */
@@ -434,6 +440,7 @@ enum req_flag_bits {#define REQ_DRV (__force blk_opf_t)(1ULL << __REQ_DRV)#define REQ_FS_PRIVATE (__force blk_opf_t)(1ULL << __REQ_FS_PRIVATE)#define REQ_ATOMIC (__force blk_opf_t)(1ULL << __REQ_ATOMIC) +#define REQ_DMABUF (__force blk_opf_t)(1ULL << __REQ_DMABUF)#define REQ_NOUNMAP (__force blk_opf_t)(1ULL << __REQ_NOUNMAP)
@@ -487,6 +494,11 @@ static inline bool op_is_discard(blk_opf_t op)return (op &REQ_OP_MASK ) ==REQ_OP_DISCARD ; } +static inline bool op_is_dmabuf (blk_opf_t op) +{ +return op &REQ_DMABUF ; +} +/* * Check if a bio or request operation is a zone management operation. */
include/linux/bvec.h +2 -1
@@ -108,7 +108,8 @@ struct bvec_iter {unsigned int bi_idx;/* -* Current offset in the bvec entry pointed to by `bi_idx`. +* Current offset in the bvec entry pointed to by `bi_idx` or into +* a dma-buf map. */ unsigned int bi_offset; } __packed__aligned(4) ;
Signature · 1 line
2.54.0
Christoph Hellwig
maintainerRe: [PATCH v6 04/13] block: introduce dma map backed bio type
Quoted 2 lines · Pavel Begunkov
Premapped buffers don't require a generic
bio_vecsince these have already been dma mapped. Repurpose thebi_io_vecspace to strore dmabuf
s/strore/store/
Can you please expland on the splitting considerations a bit? Preferably both in a comment for the details, and in the commit log for how we arrived at them and why they are fine for now?
Otherwise looks good.
Re: [PATCH v6 04/13] block: introduce dma map backed bio type
Quoted 9 lines · Christoph Hellwig
Quoted 2 lines · Pavel Begunkov
Premapped buffers don't require a generic
bio_vecsince these have already been dma mapped. Repurpose thebi_io_vecspace to strore dmabufs/strore/store/
Can you please expland on the splitting considerations a bit? Preferably both in a comment for the details, and in the commit log for how we arrived at them and why they are fine for now?
I'll add something to the commit log, but not sure which specific details in comments you mean. Do you want me to expand the comment above limiting the number of segments? Maybe I should add there that it's stricter than necessary and might cause more splitting than necessary? This one:
/*
* Limit by the number of segments. We don't expose the underlying
* mapping layout, but with a known minimum segment size, any I/O
* consisting of N full segments should be able to cover at least
* this much.
*/
Signature · 1 line
Pavel Begunkov
Christoph Hellwig
maintainerRe: [PATCH v6 04/13] block: introduce dma map backed bio type
Quoted 10 lines · Pavel Begunkov
Quoted 4 lines
Can you please expland on the splitting considerations a bit? Preferably both in a comment for the details, and in the commit log for how we arrived at them and why they are fine for now?
I'll add something to the commit log, but not sure which specific details in comments you mean. Do you want me to expand the comment above limiting the number of segments? Maybe I should add there that it's stricter than necessary and might cause more splitting than necessary? This one:
Yes, explain why we apply a relatively arbitrary limit not fully exploiting the hardware capabilities, and why we thing that is okay.
PATCH v6 5/13 block: add dma-buf support for raw bdev
In reply to Pavel BegunkovAdd a simple proxy implementation of init_dma_buf_io_ctx() forwarding the call to a new struct block_device_operations operation. Also reject dma-buf backed iterators for buffered IO.
--- block/fops.c | 22 +++++++++++++++++++++- include/linux/blkdev.h | 2 ++ 2 files changed, 23 insertions(+), 1 deletion(-)
block/fops.c +21 -1
@@ -782,7 +782,8 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)if (iocb->ki_flags &IOCB_DIRECT ) { ret =blkdev_direct_write (iocb, from); -if (ret >=0 &&iov_iter_count (from)) { +if (ret >=0 &&iov_iter_count (from) && + !iov_iter_is_dmabuf_map (from)) { ret =direct_write_fallback (iocb, from, ret,blkdev_buffered_write (iocb, from)); need_sync = true;
@@ -795,6 +796,9 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) need_sync = true; } }else { +if (unlikely (iov_iter_is_dmabuf_map (from))) +return -EOPNOTSUPP ; +/* * Take i_rwsem and invalidate_lock to avoid racing with * set_blocksize changing i_blkbits/folio order and punching
@@ -850,6 +854,8 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)if (ret <0 || !count) goto reexpand; } +if (unlikely (iov_iter_is_dmabuf_map (to))) +return -EOPNOTSUPP ;/* * Take i_rwsem and invalidate_lock to avoid racing with set_blocksize
@@ -953,6 +959,19 @@ static int blkdev_mmap_prepare(struct vm_area_desc *desc)return generic_file_mmap_prepare (desc); } +static int blkdev_init_dma_buf_io_ctx (struct file *file, +struct dma_buf_io_ctx *ctx) +{ +struct block_device *bdev =file_bdev (file); +struct gendisk *disk = bdev->bd_disk; + +if (!(file->f_flags &O_DIRECT )) +return -EINVAL ; +if (!disk->fops->init_dma_buf_io_ctx) +return -EOPNOTSUPP ; +return disk->fops->init_dma_buf_io_ctx (bdev, ctx); +} +const struct file_operations def_blk_fops = { .open = blkdev_open, .release = blkdev_release,
@@ -971,6 +990,7 @@ const struct file_operations def_blk_fops = { .fallocate = blkdev_fallocate, .uring_cmd = blkdev_uring_cmd, .fop_flags = FOP_BUFFER_RASYNC | FOP_DONTCACHE, + .init_dma_buf_io_ctx = blkdev_init_dma_buf_io_ctx, };static __init int blkdev_init (void )
include/linux/blkdev.h +2 -0
@@ -1599,6 +1599,8 @@ struct block_device_operations {/* returns the length of the identifier or a negative errno: */ int (*get_unique_id)(struct gendisk *disk,u8 id[16 ],enum blk_unique_id id_type); +int (*init_dma_buf_io_ctx)(struct block_device *, +struct dma_buf_io_ctx *);struct module *owner;const struct pr_ops *pr_ops;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v6 05/13] block: add dma-buf support for raw bdev
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] block: blkdev_read_iter() discards partial direct read success for dma-bufs - [Low] block…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] block: blkdev_read_iter() discards partial direct read success for dma-bufs
- [Low] block: dmabuf error paths skip iov_iter_reexpand() teardown
--
commit 1ef88ba1be9691ca9af0098196766a3034d434e9
Author: Pavel Begunkov <[email protected]>
block: add dma-buf support for raw bdev
Add a simple proxy implementation of init_dma_buf_io_ctx() forwarding the call to a new struct block_device_operations operation. Also reject dma-buf backed iterators for buffered IO.
Quoted 4 lines
block/fops.c +0
-0
[ ... ]
Quoted 11 lines
block/fops.c +2
-1@@ -783,7 +783,8 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)if (iocb->ki_flags &IOCB_DIRECT ) { ret =blkdev_direct_write (iocb, from); -if (ret >=0 &&iov_iter_count (from)) { +if (ret >=0 &&iov_iter_count (from) && + !iov_iter_is_dmabuf_map (from)) { ret =direct_write_fallback (iocb, from, ret,blkdev_buffered_write (iocb, from)); need_sync = true;
block/fops.c static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) · line 801
Quoted 5 lines
block/fops.c +2
-0@@ -796,6 +797,9 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) need_sync = true; } }else { +if (unlikely (iov_iter_is_dmabuf_map (from))) +return -EOPNOTSUPP ;
^^^^^^^^^^^^^^^^^^
[Severity: Low]
At the start of blkdev_write_iter(), iov_iter_truncate() restricts the I/O length and tracks the truncated amount in shorted. By returning directly here, does this bypass the iov_iter_reexpand() call at the end of the function, leaving the iterator incorrectly truncated?
Quoted 4 lines
block/fops.c +1
-0+/* * Take i_rwsem and invalidate_lock to avoid racing with * set_blocksize changing i_blkbits/folio order and punching
[ ... ]
block/fops.c static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to) · line 859
Quoted 6 lines
block/fops.c +2
-0@@ -851,6 +855,8 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)if (ret <0 || !count) goto reexpand; } +if (unlikely (iov_iter_is_dmabuf_map (to))) +return -EOPNOTSUPP ;
^^^^^^^^^^^^^^^^^^
[Severity: Medium]
If a partial O_DIRECT read using a dma-buf iterator successfully reads data and advances ki_pos in the preceding direct I/O path, could this check unconditionally return -EOPNOTSUPP? Does this overwrite the successful byte count (ret > 0) with an error and hide the read data from userspace?
[Severity: Low]
Similar to blkdev_write_iter(), if this returns directly, does it bypass the goto reexpand path, leaving the iterator incorrectly truncated if iov_iter_truncate() was called earlier in blkdev_read_iter()?
Quoted 3 lines
block/fops.c +0
-0/* * Take i_rwsem and invalidate_lock to avoid racing with set_blocksize
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
PATCH v6 6/13 nvme-pci: implement dma-buf backed requests
In reply to Pavel BegunkovEnable BIO_DMABUF_MAP backed requests. On registration we map the dma-buf and store it as a prp list, which is then used to initialise requests. All attached contexts are stored in a new list dmabuf_ctxs, and additions/removals are synchronised with dmabuf_lock.
--- drivers/nvme/host/core.c | 12 ++ drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/pci.c | 325 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 339 insertions(+)
drivers/nvme/host/core.c +12 -0
@@ -2699,6 +2699,17 @@ static int nvme_report_zones(struct gendisk *disk, sector_t sector,#define nvme_report_zones NULL#endif /* CONFIG_BLK_DEV_ZONED */ +static int nvme_init_dma_buf_io_ctx (struct block_device *bdev, +struct dma_buf_io_ctx *ctx) +{ +struct nvme_ns *ns = bdev->bd_disk->private_data; +struct nvme_ctrl *ctrl = ns->ctrl; + +if (!ctrl->ops->init_dma_buf_io_ctx) +return -EINVAL ; +return ctrl->ops->init_dma_buf_io_ctx (ctrl, ctx); +} +const struct block_device_operations nvme_bdev_ops = { .owner =THIS_MODULE , .ioctl = nvme_ioctl,
@@ -2709,6 +2720,7 @@ const struct block_device_operations nvme_bdev_ops = { .get_unique_id = nvme_get_unique_id, .report_zones = nvme_report_zones, .pr_ops = &nvme_pr_ops, + .init_dma_buf_io_ctx = nvme_init_dma_buf_io_ctx, }; static int nvme_wait_ready(struct nvme_ctrl *ctrl, u32 mask, u32 val,
drivers/nvme/host/nvme.h +2 -0
@@ -667,6 +667,8 @@ struct nvme_ctrl_ops {int (*get_address)(struct nvme_ctrl *ctrl,char *buf,int size);void (*print_device_info)(struct nvme_ctrl *ctrl);bool (*supports_pci_p2pdma)(struct nvme_ctrl *ctrl); +int (*init_dma_buf_io_ctx)(struct nvme_ctrl *ctrl, +struct dma_buf_io_ctx *ctx);unsigned long (*get_virt_boundary)(struct nvme_ctrl *ctrl,bool is_admin); };
drivers/nvme/host/pci.c +325 -0
390 lines not on this page · raw message
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v6 06/13] nvme-pci: implement dma-buf backed requests
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [Critical] nvme: deadlock in nvme_dev_disable() during device removal - [High] nvme: illegal use of…
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] nvme: deadlock in nvme_dev_disable() during device removal
- [High] nvme: illegal use of single-mapping sync API on sg-mapped dmabuf
- [High] nvme: heap buffer overflow in nvme_dma_buf_io_map()
- [High] nvme: memory leak of dmabuf map in unmap path
- [High] nvme: controller reset permanently breaks dmabuf registrations
--
commit 1bee4d32dec1291a7dc832efe4620264ba2425af
Author: Pavel Begunkov <[email protected]>
nvme-pci: implement dma-buf backed requests
Enable BIO_DMABUF_MAP backed requests. On registration we map the dma-buf and store it as a prp list, which is then used to initialise requests. All attached contexts are stored in a new list dmabuf_ctxs, and additions/removals are synchronised with dmabuf_lock.
Quoted 4 lines
drivers/nvme/host/pci.c +0
-0
[ ... ]
Quoted 4 lines
drivers/nvme/host/pci.c +3
-0@@ -865,6 +876,140 @@ static void nvme_free_descriptors(struct request *req) +static void nvme_dmabuf_map_sync_for_cpu (struct nvme_dev *nvme_dev, +struct request *req) +{
[ ... ]
drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 905
Quoted 6 lines
drivers/nvme/host/pci.c +6
-0+while (length >0 ) { +dma_sync_single_for_cpu (dev, dma_list[map_idx++], +NVME_CTRL_PAGE_SIZE , dma_dir); + length -=NVME_CTRL_PAGE_SIZE ; + } +}
[Severity: High]
Does this code unlawfully use the single-mapping sync API on an sg-mapped address?
The dmabuf memory is mapped using dma_buf_map_attachment() which uses dma_map_sg(). The driver then illegally syncs sub-pages of this scatterlist using dma_sync_single_for_cpu() in nvme_dmabuf_map_sync_for_cpu() and dma_sync_single_for_device() in nvme_dmabuf_map_sync_for_device().
This violates the DMA API. On platforms with SWIOTLB, this can crash or cause silent data corruption because the bounce buffer metadata expects the exact original dma_addr_t and size mapped by the sg API, not piecemeal chunks.
Quoted 4 lines
drivers/nvme/host/pci.c +4
-0+ +static void nvme_dmabuf_map_sync_for_device (struct nvme_dev *nvme_dev, +struct request *req) +{
[ ... ]
Quoted 6 lines
drivers/nvme/host/pci.c +6
-0+while (length >0 ) { +dma_sync_single_for_device (dev, dma_list[map_idx++], +NVME_CTRL_PAGE_SIZE , dma_dir); + length -=NVME_CTRL_PAGE_SIZE ; + } +}
[ ... ]
Quoted 27 lines
drivers/nvme/host/pci.c +27
-0+static struct dma_buf_io_map *nvme_dma_buf_io_map (struct dma_buf_io_ctx *ctx) +{ +unsigned nr_entries = ctx->dmabuf->size /NVME_CTRL_PAGE_SIZE ; +struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv; +struct dma_buf_attachment *attach = nvme_ctx->attach; +unsigned long tmp, i =0 ; +struct nvme_dmabuf_map *map; +struct scatterlist *sg; +struct sg_table *sgt; +int ret; + +dma_resv_assert_held (ctx->dmabuf->resv); + + map =kmalloc_flex (*map, dma_list, nr_entries); +if (!map) +return ERR_PTR (-ENOMEM ); + + sgt =dma_buf_map_attachment (attach, ctx->dir); +if (IS_ERR (sgt)) { + ret =PTR_ERR (sgt); + sgt =NULL ; + goto err; + } + +for_each_sgtable_dma_sg (sgt, sg, tmp) { +dma_addr_t dma_addr =sg_dma_address (sg); +unsigned long sg_len =sg_dma_len (sg);
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2497
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+ +if (sg_len %NVME_CTRL_PAGE_SIZE ) { + ret = -EINVAL ; + goto err; + } +while (sg_len) { + map->dma_list[i++] = dma_addr; + dma_addr +=NVME_CTRL_PAGE_SIZE ; + sg_len -=NVME_CTRL_PAGE_SIZE ; + }
[Severity: High]
Can an attacker controlling the dmabuf size trigger a heap buffer overflow here?
In nvme_dma_buf_io_map(), nr_entries is calculated as ctx->dmabuf->size / NVME_CTRL_PAGE_SIZE, truncating down if the size is unaligned. The flex array map is allocated for nr_entries.
The sg_table returned by the exporter covers the rounded-up size. The loop iterates based on sg_len and writes unconditionally to map->dma_list. If size was unaligned, sg_len will exceed the truncated calculated size, causing the loop to write past the end of the kmalloc_flex allocation.
Quoted 15 lines
drivers/nvme/host/pci.c +15
-0+ } + + ret =dma_buf_io_init_map (ctx, &map->base, sgt); +if (ret) + goto err; + map->nr_entries = nr_entries; + map->sgt = sgt; +return &map->base; +err: +if (sgt) +dma_buf_unmap_attachment (attach, sgt, ctx->dir); +kfree (map); +return ERR_PTR (ret); +} +
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2522
Quoted 10 lines
drivers/nvme/host/pci.c +10
-0+static void nvme_dma_buf_io_unmap (struct dma_buf_io_ctx *ctx, +struct dma_buf_io_map *map_base) +{ +struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (map_base); + +dma_resv_assert_held (ctx->dmabuf->resv); + +dma_buf_unmap_attachment (nvme_ctx->attach, map->sgt, ctx->dir); +}
[Severity: High]
Does this code leak the memory of the map structure and percpu data on every unmap operation?
nvme_dma_buf_io_map() allocates the map structure using kmalloc_flex(). The framework calls dev_ops->unmap (nvme_dma_buf_io_unmap) when the map is released. nvme_dma_buf_io_unmap() unmaps the attachment but fails to call kfree(map) and percpu_ref_exit(&map->refs).
[ ... ]
drivers/nvme/host/pci.c static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled) · line 2597
Quoted 12 lines
drivers/nvme/host/pci.c +12
-0+static void nvme_pci_remove_dmabuf (struct nvme_dev *dev) +{ +struct nvme_dma_buf_io_ctx *ctx, *tmp; + +mutex_lock (&dev->dmabuf_lock); +list_for_each_entry_safe (ctx, tmp, &dev->dmabuf_ctxs, list) { +dma_buf_io_detach (ctx->ctx); +dma_buf_detach (ctx->ctx->dmabuf, ctx->attach); +list_del_init (&ctx->list); + } +mutex_unlock (&dev->dmabuf_lock); +}
[Severity: High]
Does this permanently break dmabuf registrations across transient transport resets?
During a transient controller reset (e.g. from an IO timeout), nvme_dev_disable() calls nvme_pci_remove_dmabuf(). This permanently detaches the dmabuf and marks the context's maps as killed.
The NVMe controller resets and comes back online, but the userspace registered buffers are permanently destroyed and will return -ENODEV. This breaks the intended transparency of NVMe resets, causing IO submissions using registered dmabufs to fail unrecoverably until userspace re-registers them.
[ ... ]
drivers/nvme/host/pci.c static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown) · line 3638
Quoted 7 lines
drivers/nvme/host/pci.c +2
-0@@ -3316,6 +3633,8 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)struct pci_dev *pdev =to_pci_dev (dev->dev);bool dead; +nvme_pci_remove_dmabuf (dev); +mutex_lock (&dev->shutdown_lock);
[Severity: Critical]
Could this sequence result in an ABBA deadlock preventing device recovery?
nvme_dev_disable() calls nvme_pci_remove_dmabuf(dev) before calling nvme_cancel_tagset(). nvme_pci_remove_dmabuf() calls dma_buf_io_detach() which blocks waiting for all active dmabuf IOs to complete.
Since the device is dead or resetting, the hardware won't complete the IOs. The IOs must be aborted by nvme_cancel_tagset(), but the thread is permanently blocked waiting for them. This was not resolved in subsequent patches in the series.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
Christoph Hellwig
maintainerRe: [PATCH v6 06/13] nvme-pci: implement dma-buf backed requests
In reply to Pavel BegunkovQuoted 1 line · Pavel Begunkov
Enable
BIO_DMABUF_MAPbacked requests. On registration we map the
This is now REQ_DMABUF.
Sashiko had a few comments, which I think are correct - if we use dma_map_sg to map the data, we need to use the sync_sg APIs to transfer ownership. That only matters on non-coherent architectures with MMU, but we need to get it right.
And the reset deadlock also looks plausible.
Re: [PATCH v6 06/13] nvme-pci: implement dma-buf backed requests
Hi Christoph,
Quoted 4 lines · Christoph Hellwig
Quoted 1 line · Pavel Begunkov
Enable
BIO_DMABUF_MAPbacked requests. On registration we map theThis is now
REQ_DMABUF.
Will change
Quoted 4 lines
Sashiko had a few comments, which I think are correct - if we use
dma_map_sgto map the data, we need to use thesync_sgAPIs to transfer ownership. That only matters on non-coherent architectures with MMU, but we need to get it right.
I've seen that and fixed everything locally that should be fixed, apart from the sync. I wonder what we can do about that? I can somehow replace it with the sg variant for now, but sync'ing the entire possibly multi-GB mapping for, let's say, a 512B I/O, sounds not wise. And I can think of another place that does mix sync_single with sgs.
Quoted 1 line
And the reset deadlock also looks plausible.
Signature · 1 line
Pavel Begunkov
Christoph Hellwig
maintainerRe: [PATCH v6 06/13] nvme-pci: implement dma-buf backed requests
Quoted 10 lines · Pavel Begunkov
Quoted 4 lines
Sashiko had a few comments, which I think are correct - if we use
dma_map_sgto map the data, we need to use thesync_sgAPIs to transfer ownership. That only matters on non-coherent architectures with MMU, but we need to get it right.I've seen that and fixed everything locally that should be fixed, apart from the sync. I wonder what we can do about that? I can somehow replace it with the sg variant for now, but sync'ing the entire possibly multi-GB mapping for, let's say, a 512B I/O, sounds not wise.
If we'd want to fix this for real we'd need to add an offset to the sync by sg methods. Or move away from scatterlists in dma-buf, given that they are a horrible API. All of these are bigger projects, though. So for now I think you'd want to do the sync all, and have the people who run high-performance io_uring code on non-coherent platforms suffer.
Quoted 2 lines
And I can think of another place that does mix
sync_singlewith sgs.
Which one?
Re: [PATCH v6 06/13] nvme-pci: implement dma-buf backed requests
Quoted 19 lines · Christoph Hellwig
Quoted 10 lines · Pavel Begunkov
Quoted 4 lines
Sashiko had a few comments, which I think are correct - if we use
dma_map_sgto map the data, we need to use thesync_sgAPIs to transfer ownership. That only matters on non-coherent architectures with MMU, but we need to get it right.I've seen that and fixed everything locally that should be fixed, apart from the sync. I wonder what we can do about that? I can somehow replace it with the sg variant for now, but sync'ing the entire possibly multi-GB mapping for, let's say, a 512B I/O, sounds not wise.
If we'd want to fix this for real we'd need to add an offset to the sync by sg methods. Or move away from scatterlists in dma-buf, given that they are a horrible API. All of these are bigger projects, though. So for now I think you'd want to do the sync all, and have the people who run high-performance
io_uringcode on non-coherent platforms suffer.
Quoted 4 lines
Quoted 2 lines
And I can think of another place that does mix
sync_singlewith sgs.Which one?
All the recent networking zero-copy / dma-buf apis.
Signature · 1 line
Pavel Begunkov
PATCH v6 7/13 nvme-pci: rename nvme_pci_sgl_set_data to nvme_pci_dma_iter_set_sgl
In reply to Pavel BegunkovFrom: Anuj Gupta <[email protected]>
Rename the blk_dma_iter based nvme_pci_sgl_set_data() to nvme_pci_dma_iter_set_sgl().
--- drivers/nvme/host/pci.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/nvme/host/pci.c +5 -5
@@ -1297,7 +1297,7 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,return BLK_STS_IOERR ; } -static void nvme_pci_sgl_set_data (struct nvme_sgl_desc *sge, +static void nvme_pci_dma_iter_set_sgl (struct nvme_sgl_desc *sge,struct blk_dma_iter *iter) { sge->addr =cpu_to_le64 (iter->addr);
@@ -1327,7 +1327,7 @@ static blk_status_t nvme_pci_setup_data_sgl(struct request *req, iod->cmd.common.flags =NVME_CMD_SGL_METABUF ;if (entries ==1 ||blk_rq_dma_map_coalesce (&iod->dma_state)) { -nvme_pci_sgl_set_data (&iod->cmd.common.dptr.sgl, iter); +nvme_pci_dma_iter_set_sgl (&iod->cmd.common.dptr.sgl, iter); iod->total_len += iter->len;return BLK_STS_OK ; }
@@ -1349,7 +1349,7 @@ static blk_status_t nvme_pci_setup_data_sgl(struct request *req, iter->status =BLK_STS_IOERR ;break ; } -nvme_pci_sgl_set_data (&sg_list[mapped++], iter); +nvme_pci_dma_iter_set_sgl (&sg_list[mapped++], iter); iod->total_len += iter->len; }while (blk_rq_dma_map_iter_next (req, nvmeq->dev->dev, iter));
@@ -1512,13 +1512,13 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req) iod->cmd.common.metadata =cpu_to_le64 (sgl_dma);if (entries ==1 ) { iod->meta_total_len = iter.len; -nvme_pci_sgl_set_data (sg_list, &iter); +nvme_pci_dma_iter_set_sgl (sg_list, &iter);return BLK_STS_OK ; } sgl_dma +=sizeof (*sg_list);do { -nvme_pci_sgl_set_data (&sg_list[++i], &iter); +nvme_pci_dma_iter_set_sgl (&sg_list[++i], &iter); iod->meta_total_len += iter.len; }while (blk_rq_integrity_dma_map_iter_next (req, dev->dev, &iter));
Signature · 1 line
2.54.0
PATCH v6 8/13 nvme-pci: add SGL support for the dmabuf path
In reply to Pavel BegunkovFrom: Anuj Gupta <[email protected]>
Add SGL support in addition to PRP for dmabuf-backed requests, building the descriptor list from the mapping's sg_table.
--- drivers/nvme/host/pci.c | 153 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 149 insertions(+), 4 deletions(-)
drivers/nvme/host/pci.c +149 -4
@@ -1297,12 +1297,18 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,return BLK_STS_IOERR ; } +static void nvme_pci_sgl_set_data (struct nvme_sgl_desc *sge, +dma_addr_t addr,u32 len) +{ + sge->addr =cpu_to_le64 (addr); + sge->length =cpu_to_le32 (len); + sge->type =NVME_SGL_FMT_DATA_DESC <<4 ; +} +static void nvme_pci_dma_iter_set_sgl (struct nvme_sgl_desc *sge,struct blk_dma_iter *iter) { - sge->addr =cpu_to_le64 (iter->addr); - sge->length =cpu_to_le32 (iter->len); - sge->type =NVME_SGL_FMT_DATA_DESC <<4 ; +nvme_pci_sgl_set_data (sge, iter->addr, iter->len); } static void nvme_pci_sgl_set_seg(struct nvme_sgl_desc *sge,
@@ -1313,6 +1319,145 @@ static void nvme_pci_sgl_set_seg(struct nvme_sgl_desc *sge, sge->type =NVME_SGL_FMT_LAST_SEG_DESC <<4 ; } +static unsigned int nvme_pci_dmabuf_sgl_nents (struct request *req) +{ +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); +struct scatterlist *sg; +unsigned long tmp; +size_t offset = bio->bi_iter.bi_offset; +size_t remaining =blk_rq_payload_bytes (req); +unsigned int nents =0 ; + +for_each_sgtable_dma_sg (map->sgt, sg, tmp) { +size_t sg_len =sg_dma_len (sg); + +if (!remaining) +break ; +if (offset >= sg_len) { + offset -= sg_len; +continue ; + } + + sg_len -= offset; + offset =0 ; + +do { +size_t chunk =min (remaining, sg_len); + + nents++; + sg_len -= chunk; + remaining -= chunk; + }while (sg_len && remaining); + } + +if (unlikely (remaining)) +return 0 ; + +return nents; +} + +static blk_status_t nvme_rq_setup_dmabuf_sgl (struct request *req, +struct nvme_queue *nvmeq,unsigned int entries) +{ +struct nvme_iod *iod =blk_mq_rq_to_pdu (req); +struct bio *bio = req->bio; +struct nvme_dmabuf_map *map =to_nvme_dmabuf_map (bio->bi_dmabuf_map); +size_t length =blk_rq_payload_bytes (req); +struct nvme_sgl_desc *sg_list = &iod->cmd.common.dptr.sgl; +bool pooled = entries >1 ; +dma_addr_t sgl_dma =0 ; +unsigned int mapped =0 ; +unsigned long tmp; +struct scatterlist *sg; +size_t offset = bio->bi_iter.bi_offset; +size_t remaining = length; + +if (!entries) +return BLK_STS_IOERR ; + + iod->cmd.common.flags =NVME_CMD_SGL_METABUF ; + iod->total_len = length; + +nvme_dmabuf_map_sync_for_device (nvmeq->dev, req); + +/* entries == 1 fits in the inline descriptor; more needs a pool. */ +if (pooled) { +if (entries <=NVME_SMALL_POOL_SIZE /sizeof (*sg_list)) + iod->flags |=IOD_SMALL_DESCRIPTOR ; + + sg_list =dma_pool_alloc (nvme_dma_pool (nvmeq, iod),GFP_ATOMIC , + &sgl_dma); +if (!sg_list) +return BLK_STS_RESOURCE ; + iod->descriptors[iod->nr_descriptors++] = sg_list; + } + +for_each_sgtable_dma_sg (map->sgt, sg, tmp) { +size_t sg_len =sg_dma_len (sg); +dma_addr_t addr =sg_dma_address (sg); + +if (!remaining) +break ; +if (offset >= sg_len) { + offset -= sg_len; +continue ; + } + + addr += offset; + sg_len -= offset; + offset =0 ; + +do { +u32 chunk =min_t (size_t, remaining, sg_len); + +if (WARN_ON_ONCE (mapped == entries)) + goto err_free; +nvme_pci_sgl_set_data (&sg_list[mapped++], addr, chunk); + + addr += chunk; + sg_len -= chunk; + remaining -= chunk; + }while (sg_len && remaining); + } + +if (unlikely (remaining)) + goto err_free; + +if (pooled) +nvme_pci_sgl_set_seg (&iod->cmd.common.dptr.sgl, sgl_dma, + mapped); +return BLK_STS_OK ; +err_free: +if (pooled) { + iod->nr_descriptors--; +dma_pool_free (nvme_dma_pool (nvmeq, iod), sg_list, sgl_dma); + } +return BLK_STS_IOERR ; +} + +static blk_status_t nvme_rq_setup_dmabuf (struct request *req, +struct nvme_queue *nvmeq,enum nvme_use_sgl use_sgl) +{ +unsigned int entries; +size_t avg_seg; + +if (use_sgl ==SGL_UNSUPPORTED ) +return nvme_rq_setup_dmabuf_map (req, nvmeq); + + entries =nvme_pci_dmabuf_sgl_nents (req); + +if (use_sgl ==SGL_FORCED ) +return nvme_rq_setup_dmabuf_sgl (req, nvmeq, entries); + + avg_seg = entries ? +DIV_ROUND_UP (blk_rq_payload_bytes (req), entries) :0 ; +if (sgl_threshold && avg_seg >= sgl_threshold) +return nvme_rq_setup_dmabuf_sgl (req, nvmeq, entries); + +return nvme_rq_setup_dmabuf_map (req, nvmeq); +} +static blk_status_t nvme_pci_setup_data_sgl (struct request *req,struct blk_dma_iter *iter) {
@@ -1408,7 +1553,7 @@ static blk_status_t nvme_map_data(struct request *req)blk_status_t ret;if (blk_mq_rq_is_dmabuf (req)) -return nvme_rq_setup_dmabuf_map (req, nvmeq); +return nvme_rq_setup_dmabuf (req, nvmeq, use_sgl);/* * Try to skip the DMA iterator for single segment requests, as that
Signature · 1 line
2.54.0
PATCH v6 9/13 io_uring/rsrc: introduce buf registration structure
In reply to Pavel BegunkovIn preparation to following changes, instead of passing an iovec for buffer registration introduce a new structure. It'll be moved to uapi later, but for now it's initialised early from a user provided iovec.
--- io_uring/rsrc.c | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-)
io_uring/rsrc.c +32 -15
@@ -27,8 +27,13 @@ struct io_rsrc_update {u32 offset; }; +struct io_uring_regbuf_desc { +__u64 uaddr; +__u64 size; +}; +static struct io_rsrc_node *io_sqe_buffer_register (struct io_ring_ctx *ctx, -struct iovec *iov); +struct io_uring_regbuf_desc *desc);static int hpage_acct_ref (struct io_ring_ctx *ctx,struct page *hpage,bool *acct_new)
@@ -81,6 +86,15 @@ static bool hpage_acct_unref(struct io_ring_ctx *ctx, struct page *hpage)#define IO_CACHED_BVECS_SEGS 32 +static void io_iov_to_regbuf_desc (const struct iovec *iov, +struct io_uring_regbuf_desc *desc) +{ + *desc = (struct io_uring_regbuf_desc ) { + .uaddr = (u64 )(uintptr_t )iov->iov_base, + .size = iov->iov_len, + }; +} +int __io_account_mem (struct user_struct *user,unsigned long nr_pages) {unsigned long page_limit, cur_pages, new_pages;
@@ -381,6 +395,7 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx,return -EINVAL ;for (done =0 ; done < nr_args; done++) { +struct io_uring_regbuf_desc desc;struct io_rsrc_node *node;u64 tag =0 ;
@@ -394,7 +409,9 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx, err = -EFAULT ;break ; } - node =io_sqe_buffer_register (ctx, iov); + +io_iov_to_regbuf_desc (iov, &desc); + node =io_sqe_buffer_register (ctx, &desc);if (IS_ERR (node)) { err =PTR_ERR (node);break ;
@@ -853,26 +870,26 @@ bool io_check_coalesce_buffer(struct page **page_array, int nr_pages, }static struct io_rsrc_node *io_sqe_buffer_register (struct io_ring_ctx *ctx, -struct iovec *iov) +struct io_uring_regbuf_desc *desc) { +unsigned long uaddr = (unsigned long )desc->uaddr; +size_t size = desc->size;struct io_mapped_ubuf *imu =NULL ;struct page **pages =NULL ;struct io_rsrc_node *node;unsigned long off; -size_t size;int ret, nr_pages, i;struct io_imu_folio_data data;bool coalesced = false; -if (!iov->iov_base) { -if (iov->iov_len) +if (!uaddr) { +if (size)return ERR_PTR (-EFAULT );/* remove the buffer without installing a new one */ return NULL ; } - ret =io_validate_user_buf_range ((unsigned long )iov->iov_base, - iov->iov_len); + ret =io_validate_user_buf_range (uaddr, size);if (ret)return ERR_PTR (ret);
@@ -881,8 +898,7 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx,return ERR_PTR (-ENOMEM ); ret = -ENOMEM ; - pages =io_pin_pages ((unsigned long ) iov->iov_base, iov->iov_len, - &nr_pages); + pages =io_pin_pages (uaddr, size, &nr_pages);if (IS_ERR (pages)) { ret =PTR_ERR (pages); pages =NULL ;
@@ -904,10 +920,9 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx,if (ret) goto done; - size = iov->iov_len;/* store original address for later verification */ - imu->ubuf = (unsigned long ) iov->iov_base; - imu->len = iov->iov_len; + imu->ubuf = uaddr; + imu->len = size; imu->folio_shift =PAGE_SHIFT ; imu->release = io_release_ubuf; imu->priv = imu;
@@ -917,7 +932,7 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx, imu->folio_shift = data.folio_shift;refcount_set (&imu->refs,1 ); - off = (unsigned long )iov->iov_base & ~PAGE_MASK ; + off = uaddr & ~PAGE_MASK ;if (coalesced) off += data.first_folio_page_idx <<PAGE_SHIFT ;
@@ -969,6 +984,7 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,memset (iov,0 ,sizeof (*iov));for (i =0 ; i < nr_args; i++) { +struct io_uring_regbuf_desc desc;struct io_rsrc_node *node;u64 tag =0 ;
@@ -992,7 +1008,8 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg, } } - node =io_sqe_buffer_register (ctx, iov); +io_iov_to_regbuf_desc (iov, &desc); + node =io_sqe_buffer_register (ctx, &desc);if (IS_ERR (node)) { ret =PTR_ERR (node);break ;
Signature · 1 line
2.54.0
PATCH v6 10/13 io_uring/rsrc: extend buffer update
In reply to Pavel BegunkovWe need to pass more information to buffer registration than we can fit into a single struct iovec. This patch allows users to optionally pass struct io_uring_regbuf_desc. Apart from having more space for future use cases, it also introduces registration types.
Currently, the type can be either of IO_REGBUF_TYPE_UADDR, which mirrors the iovec path, or IO_REGBUF_TYPE_EMPTY for leaving a buffer table slot empty. The next patch introduces a dmabuf backed type, and can be useful for other extensions like splicing a list of user addresses (i.e. iovec[]), interoperability with zcrx, kernel allocated memory like was brough up by Cristoph. Note, the type only represents a registration option, which is distinct from how io_uring internally stores it.
The flags field is not used yet but always useful to have, e.g. we can encode read-only / write-only restrictions using it.
--- include/uapi/linux/io_uring.h | 27 +++++++++++++- io_uring/rsrc.c | 69 ++++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 27 deletions(-)
include/uapi/linux/io_uring.h +26 -1
@@ -790,13 +790,38 @@ struct io_uring_rsrc_update {struct io_uring_rsrc_update2 {__u32 offset; -__u32 resv; +__u32 flags;__aligned_u64 data;__aligned_u64 tags;__u32 nr;__u32 resv2; }; +/* struct io_uring_rsrc_update2::flags */ +enum io_uring_rsrc_reg_flags { +/* +* Use the extended descriptor format for buffer updates, +* see struct io_uring_regbuf_desc +*/ +IORING_RSRC_UPDATE_EXTENDED =1U <<1 , +}; + +/* Buffer registration type, passed in struct io_uring_regbuf_desc::type */ +enum io_uring_regbuf_type { +IO_REGBUF_TYPE_EMPTY , +IO_REGBUF_TYPE_UADDR , + + __IO_REGBUF_TYPE_MAX, +}; + +struct io_uring_regbuf_desc { +__u32 type;/* enum io_uring_regbuf_type */ +__u32 flags; +__u64 size; +__u64 uaddr; +__u64 __resv[7 ]; +}; +/* Skip updating fd indexes set to this value in the fd table */ #define IORING_REGISTER_FILES_SKIP (-2)
io_uring/rsrc.c +43 -26
151 lines not on this page · raw message
Signature · 1 line
2.54.0
PATCH v6 11/13 io_uring/rsrc: add uncloneable regbuf flag
In reply to Pavel BegunkovIt's hard to implement cloning if the internal structure needs to be mutable and/or relies on other ring resources. In preparation to such buffer types, add a flag indicating that the buffer can't be cloned. It might be possible to add cloning in the future for them, but that would likely need reallocating the structure and reacquiring resources in case by case manner.
--- io_uring/rsrc.c | 5 +++++ io_uring/rsrc.h | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-)
io_uring/rsrc.c +5 -0
@@ -1434,6 +1434,11 @@ static int io_clone_buffers(struct io_ring_ctx *ctx, struct io_ring_ctx *src_ctxif (!src_node) { dst_node =NULL ; }else { +if (src_node->buf->flags &IO_REGBUF_F_UNCLONEABLE ) { +io_rsrc_data_free (ctx, &data); +return -ENOMEM ; + } + dst_node =io_rsrc_node_alloc (ctx,IORING_RSRC_BUFFER );if (!dst_node) {io_rsrc_data_free (ctx, &data);
io_uring/rsrc.h +2 -1
@@ -26,7 +26,8 @@ struct io_rsrc_node { };enum { -IO_REGBUF_F_KBUF =1 , +IO_REGBUF_F_KBUF =1 <<0 , +IO_REGBUF_F_UNCLONEABLE =1 <<1 , };struct io_mapped_ubuf {
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v6 11/13] io_uring/rsrc: add uncloneable regbuf flag
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] io_uring: incorrect error code -ENOMEM for uncloneable buffers --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] io_uring: incorrect error code -ENOMEM for uncloneable buffers --
commit 09226c4686b3c73a879519b5ef4315732d82b7f5
Author: Pavel Begunkov <[email protected]>
io_uring/rsrc: add uncloneable regbuf flag
It's hard to implement cloning if the internal structure needs to be mutable and/or relies on other ring resources. In preparation to such buffer types, add a flag indicating that the buffer can't be cloned.
io_uring/rsrc.c static int io_clone_buffers(struct io_ring_ctx *ctx, struct io_ring_ctx *src_ctx · line 1439
Quoted 11 lines
io_uring/rsrc.c +3
-0@@ -1434,6 +1434,11 @@ static int io_clone_buffers(struct io_ring_ctx *ctx, struct io_ring_ctx *src_ctxif (!src_node) { dst_node =NULL ; }else { +if (src_node->buf->flags &IO_REGBUF_F_UNCLONEABLE ) { +io_rsrc_data_free (ctx, &data); +return -ENOMEM ;
[Severity: Medium]
Is -ENOMEM the correct error code to return from io_clone_buffers() for this case? Returning -ENOMEM for an unsupported operation (cloning an explicitly uncloneable buffer) might be misleading, as it usually signifies a transient out-of-memory condition.
If a user registers an uncloneable buffer and subsequently attempts to clone the buffer table via the IORING_REGISTER_CLONE_BUFFERS operation, robust userspace applications encountering -ENOMEM might enter a back-off and retry loop. This could potentially lead to infinite stalls rather than aborting the unsupported operation. Would an error code like -EOPNOTSUPP or -EINVAL be more appropriate to indicate that the buffer type simply doesn't support cloning?
Quoted 5 lines
io_uring/rsrc.c +2
-0+ } + dst_node =io_rsrc_node_alloc (ctx,IORING_RSRC_BUFFER );if (!dst_node) {io_rsrc_data_free (ctx, &data);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
PATCH v6 12/13 io_uring/rsrc: add regbuf import flags
In reply to Pavel BegunkovWe'll have special registered buffer types that can't be used with all opcodes and need special handling. Add separate flags to control registered buffer import, which will be used to specify what kind of buffers the caller can handle.
--- io_uring/rsrc.c | 8 ++++---- io_uring/rsrc.h | 24 ++++++++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-)
io_uring/rsrc.c +4 -4
@@ -1298,9 +1298,9 @@ inline struct io_rsrc_node *io_find_buf_node(struct io_kiocb *req,return NULL ; } -int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +int __io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter,u64 buf_addr,size_t len,int ddir, -unsigned issue_flags) +unsigned issue_flags,unsigned import_flags) {struct io_rsrc_node *node;
@@ -1709,9 +1709,9 @@ static int io_kern_bvec_size(struct iovec *iov, unsigned nr_iovs,return 0 ; } -int io_import_reg_vec (int ddir,struct iov_iter *iter, +int __io_import_reg_vec (int ddir,struct iov_iter *iter,struct io_kiocb *req,struct iou_vec *vec, -unsigned nr_iovs,unsigned issue_flags) +unsigned nr_iovs,unsigned issue_flags,unsigned import_flags) {struct io_rsrc_node *node;struct io_mapped_ubuf *imu;
io_uring/rsrc.h +20 -4
@@ -62,12 +62,28 @@ int io_rsrc_data_alloc(struct io_rsrc_data *data, unsigned nr);struct io_rsrc_node *io_find_buf_node (struct io_kiocb *req,unsigned issue_flags); -int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +int __io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter,u64 buf_addr,size_t len,int ddir, -unsigned issue_flags); -int io_import_reg_vec (int ddir,struct iov_iter *iter, +unsigned issue_flags,unsigned import_flags); +int __io_import_reg_vec (int ddir,struct iov_iter *iter,struct io_kiocb *req,struct iou_vec *vec, -unsigned nr_iovs,unsigned issue_flags); +unsigned nr_iovs,unsigned issue_flags, +unsigned import_flags); + +static inline int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +u64 buf_addr,size_t len,int ddir, +unsigned issue_flags) +{ +return __io_import_reg_buf (req, iter, buf_addr, len, ddir, issue_flags,0 ); +} + +static inline int io_import_reg_vec (int ddir,struct iov_iter *iter, +struct io_kiocb *req,struct iou_vec *vec, +unsigned nr_iovs,unsigned issue_flags) +{ +return __io_import_reg_vec (ddir, iter, req, vec, nr_iovs, issue_flags,0 ); +} +int io_prep_reg_iovec (struct io_kiocb *req,struct iou_vec *iv,const struct iovec __user *uvec,size_t uvec_segs);
Signature · 1 line
2.54.0
PATCH v6 13/13 io_uring/rsrc: add dmabuf backed registered buffers
In reply to Pavel BegunkovImplement dmabuf backed registered buffers. To register them, the user should specify IO_REGBUF_TYPE_DMABUF for the regitration and pass the desired dmabuf fd and a file for which it should be registered.
From there, it can be used with io_uring read/write requests IORING_OP_{READ,WRITE}_FIXED) as normal. The requests should be issued against the file specified during registration, and otherwise they'll be failed. The user should also be prepared to handle spurious -EAGAIN by reissuing the request.
Internally, dmabuf registered buffers is an optin feature for io_uring request opcodes and they should pass a special flag on import to use it.
--- include/linux/io_uring_types.h | 5 + include/uapi/linux/io_uring.h | 6 +- io_uring/io_uring.c | 3 +- io_uring/rsrc.c | 161 ++++++++++++++++++++++++++++++++- io_uring/rsrc.h | 18 ++++ io_uring/rw.c | 6 +- 6 files changed, 191 insertions(+), 8 deletions(-)
include/linux/io_uring_types.h +5 -0
@@ -11,6 +11,7 @@struct iou_loop_params ;struct io_uring_bpf_ops ; +struct dma_buf_io_map ;enum {/*
@@ -610,6 +611,7 @@ enum {REQ_F_IMPORT_BUFFER_BIT ,REQ_F_SQE_COPIED_BIT ,REQ_F_IOPOLL_BIT , +REQ_F_DROP_DMABUF_BIT ,/* not a real bit, just to check we're not overflowing the space */ __REQ_F_LAST_BIT,
@@ -705,6 +707,8 @@ enum {REQ_F_SQE_COPIED =IO_REQ_FLAG (REQ_F_SQE_COPIED_BIT ),/* request must be iopolled to completion (set in ->issue()) */ REQ_F_IOPOLL =IO_REQ_FLAG (REQ_F_IOPOLL_BIT ), +/* there is a dma map attached to request that needs to be dropped */ + REQ_F_DROP_DMABUF = IO_REQ_FLAG(REQ_F_DROP_DMABUF_BIT), }; struct io_tw_req {
@@ -827,6 +831,7 @@ struct io_kiocb {/* custom credentials, valid IFF REQ_F_CREDS is set */ const struct cred *creds;struct io_wq_work work; +struct dma_buf_io_map *dmabuf_map;struct io_big_cqe {u64 extra1;
include/uapi/linux/io_uring.h +5 -1
@@ -810,6 +810,7 @@ enum io_uring_rsrc_reg_flags {enum io_uring_regbuf_type {IO_REGBUF_TYPE_EMPTY ,IO_REGBUF_TYPE_UADDR , +IO_REGBUF_TYPE_DMABUF , __IO_REGBUF_TYPE_MAX, };
@@ -819,7 +820,10 @@ struct io_uring_regbuf_desc {__u32 flags;__u64 size;__u64 uaddr; -__u64 __resv[7 ]; + +__s32 dmabuf_fd; +__s32 target_fd; +__u64 __resv[6 ]; };/* Skip updating fd indexes set to this value in the fd table */
io_uring/io_uring.c +2 -1
@@ -109,7 +109,7 @@#define IO_REQ_CLEAN_SLOW_FLAGS (REQ_F_REFCOUNT | IO_REQ_LINK_FLAGS | \ REQ_F_REISSUE | REQ_F_POLLED | \ - IO_REQ_CLEAN_FLAGS) + IO_REQ_CLEAN_FLAGS | REQ_F_DROP_DMABUF)#define IO_TCTX_REFS_CACHE_NR (1U << 10)
@@ -1134,6 +1134,7 @@ static void io_free_batch_list(struct io_ring_ctx *ctx,io_queue_next (req);if (unlikely (req->flags &IO_REQ_CLEAN_FLAGS ))io_clean_op (req); +io_req_drop_dmabuf (req); }io_put_file (req);io_req_put_rsrc_nodes (req);
io_uring/rsrc.c +157 -4
204 lines not on this page · raw message
io_uring/rsrc.h +18 -0
@@ -28,6 +28,11 @@ struct io_rsrc_node {enum {IO_REGBUF_F_KBUF =1 <<0 ,IO_REGBUF_F_UNCLONEABLE =1 <<1 , +IO_REGBUF_F_DMABUF =1 <<3 , +}; + +enum { +IO_REGBUF_IMPORT_ALLOW_DMABUF =1 <<1 , };struct io_mapped_ubuf {
@@ -170,4 +175,17 @@ static inline void io_alloc_cache_vec_kasan(struct iou_vec *iv) io_vec_free(iv); } +void io_drop_dmabuf_node (struct io_kiocb *req); + +static inline void io_req_drop_dmabuf (struct io_kiocb *req) +{ +if (!IS_ENABLED (CONFIG_DMA_SHARED_BUFFER )) +return ; +if (!(req->flags &REQ_F_DROP_DMABUF )) +return ; +if (WARN_ON_ONCE (!(req->flags &REQ_F_BUF_NODE ))) +return ; +io_drop_dmabuf_node (req); +} +#endif
io_uring/rw.c +4 -2
@@ -367,8 +367,8 @@ static int io_init_rw_fixed(struct io_kiocb *req, unsigned int issue_flags,if (io->bytes_done)return 0 ; - ret =io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, - issue_flags); + ret =__io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, + issue_flags,IO_REGBUF_IMPORT_ALLOW_DMABUF );iov_iter_save_state (&io->iter, &io->iter_state);return ret; }
@@ -583,6 +583,8 @@ static void io_complete_rw(struct kiocb *kiocb, long res) struct io_rw *rw = container_of(kiocb, struct io_rw, kiocb);struct io_kiocb *req =cmd_to_io_kiocb (rw); +io_req_drop_dmabuf (req); +/* ring owner may block in freeze_super() before task_work runs */ if (kiocb->ki_flags &IOCB_WRITE )io_req_end_write (req);
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v6 13/13] io_uring/rsrc: add dmabuf backed registered buffers
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] io_uring: memory leak of io_regbuf_dma in io_release_reg_dmabuf() --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] io_uring: memory leak of io_regbuf_dma in io_release_reg_dmabuf() --
commit 9778a7461fd88cbd1c8b6991ebea933808628a8d
Author: Pavel Begunkov <[email protected]>
io_uring/rsrc: add dmabuf backed registered buffers
This commit implements dmabuf-backed registered buffers, allowing them to be used with io_uring fixed read/write requests. It introduces a new buffer type IO_REGBUF_TYPE_DMABUF to set up a DMA map against a specific target file.
Quoted 4 lines
io_uring/rsrc.c +0
-0
[ ... ]
io_uring/rsrc.c bool io_check_coalesce_buffer(struct page **page_array, int nr_pages, · line 895
Quoted 6 lines
io_uring/rsrc.c +6
-0+static void io_release_reg_dmabuf (void *priv) +{ +struct io_regbuf_dma *db = priv; + +fput (db->target_file); +dma_buf_io_ctx_release (db->ctx);
[Severity: Medium]
Does this code leak the io_regbuf_dma structure?
The db structure is dynamically allocated via kzalloc() in io_register_dmabuf() and assigned to imu->priv. When a user unregisters a dmabuf-backed registered buffer, this release callback drops the file and context references, but doesn't seem to free the db structure itself. Should this include a call to kfree(db)?
Quoted 5 lines
io_uring/rsrc.c +5
-0+} + +static struct io_rsrc_node *io_register_dmabuf (struct io_ring_ctx *ctx, +struct io_uring_regbuf_desc *desc) +{
[ ... ]
Quoted 3 lines
io_uring/rsrc.c +3
-0+ regbuf =kzalloc (sizeof (*regbuf),GFP_KERNEL ); +if (!regbuf) + goto err;
[ ... ]
Quoted 8 lines
io_uring/rsrc.c +8
-0+ imu->release = io_release_reg_dmabuf; + imu->priv = regbuf; + imu->flags =IO_REGBUF_F_DMABUF ; + imu->dir =IO_BUF_DEST |IO_BUF_SOURCE ; +refcount_set (&imu->refs,1 ); + node->buf = imu; +dma_buf_put (dmabuf); +return node;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13