Add dmabuf read/write via io_uring

The method name feels a bit convoluted, but given all the previous discussions I don't care too strongly. I'll leave the dma-buf side review to those who understand it.

Quoted 3 lines · Christoph Hellwig

The method name feels a bit convoluted, but given all the previous discussions I don't care too strongly. I'll leave the dma-buf side review to those who understand it.

I assume you mean this:

include/linux/fs.h +1 -0

+	int (*init_dma_buf_io_ctx)(struct file *, struct dma_buf_io_ctx *);

I agree, and all dma_buf_io_[ctx,map] look clunky, but I don't see what I can drop out of the name. Suggestions? Maybe I at least should make the fs op sth like "register_dma_buf".

Signature · 2 lines

-- 
Pavel Begunkov

Quoted 6 lines · Pavel Begunkov

Quoted 3 lines · Christoph Hellwig

The method name feels a bit convoluted, but given all the previous discussions I don't care too strongly. I'll leave the dma-buf side review to those who understand it.

I assume you mean this:

Yes.

Quoted 3 lines

include/linux/fs.h +1 -0

+	int (*init_dma_buf_io_ctx)(struct file *, struct dma_buf_io_ctx *);

Quoted 3 lines

I agree, and all dma_buf_io_[ctx,map] look clunky, but I don't see what I can drop out of the name. Suggestions? Maybe I at least should make the fs op sth like "register_dma_buf".

I just remember scares from the last discussion :)

register_dma_buf sounds fine to be, but unless I misremember there were objections to that before.

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. Invalidation is delayed by inserting a dma fence, which is signaled when all flight requests are completed.

[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 | 275 +++++++++++++++++++++++++++++++++++
 include/linux/dma-buf-io.h   |  91 ++++++++++++
 include/linux/fs.h           |   2 +
 4 files changed, 369 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

index b25d7550bacf..523731b0f83e 100644
@@ -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 +3 -0

new file mode 100644
index 000000000000..5f545a1bcf32
@@ -0,0 +1,275 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common infrastructure for supporing dma-buf in the I/O path.

drivers/dma-buf/dma-buf-io.c new file +33 -0

+
+static const char *dma_buf_io_fence_drv_name(struct dma_fence *fence)
+{
+	/* default fence release kfree's the base pointer */
+	BUILD_BUG_ON(offsetof(struct dma_buf_io_fence, base));
+
+	return "dma-buf-io-ctx";
+}
+
+static const char *dma_buf_io_fence_timeline_name(struct dma_fence *fence)
+{
+	return "dma-buf-io-ctx";
+}
+
+const struct dma_fence_ops dma_buf_io_fence_ops = {
+	.get_driver_name = dma_buf_io_fence_drv_name,
+	.get_timeline_name = dma_buf_io_fence_timeline_name,
+};
+
+static void dma_buf_io_ctx_destroy_work(struct work_struct *work)
+{
+	struct dma_buf_io_ctx *ctx = container_of(work, struct dma_buf_io_ctx,
+						  release_work);
+
+	if (WARN_ON_ONCE(refcount_read(&ctx->refs)))
+		return;
+
+	ctx->dev_ops->release(ctx);
+	dma_buf_put(ctx->dmabuf);
+	kfree(ctx);
+}
+
+static void dma_buf_io_map_release_work(struct work_struct *work)

drivers/dma-buf/dma-buf-io.c new file +48 -0

+	percpu_ref_exit(&map->refs);
+	kfree(map);
+
+	if (refcount_dec_and_test(&ctx->refs)) {
+		/*
+		 * Destruction needs to wait for I/O and dma fences. Defer it to
+		 * simplify locking.
+		 */
+		INIT_WORK(&ctx->release_work, dma_buf_io_ctx_destroy_work);
+		queue_work(system_wq, &ctx->release_work);
+	}
+}
+
+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);
+
+	/* might sleep, use a worker */
+	INIT_WORK(&map->release_work, dma_buf_io_map_release_work);
+	queue_work(system_wq, &map->release_work);
+}
+
+int dma_buf_io_init_map(struct dma_buf_io_ctx *ctx, struct dma_buf_io_map *map)
+{
+	struct dma_buf_io_fence *fence = NULL;
+	int ret;
+
+	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
+	if (!fence)
+		return -ENOMEM;
+
+	ret = percpu_ref_init(&map->refs, dma_buf_io_map_refs_release, 0, GFP_KERNEL);
+	if (ret) {
+		kfree(fence);
+		return ret;
+	}
+
+	spin_lock_init(&fence->lock);
+	dma_fence_init(&fence->base, &dma_buf_io_fence_ops, &fence->lock,
+			ctx->fence_ctx, atomic_inc_return(&ctx->fence_seq));
+	map->fence = fence;
+	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)
+{

drivers/dma-buf/dma-buf-io.c new file +55 -0

+	ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
+				    true, MAX_SCHEDULE_TIMEOUT);
+	if (!ret)
+		ret = -EAGAIN;
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ret = dma_resv_lock_interruptible(dmabuf->resv, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+
+	map = dma_buf_io_get_map(ctx);
+	if (map) {
+		ret = 0;
+		goto out;
+	}
+
+	if (dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
+				  true, 0) < 0) {
+		dma_resv_unlock(dmabuf->resv);
+		goto retry;
+	}
+
+	map = ctx->dev_ops->map(ctx);
+	if (IS_ERR(map)) {
+		ret = PTR_ERR(map);
+		goto out;
+	}
+
+	percpu_ref_get(&map->refs);
+	rcu_assign_pointer(ctx->map, map);
+out:
+	dma_resv_unlock(dmabuf->resv);
+	if (ret < 0)
+		return ERR_PTR(ret);
+	return map;
+}
+
+static void dma_buf_io_drop_map(struct dma_buf_io_ctx *ctx)
+{
+	struct dma_buf *dmabuf = ctx->dmabuf;
+	struct dma_buf_io_map *map;
+	int ret;
+
+	dma_resv_assert_held(dmabuf->resv);
+
+	map = rcu_dereference_protected(ctx->map,
+					dma_resv_held(dmabuf->resv));
+	if (!map)
+		return;
+	rcu_assign_pointer(ctx->map, NULL);
+
 
 
+	ret = dma_resv_reserve_fences(dmabuf->resv, 1);
+	if (WARN_ON_ONCE(ret)) {
+		struct dma_fence *fence = &map->fence->base;

drivers/dma-buf/dma-buf-io.c new file +80 -0

+	/*
+	 * Delay destruction until all inflight requests using the map arefence->
+	 * gone. It'll also signal the fence then.
+	 */
+	percpu_ref_kill(&map->refs);
+}
+
+void dma_buf_io_invalidate_mappings(struct dma_buf_io_ctx *ctx)
+{
+	dma_buf_io_drop_map(ctx);
+}
+EXPORT_SYMBOL_NS_GPL(dma_buf_io_invalidate_mappings, "DMA_BUF");
+
+static void dma_buf_io_ctx_release_work(struct work_struct *work)
+{
+	struct dma_buf_io_ctx *ctx = container_of(work, struct dma_buf_io_ctx,
+						  release_work);
+	struct dma_buf *dmabuf = ctx->dmabuf;
+	long ret;
+
+	dma_resv_lock(dmabuf->resv, NULL);
+	/* Remove the last map, there should be no new ones going forward. */
+	dma_buf_io_drop_map(ctx);
+	dma_resv_unlock(dmabuf->resv);
+
+	/* Wait until all maps are destroyed. */
+	ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
+				    false, MAX_SCHEDULE_TIMEOUT);
+
+	if (WARN_ON_ONCE(ret <= 0))
+		return;
+	if (WARN_ON_ONCE(rcu_dereference_protected(ctx->map, true)))
+		return;
+
+	if (refcount_dec_and_test(&ctx->refs))
+		dma_buf_io_ctx_destroy_work(&ctx->release_work);
+}
+
+void dma_buf_io_ctx_release(struct dma_buf_io_ctx *ctx)
+{
+	/*
+	 * Destruction needs to wait for I/O and dma fences. Defer it to
+	 * simplify locking.
+	 */
+	INIT_WORK(&ctx->release_work, dma_buf_io_ctx_release_work);
+	queue_work(system_wq, &ctx->release_work);
+}
+
+int dma_buf_io_ctx_create(struct file *file,
+			   struct dma_buf_io_ctx *ctx,
+			   struct dma_buf *dmabuf,
+			   enum dma_data_direction dir)
+{
+	int ret;
+
+	if (!file->f_op->init_dma_buf_io_ctx)
+		return -EOPNOTSUPP;
+
+	memset(ctx, 0, sizeof(*ctx));
+	ctx->fence_ctx = dma_fence_context_alloc(1);
+	ctx->dir = dir;
+	ctx->dmabuf = dmabuf;
+	refcount_set(&ctx->refs, 1);
+	get_dma_buf(dmabuf);
+
+	ret = file->f_op->init_dma_buf_io_ctx(file, ctx);
+	if (ret) {
+		memset(ctx, 0, sizeof(*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;
+
+	return ret;
+}

include/linux/dma-buf-io.h new file +91 -0

new file mode 100644
index 000000000000..bcffc6202556
@@ -0,0 +1,91 @@
+/* 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_fence;
+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;
+
+	struct work_struct		release_work;
+	struct dma_buf_io_fence		*fence;
+	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;
+
+	atomic_t				fence_seq;
+	u64					fence_ctx;
+	struct work_struct			release_work;
+	refcount_t				refs;
+
+	void					*dev_priv;
+	const struct dma_buf_io_ops		*dev_ops;
+};
+
+int dma_buf_io_ctx_create(struct file *file,
+			   struct dma_buf_io_ctx *ctx,
+			   struct dma_buf *dmabuf,
+			   enum dma_data_direction dir);
+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 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);
+
+#endif

include/linux/fs.h +2 -0

index d10897b3a1e3..9fe349a279c7 100644
@@ -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;
 
 typedef unsigned int __bitwise fop_flags_t;

@@ -1962,6 +1963,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 */

drivers/dma-buf/dma-buf-io.c line 13

Quoted 3 lines

drivers/dma-buf/dma-buf-io.c new file +3 -0

+	struct dma_fence base;
+	spinlock_t lock;
+};

Upstream has change to allow embedding the spinlock into the dma_fence, so this structure here is most likely not necessary any more.

drivers/dma-buf/dma-buf-io.c line 21

Quoted 7 lines

drivers/dma-buf/dma-buf-io.c new file +7 -0

+static const char *dma_buf_io_fence_drv_name(struct dma_fence *fence)
+{
+	/* default fence release kfree's the base pointer */
+	BUILD_BUG_ON(offsetof(struct dma_buf_io_fence, base));
+
+	return "dma-buf-io-ctx";
+}

Quoted 4 lines

drivers/dma-buf/dma-buf-io.c new file +4 -0

+{
+	struct dma_buf_io_map *map = container_of(work, struct dma_buf_io_map,
+						  release_work);
+	struct dma_buf_io_fence *fence = map->fence;

drivers/dma-buf/dma-buf-io.c line 56

Quoted 6 lines

drivers/dma-buf/dma-buf-io.c new file +6 -0

+	struct dma_buf_io_ctx *ctx = map->ctx;
+	struct dma_buf *dmabuf = ctx->dmabuf;
+
+	/* the release path must wait for fences */
+	if (WARN_ON_ONCE(refcount_read(&ctx->refs) == 0))
+		return;

Stuff like that is usually illegal.

And why are you using refcount directly instead of kref?

drivers/dma-buf/dma-buf-io.c line 66

Quoted 12 lines

drivers/dma-buf/dma-buf-io.c new file +10 -0

+
+	/* Prevent from destoying the ctx while unmapping */
+	refcount_inc(&ctx->refs);
 
+
+	/*
+	 * There are no more requests using the map, we can signal the fence.
+	 * It should be done before taking the resv lock as someone could be
+	 * waiting for the fence while holding the lock.
+	 */
+	dma_fence_signal(&fence->base);

Signaling fences has a whole bunch of very strict rules associated with it. E.g. you can't alocate memory for example.

Are you sure you actually need and want a dma_fence here?

drivers/dma-buf/dma-buf-io.c line 72

Quoted 6 lines

drivers/dma-buf/dma-buf-io.c new file +6 -0

+
+	dma_resv_lock(dmabuf->resv, NULL);
+	ctx->dev_ops->unmap(ctx, map);
+	dma_resv_unlock(dmabuf->resv);
+
+	dma_fence_put(&fence->base);

You should probably set map->fence to NULL after that.

drivers/dma-buf/dma-buf-io.c line 74

Quoted 2 lines

drivers/dma-buf/dma-buf-io.c new file +2 -0

+	percpu_ref_exit(&map->refs);
+	kfree(map);

drivers/dma-buf/dma-buf-io.c line 130

Quoted 11 lines

drivers/dma-buf/dma-buf-io.c new file +11 -0

+{
+	struct dma_buf *dmabuf = ctx->dmabuf;
+	struct dma_buf_io_map *map;
+	long ret;
+
+retry:
+	/*
+	 * ->dmabuf_map() will be calling dma_buf_map_attachment(), for which
+	 * we'll need to wait for fences. Do a bit nicer and try to wait
+	 * without the resv lock first.
+	 */

Clear NAK to that. Always wait while holding the resv lock if you can!

It is absolutely not beneficial to do this outside of the lock and usually just hides problems instead and prevent fixing them.

drivers/dma-buf/dma-buf-io.c line 195

Quoted 13 lines

drivers/dma-buf/dma-buf-io.c new file +13 -0

+	ret = dma_resv_reserve_fences(dmabuf->resv, 1);
+	if (WARN_ON_ONCE(ret)) {
+		struct dma_fence *fence = &map->fence->base;
+
+		dma_fence_get(fence);
+		percpu_ref_kill(&map->refs);
+		dma_fence_wait(fence, false);
+		dma_fence_put(fence);
+		return;
+	}
+
+	dma_resv_add_fence(dmabuf->resv, &map->fence->base,
+			   DMA_RESV_USAGE_KERNEL);

That sequence is clearly incorrect!

The fence must be created after dma_resv_reserve_fences(), otherwise you definately have an illegal memory operation here.

Quoted 8 lines · Christian König

drivers/dma-buf/dma-buf-io.c line 72

Quoted 6 lines

drivers/dma-buf/dma-buf-io.c new file +6 -0

+
+    dma_resv_lock(dmabuf->resv, NULL);
+    ctx->dev_ops->unmap(ctx, map);
+    dma_resv_unlock(dmabuf->resv);
+
+    dma_fence_put(&fence->base);

You should probably set map->fence to NULL after that.

The map is freed two lines below, but I can add it as a defensive measure.

...

Quoted 16 lines

drivers/dma-buf/dma-buf-io.c line 195

Quoted 13 lines

+    ret = dma_resv_reserve_fences(dmabuf->resv, 1); +    if (WARN_ON_ONCE(ret)) {
+        struct dma_fence *fence = &map->fence->base;

drivers/dma-buf/dma-buf-io.c new file +10 -0

+
+        dma_fence_get(fence);
+        percpu_ref_kill(&map->refs);
+        dma_fence_wait(fence, false);
+        dma_fence_put(fence);
+        return;
+    }
+
+    dma_resv_add_fence(dmabuf->resv, &map->fence->base,
+               DMA_RESV_USAGE_KERNEL);

That sequence is clearly incorrect!

Quoted 1 line

The fence must be created after dma_resv_reserve_fences(), otherwise you definately have an illegal memory operation here.

I'm not sure what you mean, can you elaborate? I only cared about pre-allocating it to avoid allocations here. We add / signal the fence only once, no reuse. The map is going to be killed here, and if we create a new map, it'll have its own fence.

I can move the dma_fence_init() call here if that makes a difference?

Quoted 11 lines · Pavel Begunkov

Quoted 8 lines · Christian König

drivers/dma-buf/dma-buf-io.c line 72

Quoted 6 lines

drivers/dma-buf/dma-buf-io.c new file +6 -0

+
+    dma_resv_lock(dmabuf->resv, NULL);
+    ctx->dev_ops->unmap(ctx, map);
+    dma_resv_unlock(dmabuf->resv);
+
+    dma_fence_put(&fence->base);

You should probably set map->fence to NULL after that.

Quoted 2 lines

The map is freed two lines below, but I can add it as a defensive measure.

In that case it's ok, I've just haven't seen the kfree(map) below.

Quoted 17 lines

...

Quoted 16 lines

drivers/dma-buf/dma-buf-io.c line 195

Quoted 13 lines

+    ret = dma_resv_reserve_fences(dmabuf->resv, 1); +    if (WARN_ON_ONCE(ret)) {
+        struct dma_fence *fence = &map->fence->base;

drivers/dma-buf/dma-buf-io.c new file +10 -0

+
+        dma_fence_get(fence);
+        percpu_ref_kill(&map->refs);
+        dma_fence_wait(fence, false);
+        dma_fence_put(fence);
+        return;
+    }
+
+    dma_resv_add_fence(dmabuf->resv, &map->fence->base,
+               DMA_RESV_USAGE_KERNEL);

That sequence is clearly incorrect!

Quoted 8 lines

Quoted 1 line

The fence must be created after dma_resv_reserve_fences(), otherwise you definately have an illegal memory operation here.

I'm not sure what you mean, can you elaborate? I only cared about pre-allocating it to avoid allocations here. We add / signal the fence only once, no reuse. The map is going to be killed here, and if we create a new map, it'll have its own fence.

I can move the dma_fence_init() call here if that makes a difference?

Yeah that is a good start, but you might need a bit more.

Here is a summary of the usual procedure you need to follow when implementing a dma_fence backend:

1. Allocate your operation object, in this case here it's your mapping I think.
2. Prepare your operation, including all memory allocations.
3. Call dma_resv_reserve_fences() to reserve a fence slot.
4. Allocate and init your dma_fence object.

Quoted 63 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_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. Invalidation is delayed by inserting a dma fence, which is signaled when all flight requests are completed.

[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 | 275 +++++++++++++++++++++++++++++++++++
 include/linux/dma-buf-io.h   |  91 ++++++++++++
 include/linux/fs.h           |   2 +
 4 files changed, 369 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

index b25d7550bacf..523731b0f83e 100644
@@ -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 +3 -0

new file mode 100644
index 000000000000..5f545a1bcf32
@@ -0,0 +1,275 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common infrastructure for supporing dma-buf in the I/O path.

drivers/dma-buf/dma-buf-io.c line 13

Quoted 10 lines

drivers/dma-buf/dma-buf-io.c new file +10 -0

+ *
+ * Copyright (C) 2026 Pavel Begunkov <[email protected]>
+ */
+#include <linux/dma-buf-io.h>
+#include <linux/dma-resv.h>
+
+struct dma_buf_io_fence {
+	struct dma_fence base;
+	spinlock_t lock;
+};

Upstream has change to allow embedding the spinlock into the dma_fence, so this structure here is most likely not necessary any more.

Quoted 33 lines

drivers/dma-buf/dma-buf-io.c new file +33 -0

+
+static const char *dma_buf_io_fence_drv_name(struct dma_fence *fence)
+{
+	/* default fence release kfree's the base pointer */
+	BUILD_BUG_ON(offsetof(struct dma_buf_io_fence, base));
+
+	return "dma-buf-io-ctx";
+}
+
+static const char *dma_buf_io_fence_timeline_name(struct dma_fence *fence)
+{
+	return "dma-buf-io-ctx";
+}
+
+const struct dma_fence_ops dma_buf_io_fence_ops = {
+	.get_driver_name = dma_buf_io_fence_drv_name,
+	.get_timeline_name = dma_buf_io_fence_timeline_name,
+};
+
+static void dma_buf_io_ctx_destroy_work(struct work_struct *work)
+{
+	struct dma_buf_io_ctx *ctx = container_of(work, struct dma_buf_io_ctx,
+						  release_work);
+
+	if (WARN_ON_ONCE(refcount_read(&ctx->refs)))
+		return;
+
+	ctx->dev_ops->release(ctx);
+	dma_buf_put(ctx->dmabuf);
+	kfree(ctx);
+}
+
+static void dma_buf_io_map_release_work(struct work_struct *work)

drivers/dma-buf/dma-buf-io.c line 56

Quoted 10 lines

drivers/dma-buf/dma-buf-io.c new file +10 -0

+{
+	struct dma_buf_io_map *map = container_of(work, struct dma_buf_io_map,
+						  release_work);
+	struct dma_buf_io_fence *fence = map->fence;
+	struct dma_buf_io_ctx *ctx = map->ctx;
+	struct dma_buf *dmabuf = ctx->dmabuf;
+
+	/* the release path must wait for fences */
+	if (WARN_ON_ONCE(refcount_read(&ctx->refs) == 0))
+		return;

Stuff like that is usually illegal.

And why are you using refcount directly instead of kref?

Quoted 3 lines

drivers/dma-buf/dma-buf-io.c new file +3 -0

+
+	/* Prevent from destoying the ctx while unmapping */
+	refcount_inc(&ctx->refs);

drivers/dma-buf/dma-buf-io.c line 66

Quoted 7 lines

drivers/dma-buf/dma-buf-io.c new file +7 -0

+
+	/*
+	 * There are no more requests using the map, we can signal the fence.
+	 * It should be done before taking the resv lock as someone could be
+	 * waiting for the fence while holding the lock.
+	 */
+	dma_fence_signal(&fence->base);

Signaling fences has a whole bunch of very strict rules associated with it. E.g. you can't alocate memory for example.

Are you sure you actually need and want a dma_fence here?

drivers/dma-buf/dma-buf-io.c line 68

Quoted 2 lines

drivers/dma-buf/dma-buf-io.c new file +2 -0

+
+	dma_resv_lock(dmabuf->resv, NULL);

drivers/dma-buf/dma-buf-io.c line 72

Quoted 4 lines

drivers/dma-buf/dma-buf-io.c new file +4 -0

+	ctx->dev_ops->unmap(ctx, map);
+	dma_resv_unlock(dmabuf->resv);
+
+	dma_fence_put(&fence->base);

You should probably set map->fence to NULL after that.

Quoted 12 lines

drivers/dma-buf/dma-buf-io.c new file +12 -0

+	percpu_ref_exit(&map->refs);
+	kfree(map);
+
+	if (refcount_dec_and_test(&ctx->refs)) {
+		/*
+		 * Destruction needs to wait for I/O and dma fences. Defer it to
+		 * simplify locking.
+		 */
+		INIT_WORK(&ctx->release_work, dma_buf_io_ctx_destroy_work);
+		queue_work(system_wq, &ctx->release_work);
+	}
+}

drivers/dma-buf/dma-buf-io.c line 92

Quoted 8 lines

drivers/dma-buf/dma-buf-io.c new file +8 -0

+
+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);
+
+	/* might sleep, use a worker */
+	INIT_WORK(&map->release_work, dma_buf_io_map_release_work);
+	queue_work(system_wq, &map->release_work);

Quoted 28 lines

drivers/dma-buf/dma-buf-io.c new file +28 -0

+}
+
+int dma_buf_io_init_map(struct dma_buf_io_ctx *ctx, struct dma_buf_io_map *map)
+{
+	struct dma_buf_io_fence *fence = NULL;
+	int ret;
+
+	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
+	if (!fence)
+		return -ENOMEM;
+
+	ret = percpu_ref_init(&map->refs, dma_buf_io_map_refs_release, 0, GFP_KERNEL);
+	if (ret) {
+		kfree(fence);
+		return ret;
+	}
+
+	spin_lock_init(&fence->lock);
+	dma_fence_init(&fence->base, &dma_buf_io_fence_ops, &fence->lock,
+			ctx->fence_ctx, atomic_inc_return(&ctx->fence_seq));
+	map->fence = fence;
+	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)
+{

drivers/dma-buf/dma-buf-io.c line 130

Quoted 10 lines

drivers/dma-buf/dma-buf-io.c new file +10 -0

+	struct dma_buf *dmabuf = ctx->dmabuf;
+	struct dma_buf_io_map *map;
+	long ret;
+
+retry:
+	/*
+	 * ->dmabuf_map() will be calling dma_buf_map_attachment(), for which
+	 * we'll need to wait for fences. Do a bit nicer and try to wait
+	 * without the resv lock first.
+	 */

Clear NAK to that. Always wait while holding the resv lock if you can!

It is absolutely not beneficial to do this outside of the lock and usually just hides problems instead and prevent fixing them.

Quoted 57 lines

drivers/dma-buf/dma-buf-io.c new file +55 -0

+	ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
+				    true, MAX_SCHEDULE_TIMEOUT);
+	if (!ret)
+		ret = -EAGAIN;
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ret = dma_resv_lock_interruptible(dmabuf->resv, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+
+	map = dma_buf_io_get_map(ctx);
+	if (map) {
+		ret = 0;
+		goto out;
+	}
+
+	if (dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
+				  true, 0) < 0) {
+		dma_resv_unlock(dmabuf->resv);
+		goto retry;
+	}
+
+	map = ctx->dev_ops->map(ctx);
+	if (IS_ERR(map)) {
+		ret = PTR_ERR(map);
+		goto out;
+	}
+
+	percpu_ref_get(&map->refs);
+	rcu_assign_pointer(ctx->map, map);
+out:
+	dma_resv_unlock(dmabuf->resv);
+	if (ret < 0)
+		return ERR_PTR(ret);
+	return map;
+}
+
+static void dma_buf_io_drop_map(struct dma_buf_io_ctx *ctx)
+{
+	struct dma_buf *dmabuf = ctx->dmabuf;
+	struct dma_buf_io_map *map;
+	int ret;
+
+	dma_resv_assert_held(dmabuf->resv);
+
+	map = rcu_dereference_protected(ctx->map,
+					dma_resv_held(dmabuf->resv));
+	if (!map)
+		return;
+	rcu_assign_pointer(ctx->map, NULL);
+
 
 
+	ret = dma_resv_reserve_fences(dmabuf->resv, 1);
+	if (WARN_ON_ONCE(ret)) {
+		struct dma_fence *fence = &map->fence->base;

drivers/dma-buf/dma-buf-io.c line 195

Quoted 10 lines

drivers/dma-buf/dma-buf-io.c new file +10 -0

+
+		dma_fence_get(fence);
+		percpu_ref_kill(&map->refs);
+		dma_fence_wait(fence, false);
+		dma_fence_put(fence);
+		return;
+	}
+
+	dma_resv_add_fence(dmabuf->resv, &map->fence->base,
+			   DMA_RESV_USAGE_KERNEL);

That sequence is clearly incorrect!

The fence must be created after dma_resv_reserve_fences(), otherwise you definately have an illegal memory operation here.

Regards,
Christian.

Quoted 190 lines

drivers/dma-buf/dma-buf-io.c new file +80 -0

+	/*
+	 * Delay destruction until all inflight requests using the map arefence->
+	 * gone. It'll also signal the fence then.
+	 */
+	percpu_ref_kill(&map->refs);
+}
+
+void dma_buf_io_invalidate_mappings(struct dma_buf_io_ctx *ctx)
+{
+	dma_buf_io_drop_map(ctx);
+}
+EXPORT_SYMBOL_NS_GPL(dma_buf_io_invalidate_mappings, "DMA_BUF");
+
+static void dma_buf_io_ctx_release_work(struct work_struct *work)
+{
+	struct dma_buf_io_ctx *ctx = container_of(work, struct dma_buf_io_ctx,
+						  release_work);
+	struct dma_buf *dmabuf = ctx->dmabuf;
+	long ret;
+
+	dma_resv_lock(dmabuf->resv, NULL);
+	/* Remove the last map, there should be no new ones going forward. */
+	dma_buf_io_drop_map(ctx);
+	dma_resv_unlock(dmabuf->resv);
+
+	/* Wait until all maps are destroyed. */
+	ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
+				    false, MAX_SCHEDULE_TIMEOUT);
+
+	if (WARN_ON_ONCE(ret <= 0))
+		return;
+	if (WARN_ON_ONCE(rcu_dereference_protected(ctx->map, true)))
+		return;
+
+	if (refcount_dec_and_test(&ctx->refs))
+		dma_buf_io_ctx_destroy_work(&ctx->release_work);
+}
+
+void dma_buf_io_ctx_release(struct dma_buf_io_ctx *ctx)
+{
+	/*
+	 * Destruction needs to wait for I/O and dma fences. Defer it to
+	 * simplify locking.
+	 */
+	INIT_WORK(&ctx->release_work, dma_buf_io_ctx_release_work);
+	queue_work(system_wq, &ctx->release_work);
+}
+
+int dma_buf_io_ctx_create(struct file *file,
+			   struct dma_buf_io_ctx *ctx,
+			   struct dma_buf *dmabuf,
+			   enum dma_data_direction dir)
+{
+	int ret;
+
+	if (!file->f_op->init_dma_buf_io_ctx)
+		return -EOPNOTSUPP;
+
+	memset(ctx, 0, sizeof(*ctx));
+	ctx->fence_ctx = dma_fence_context_alloc(1);
+	ctx->dir = dir;
+	ctx->dmabuf = dmabuf;
+	refcount_set(&ctx->refs, 1);
+	get_dma_buf(dmabuf);
+
+	ret = file->f_op->init_dma_buf_io_ctx(file, ctx);
+	if (ret) {
+		memset(ctx, 0, sizeof(*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;
+
+	return ret;
+}

include/linux/dma-buf-io.h new file +91 -0

new file mode 100644
index 000000000000..bcffc6202556
@@ -0,0 +1,91 @@
+/* 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_fence;
+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;
+
+	struct work_struct		release_work;
+	struct dma_buf_io_fence		*fence;
+	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;
+
+	atomic_t				fence_seq;
+	u64					fence_ctx;
+	struct work_struct			release_work;
+	refcount_t				refs;
+
+	void					*dev_priv;
+	const struct dma_buf_io_ops		*dev_ops;
+};
+
+int dma_buf_io_ctx_create(struct file *file,
+			   struct dma_buf_io_ctx *ctx,
+			   struct dma_buf *dmabuf,
+			   enum dma_data_direction dir);
+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 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);
+
+#endif

include/linux/fs.h +1 -0

index d10897b3a1e3..9fe349a279c7 100644
@@ -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;
 
 typedef unsigned int __bitwise fop_flags_t;

include/linux/fs.h struct file_operations { · line 1969

Quoted 7 lines

include/linux/fs.h +1 -0

@@ -1962,6 +1963,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 */

_______________________________________________
Linaro-mm-sig mailing list -- [email protected] To unsubscribe send an email to [email protected]

Quoted 1 line · Christian König

On 7/28/26 23:29, Pavel Begunkov wrote:

Quoted 8 lines

drivers/dma-buf/dma-buf-io.c line 66

Quoted 5 lines

drivers/dma-buf/dma-buf-io.c new file +5 -0

+	 * There are no more requests using the map, we can signal the fence.
+	 * It should be done before taking the resv lock as someone could be
+	 * waiting for the fence while holding the lock.
+	 */
+	dma_fence_signal(&fence->base);

Signaling fences has a whole bunch of very strict rules associated with it. E.g. you can't alocate memory for example.

Yes, and the rules around signaling fences from worker threads become interesting as well. In practice, the entire workqueue (or any work item scheduled on that workqueue) effectively becomes part of the fence signaling and reclaim path.

Quoted 4 lines

Are you sure you actually need and want a dma_fence here?

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+	dma_resv_lock(dmabuf->resv, NULL);

So this is illegal because code is allowed to hold dma-resv locks while waiting on dma-fences. See dma_resv_lockdep / __dma_fence_might_wait / dma_fence_begin_signalling.

Quoted 19 lines

drivers/dma-buf/dma-buf-io.c line 72

Quoted 4 lines

drivers/dma-buf/dma-buf-io.c new file +4 -0

+	ctx->dev_ops->unmap(ctx, map);
+	dma_resv_unlock(dmabuf->resv);
+
+	dma_fence_put(&fence->base);

You should probably set map->fence to NULL after that.

Quoted 12 lines

drivers/dma-buf/dma-buf-io.c new file +12 -0

+	percpu_ref_exit(&map->refs);
+	kfree(map);
+
+	if (refcount_dec_and_test(&ctx->refs)) {
+		/*
+		 * Destruction needs to wait for I/O and dma fences. Defer it to
+		 * simplify locking.
+		 */
+		INIT_WORK(&ctx->release_work, dma_buf_io_ctx_destroy_work);
+		queue_work(system_wq, &ctx->release_work);
+	}
+}

Quoted 8 lines

drivers/dma-buf/dma-buf-io.c line 92

Quoted 8 lines

drivers/dma-buf/dma-buf-io.c new file +8 -0

+
+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);
+
+	/* might sleep, use a worker */
+	INIT_WORK(&map->release_work, dma_buf_io_map_release_work);
+	queue_work(system_wq, &map->release_work);

You can't guarantee that a GFP_KERNEL allocation won't be performed from a system worker thread, so you can't safely signal a fence from one. The pathological case is when all threads in system_wq are running work items that perform GFP_KERNEL allocations, enter reclaim, and then wait on a fence that is signaled by another work item queued on system_wq. Since all worker threads are occupied, the signaling work item cannot be scheduled, resulting in a deadlock. We actually hit this exact deadlock early in Xe.

So, as Christian says, think carefully about whether you really need a dma-fence here. If you do, then you need to play by the rules.

Introduce 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.

Suggested-by Keith Busch <[email protected]>

Signed-off-by Pavel Begunkov <[email protected]>

Quoted 7 lines · Pavel Begunkov

Introduce 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.

Quoted 2 lines

Suggested-by Keith Busch <[email protected]>

Signed-off-by Pavel Begunkov <[email protected]>

Looks good:

Reviewed-by Christoph Hellwig <[email protected]>

I'm also pretty sure I reviewed this and vairous other patches before..

Review

Quoted 7 lines

(quoted diff) +6 -1

-	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;
+	}

No need for the braces.

even bother doing both sides and not rely on the union?

Review

Quoted 9 lines

(quoted diff) +9 -0

+void bio_dmabuf_map_set(struct bio *bio, struct iov_iter *iter)
+{
+	WARN_ON_ONCE(bio->bi_max_vecs);
+
+	bio->bi_dmabuf_map = iter->dmabuf_map;
+	bio->bi_vcnt = 0;
+	bio->bi_iter.bi_offset = iter->iov_offset;
+	bio->bi_iter.bi_size = iov_iter_count(iter);
+	bio->bi_opf |= REQ_NOMERGE | REQ_DMABUF;

This seems to be largely copied from bio_iov_bvec_set, but misses the REQ_CLONE there that we should probably set as well to indicate that the data descriptor is not owned (although not setting it is not really a bug).

What about merging these two into a single and easier to use interface like this:

block/bio.c +11 -25

index cc2bb9183c1a..25393bdd119d 100644
@@ -860,12 +860,9 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp)
  	bio->bi_write_hint = bio_src->bi_write_hint;
  	bio->bi_write_stream = bio_src->bi_write_stream;
  	bio->bi_iter = bio_src->bi_iter;
-
-	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;
-	}
+	static_assert(offsetof(struct bio, bi_io_vec) ==
+		      offsetof(struct bio, bi_dmabuf_map));
+	bio->bi_io_vec = bio_src->bi_io_vec;
 
  	if (bio->bi_bdev) {
  		if (bio->bi_bdev == bio_src->bi_bdev &&

@@ -1186,26 +1183,21 @@ void __bio_release_pages(struct bio *bio, bool mark_dirty)
  }
  EXPORT_SYMBOL_GPL(__bio_release_pages);
 
-void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)
+bool bio_iov_iter_set(struct bio *bio, const struct iov_iter *iter)
  {
  	WARN_ON_ONCE(bio->bi_max_vecs);
 
+	if (!iov_iter_is_bvec(iter) && !iov_iter_is_dmabuf_map(iter))
+		return false;
+
  	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);
-}
-
-void bio_dmabuf_map_set(struct bio *bio, struct iov_iter *iter)
-{
-	WARN_ON_ONCE(bio->bi_max_vecs);
-
-	bio->bi_dmabuf_map = iter->dmabuf_map;
-	bio->bi_vcnt = 0;
-	bio->bi_iter.bi_offset = iter->iov_offset;
-	bio->bi_iter.bi_size = iov_iter_count(iter);
-	bio->bi_opf |= REQ_NOMERGE | REQ_DMABUF;
+	if (iov_iter_is_dmabuf_map(iter))
+		bio->bi_opf |= REQ_NOMERGE | REQ_DMABUF;
+	return true;
  }
 
  /*

@@ -1265,13 +1257,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
  	if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
  		return -EIO;
 
-	if (iov_iter_is_bvec(iter)) {
-		bio_iov_bvec_set(bio, iter);
-		iov_iter_advance(iter, bio->bi_iter.bi_size);
-		return 0;
-	}
-	if (iov_iter_is_dmabuf_map(iter)) {
-		bio_dmabuf_map_set(bio, iter);
+	if (bio_iov_iter_set(bio, iter)) {
  		iov_iter_advance(iter, bio->bi_iter.bi_size);
  		return 0;
  	}

block/blk-map.c +1 -1

index d1d6bbe0ecf1..34816e6e64de 100644
@@ -473,7 +473,7 @@ static int blk_rq_map_user_bvec(struct request *rq, const struct iov_iter *iter)
  	bio = blk_rq_map_bio_alloc(rq, 0, GFP_KERNEL);
  	if (!bio)
  		return -ENOMEM;
-	bio_iov_bvec_set(bio, iter);
+	bio_iov_iter_set(bio, iter);
 
  	ret = blk_rq_append_bio(rq, bio);
  	if (ret)

block/fops.c +6 -11

index d83cdbab65a6..97fb124a19b2 100644
@@ -340,17 +340,12 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
  	bio->bi_end_io = blkdev_bio_end_io_async;
  	bio->bi_ioprio = iocb->ki_ioprio;
 
-	if (iov_iter_is_bvec(iter)) {
-		/*
-		 * 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_bvec_set(bio, iter);
-	} else if (iov_iter_is_dmabuf_map(iter)) {
-		bio_dmabuf_map_set(bio, iter);
-	} else {
+	/*
+	 * Users don't rely on the iterator being in any particular state for
+	 * async I/O returning -EIOCBQUEUED, hence we can avoid the expensive
+	 * iov_iter_advance() if we could set the bvec/dmabuf directly.
+	 */
+	if (!bio_iov_iter_set(bio, iter)) {
  		ret = blkdev_iov_iter_get_pages(bio, iter, bdev);
  		if (unlikely(ret))
  			goto out_bio_put;

include/linux/bio.h +1 -2

index c053e9444514..1eff02a843ef 100644
@@ -484,8 +484,7 @@ int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data,
  int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
  		unsigned len_align_mask);
 
-void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter);
-void bio_dmabuf_map_set(struct bio *bio, struct iov_iter *iter);
+bool bio_iov_iter_set(struct bio *bio, const struct iov_iter *iter);
  void __bio_release_pages(struct bio *bio, bool mark_dirty);
  extern void bio_set_pages_dirty(struct bio *bio);
  extern void bio_check_pages_dirty(struct bio *bio);

+#if defined(CONFIG_DMA_SHARED_BUFFER)

drivers/nvme/host/core.c static int nvme_report_zones(struct gendisk *disk, sector_t sector, · line 2680

Quoted 2 lines

drivers/nvme/host/core.c +2 -0

+static int nvme_init_dma_buf_io_ctx(struct block_device *bdev,
+				    struct dma_buf_io_ctx *ctx)

Please stick to two-tab indents for function declaration continuation for the nvme code to keep the code easy to maintain.

Quoted 1 line

+#if defined(CONFIG_DMA_SHARED_BUFFER)

This should be good old #ifdef. Same in a few other places over the series.

Modulo these minor nits the patch looks good.

Enable BIO_DMABUF_MAP backed requests. It creates a prp list for the dmabuf when it's mapped, which is then used to initialise requests.

Suggested-by Keith Busch <[email protected]>

Signed-off-by Pavel Begunkov <[email protected]>

---
 drivers/nvme/host/core.c |  12 ++
 drivers/nvme/host/nvme.h |   2 +
 drivers/nvme/host/pci.c  | 259 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 273 insertions(+)

drivers/nvme/host/core.c +12 -0

index 453c1f0b2dd0..ce66a1843bec 100644
@@ -2676,6 +2676,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,

@@ -2686,6 +2697,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

index 824651cc898d..034c31af8fec 100644
@@ -652,6 +652,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 +95 -0

index 69932d640b53..f1b67c191892 100644
@@ -27,6 +27,8 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/io-64-nonatomic-hi-lo.h>
 #include <linux/sed-opal.h>
+#include <linux/dma-buf-io.h>
+#include <linux/dma-resv.h>
 
 #include "trace.h"
 #include "nvme.h"

@@ -393,6 +395,13 @@ struct nvme_queue {
        struct completion delete_done;
 };
 
+struct nvme_dmabuf_map {
+       struct dma_buf_io_map base;
+       struct sg_table *sgt;
+       unsigned nr_entries;
+       dma_addr_t dma_list[];
+};
+
 /* bits for iod->flags */
 enum nvme_iod_flags {
        /* this command has been aborted by the timeout handler */

@@ -859,6 +868,138 @@ static void nvme_free_descriptors(struct request *req)
        }
 }
 
+static inline struct nvme_dmabuf_map *
+to_nvme_dmabuf_map(struct dma_buf_io_map *map)
+{
+       return container_of(map, struct nvme_dmabuf_map, base);
+}
+
+static void nvme_dmabuf_map_sync_for_cpu(struct nvme_dev *nvme_dev,
+                                        struct request *req)
+{
+       struct device *dev = nvme_dev->dev;
+       enum dma_data_direction dma_dir;
+       struct bio *bio = req->bio;
+       struct nvme_dmabuf_map *map = to_nvme_dmabuf_map(bio->bi_dmabuf_map);
+       dma_addr_t *dma_list = map->dma_list;
+       unsigned offset = bio->bi_iter.bi_offset;
+       unsigned map_idx = offset / NVME_CTRL_PAGE_SIZE;
+       int length = blk_rq_payload_bytes(req) +
+                    (offset & (NVME_CTRL_PAGE_SIZE - 1));
+
+       dma_dir = rq_data_dir(req) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+       while (length > 0) {
+               dma_sync_single_for_cpu(dev, dma_list[map_idx++],
+                                       NVME_CTRL_PAGE_SIZE, dma_dir);
+               length -= NVME_CTRL_PAGE_SIZE;
+       }
+}
+
+static void nvme_dmabuf_map_sync_for_device(struct nvme_dev *nvme_dev,
+                                           struct request *req)
+{
+       struct device *dev = nvme_dev->dev;
+       enum dma_data_direction dma_dir;
+       struct bio *bio = req->bio;
+       struct nvme_dmabuf_map *map = to_nvme_dmabuf_map(bio->bi_dmabuf_map);
+       dma_addr_t *dma_list = map->dma_list;
+       unsigned offset = bio->bi_iter.bi_offset;
+       unsigned map_idx = offset / NVME_CTRL_PAGE_SIZE;
+       int length = blk_rq_payload_bytes(req) +
+                    (offset & (NVME_CTRL_PAGE_SIZE - 1));
+
+       dma_dir = rq_data_dir(req) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+       while (length > 0) {
+               dma_sync_single_for_device(dev, dma_list[map_idx++],
+                                          NVME_CTRL_PAGE_SIZE, dma_dir);
+               length -= NVME_CTRL_PAGE_SIZE;
+       }
+}
+
+static void nvme_rq_clean_dmabuf_map(struct nvme_dev *dev,
+                                     struct request *req)
+{
+       struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
+
+       nvme_dmabuf_map_sync_for_cpu(dev, req);
+
+       if (!(iod->flags & IOD_SINGLE_SEGMENT))
+               nvme_free_descriptors(req);
+}
+
+static blk_status_t nvme_rq_setup_dmabuf_map(struct request *req,
+                                            struct nvme_queue *nvmeq)
+{
+       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);
+       unsigned bvec_done = bio->bi_iter.bi_offset;
+       unsigned map_idx = bvec_done / NVME_CTRL_PAGE_SIZE;
+       unsigned offset = bvec_done & (NVME_CTRL_PAGE_SIZE - 1);
+       int length = blk_rq_payload_bytes(req) - (NVME_CTRL_PAGE_SIZE - offset);
+       dma_addr_t *dma_list = map->dma_list;
+       u64 prp1_dma = dma_list[map_idx++] + offset;
+       u64 dma_addr, prp2_dma;
+       dma_addr_t prp_dma;
+       __le64 *prp_list;
+       unsigned i;
+
+       nvme_dmabuf_map_sync_for_device(nvmeq->dev, req);
+
+       if (length <= 0) {
+               prp2_dma = 0;
+               goto done;
+       }
+
+       if (length <= NVME_CTRL_PAGE_SIZE) {

drivers/nvme/host/pci.c +154 -1

+       if (!prp_list)
+               return BLK_STS_RESOURCE;
+
+       iod->descriptors[iod->nr_descriptors++] = prp_list;
+       prp2_dma = prp_dma;
+       i = 0;
+       for (;;) {
+               if (i == NVME_CTRL_PAGE_SIZE >> 3) {
+                       __le64 *old_prp_list = prp_list;
+
+                       prp_list = dma_pool_alloc(nvmeq->descriptor_pools.large,
+                                       GFP_ATOMIC, &prp_dma);
+                       if (!prp_list)
+                               goto free_prps;
+                       iod->descriptors[iod->nr_descriptors++] = prp_list;
+                       prp_list[0] = old_prp_list[i - 1];
+                       old_prp_list[i - 1] = cpu_to_le64(prp_dma);
+                       i = 1;
+               }
+
+               dma_addr = dma_list[map_idx++];
+               prp_list[i++] = cpu_to_le64(dma_addr);
+
+               length -= NVME_CTRL_PAGE_SIZE;
+               if (length <= 0)
+                       break;
+       }
+done:
+       iod->cmd.common.dptr.prp1 = cpu_to_le64(prp1_dma);
+       iod->cmd.common.dptr.prp2 = cpu_to_le64(prp2_dma);
+       return BLK_STS_OK;
+free_prps:
+       nvme_free_descriptors(req);
+       return BLK_STS_RESOURCE;
+}
+
 static void nvme_free_prps(struct request *req, unsigned int attrs)
 {
        struct nvme_iod *iod = blk_mq_rq_to_pdu(req);

@@ -937,6 +1078,11 @@ static void nvme_unmap_data(struct request *req)
        struct device *dma_dev = nvmeq->dev->dev;
        unsigned int attrs = 0;
 
+       if (blk_mq_rq_is_dmabuf(req)) {
+               nvme_rq_clean_dmabuf_map(nvmeq->dev, req);
+               return;
+       }
+
        if (iod->flags & IOD_SINGLE_SEGMENT) {
                static_assert(offsetof(union nvme_data_ptr, prp1) ==
                                offsetof(union nvme_data_ptr, sgl.addr));

@@ -1251,6 +1397,9 @@ static blk_status_t nvme_map_data(struct request *req)
        struct blk_dma_iter iter;
        blk_status_t ret;
 
+       if (blk_mq_rq_is_dmabuf(req))
+               return nvme_rq_setup_dmabuf_map(req, nvmeq);
+
        /*
         * Try to skip the DMA iterator for single segment requests, as that
         * significantly improves performances for small I/O sizes.

@@ -2269,6 +2418,112 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled)
        return result;
 }
 
+#if defined(CONFIG_DMA_SHARED_BUFFER)
+static void nvme_dmabuf_invalidate_mappings(struct dma_buf_attachment *attach)
+{
+       struct dma_buf_io_ctx *ctx = attach->importer_priv;
+
+       dma_buf_io_invalidate_mappings(ctx);
+}
+
+const struct dma_buf_attach_ops nvme_dmabuf_importer_ops = {
+       .invalidate_mappings    = nvme_dmabuf_invalidate_mappings,
+       .allow_peer2peer        = true,
+};
+
+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 dma_buf_attachment *attach = ctx->dev_priv;
+       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);
+
+               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;
+               }
+       }
+
+       ret = dma_buf_io_init_map(ctx, &map->base);
+       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);
+}
+
+static void nvme_dma_buf_io_unmap(struct dma_buf_io_ctx *ctx,
+                                 struct dma_buf_io_map *map_base)
+{
+       struct dma_buf_attachment *attach = 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(attach, map->sgt, ctx->dir);
+}
+
+static void nvme_dma_buf_io_release(struct dma_buf_io_ctx *ctx)
+{
+       struct dma_buf_attachment *attach = ctx->dev_priv;
+
+       dma_buf_detach(ctx->dmabuf, attach);
+}
+
+const struct dma_buf_io_ops nvme_dma_buf_io_ops = {
+       .map            = nvme_dma_buf_io_map,
+       .unmap          = nvme_dma_buf_io_unmap,
+       .release        = nvme_dma_buf_io_release,
+};
+
+static int nvme_pci_init_dma_buf_io_ctx(struct nvme_ctrl *ctrl,
+                                       struct dma_buf_io_ctx *ctx)
+{
+       struct dma_buf_attachment *attach;
+       struct nvme_dev *dev = to_nvme_dev(ctrl);
+
+       attach = dma_buf_dynamic_attach(ctx->dmabuf, dev->dev,
+                                       &nvme_dmabuf_importer_ops, ctx);
+       if (IS_ERR(attach))
+               return PTR_ERR(attach);
+
+       ctx->dev_priv = attach;
+       ctx->dev_ops = &nvme_dma_buf_io_ops;
+       return 0;
+}
+#endif
+
 static const struct blk_mq_ops nvme_mq_admin_ops = {
        .queue_rq       = nvme_queue_rq,
        .complete       = nvme_pci_complete_rq,

@@ -3563,6 +3818,9 @@ static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
        .print_device_info      = nvme_pci_print_device_info,
        .supports_pci_p2pdma    = nvme_pci_supports_pci_p2pdma,
        .get_virt_boundary      = nvme_pci_get_virt_boundary,
+#if defined(CONFIG_DMA_SHARED_BUFFER)
+       .init_dma_buf_io_ctx    = nvme_pci_init_dma_buf_io_ctx,
+#endif
 };
 
 static int nvme_dev_map(struct nvme_dev *dev)

@@ -4327,5 +4585,6 @@ MODULE_AUTHOR("Matthew Wilcox <[email protected]>");
 MODULE_LICENSE("GPL");
 MODULE_VERSION("1.0");
 MODULE_DESCRIPTION("NVMe host PCIe transport driver");
+MODULE_IMPORT_NS("DMA_BUF");
 module_init(nvme_init);
 module_exit(nvme_exit);
--

2.54.0

Quoted 172 lines · Pavel Begunkov

Enable BIO_DMABUF_MAP backed requests. It creates a prp list for the dmabuf when it's mapped, which is then used to initialise requests.

Suggested-by Keith Busch <[email protected]>

Signed-off-by Pavel Begunkov <[email protected]>

---
  drivers/nvme/host/core.c |  12 ++
  drivers/nvme/host/nvme.h |   2 +
  drivers/nvme/host/pci.c  | 259 +++++++++++++++++++++++++++++++++++++++
  3 files changed, 273 insertions(+)

drivers/nvme/host/core.c +12 -0

index 453c1f0b2dd0..ce66a1843bec 100644
@@ -2676,6 +2676,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,

@@ -2686,6 +2697,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

index 824651cc898d..034c31af8fec 100644
@@ -652,6 +652,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 +95 -0

index 69932d640b53..f1b67c191892 100644
@@ -27,6 +27,8 @@
  #include <linux/io-64-nonatomic-lo-hi.h>
  #include <linux/io-64-nonatomic-hi-lo.h>
  #include <linux/sed-opal.h>
+#include <linux/dma-buf-io.h>
+#include <linux/dma-resv.h>
 
  #include "trace.h"
  #include "nvme.h"

@@ -393,6 +395,13 @@ struct nvme_queue {
         struct completion delete_done;
  };
 
+struct nvme_dmabuf_map {
+       struct dma_buf_io_map base;
+       struct sg_table *sgt;
+       unsigned nr_entries;
+       dma_addr_t dma_list[];
+};
+
  /* bits for iod->flags */
  enum nvme_iod_flags {
         /* this command has been aborted by the timeout handler */

@@ -859,6 +868,138 @@ static void nvme_free_descriptors(struct request *req)
         }
  }
 
+static inline struct nvme_dmabuf_map *
+to_nvme_dmabuf_map(struct dma_buf_io_map *map)
+{
+       return container_of(map, struct nvme_dmabuf_map, base);
+}
+
+static void nvme_dmabuf_map_sync_for_cpu(struct nvme_dev *nvme_dev,
+                                        struct request *req)
+{
+       struct device *dev = nvme_dev->dev;
+       enum dma_data_direction dma_dir;
+       struct bio *bio = req->bio;
+       struct nvme_dmabuf_map *map = to_nvme_dmabuf_map(bio->bi_dmabuf_map);
+       dma_addr_t *dma_list = map->dma_list;
+       unsigned offset = bio->bi_iter.bi_offset;
+       unsigned map_idx = offset / NVME_CTRL_PAGE_SIZE;
+       int length = blk_rq_payload_bytes(req) +
+                    (offset & (NVME_CTRL_PAGE_SIZE - 1));
+
+       dma_dir = rq_data_dir(req) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+       while (length > 0) {
+               dma_sync_single_for_cpu(dev, dma_list[map_idx++],
+                                       NVME_CTRL_PAGE_SIZE, dma_dir);
+               length -= NVME_CTRL_PAGE_SIZE;
+       }
+}
+
+static void nvme_dmabuf_map_sync_for_device(struct nvme_dev *nvme_dev,
+                                           struct request *req)
+{
+       struct device *dev = nvme_dev->dev;
+       enum dma_data_direction dma_dir;
+       struct bio *bio = req->bio;
+       struct nvme_dmabuf_map *map = to_nvme_dmabuf_map(bio->bi_dmabuf_map);
+       dma_addr_t *dma_list = map->dma_list;
+       unsigned offset = bio->bi_iter.bi_offset;
+       unsigned map_idx = offset / NVME_CTRL_PAGE_SIZE;
+       int length = blk_rq_payload_bytes(req) +
+                    (offset & (NVME_CTRL_PAGE_SIZE - 1));
+
+       dma_dir = rq_data_dir(req) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+       while (length > 0) {
+               dma_sync_single_for_device(dev, dma_list[map_idx++],
+                                          NVME_CTRL_PAGE_SIZE, dma_dir);
+               length -= NVME_CTRL_PAGE_SIZE;
+       }
+}
+
+static void nvme_rq_clean_dmabuf_map(struct nvme_dev *dev,
+                                     struct request *req)
+{
+       struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
+
+       nvme_dmabuf_map_sync_for_cpu(dev, req);
+
+       if (!(iod->flags & IOD_SINGLE_SEGMENT))
+               nvme_free_descriptors(req);
+}
+
+static blk_status_t nvme_rq_setup_dmabuf_map(struct request *req,
+                                            struct nvme_queue *nvmeq)
+{
+       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);
+       unsigned bvec_done = bio->bi_iter.bi_offset;
+       unsigned map_idx = bvec_done / NVME_CTRL_PAGE_SIZE;
+       unsigned offset = bvec_done & (NVME_CTRL_PAGE_SIZE - 1);
+       int length = blk_rq_payload_bytes(req) - (NVME_CTRL_PAGE_SIZE - offset);
+       dma_addr_t *dma_list = map->dma_list;
+       u64 prp1_dma = dma_list[map_idx++] + offset;
+       u64 dma_addr, prp2_dma;
+       dma_addr_t prp_dma;
+       __le64 *prp_list;
+       unsigned i;
+
+       nvme_dmabuf_map_sync_for_device(nvmeq->dev, req);
+
+       if (length <= 0) {
+               prp2_dma = 0;
+               goto done;
+       }
+
+       if (length <= NVME_CTRL_PAGE_SIZE) {

drivers/nvme/host/pci.c static void nvme_free_descriptors(struct request *req) · line 966

Quoted 10 lines

drivers/nvme/host/pci.c +10 -0

+               prp2_dma = dma_list[map_idx];
+               goto done;
+       }
+
+       if (DIV_ROUND_UP(length, NVME_CTRL_PAGE_SIZE) <=
+           NVME_SMALL_POOL_SIZE / sizeof(__le64))
+               iod->flags |= IOD_SMALL_DESCRIPTOR;
+
+       prp_list = dma_pool_alloc(nvme_dma_pool(nvmeq, iod), GFP_ATOMIC,
+                       &prp_dma);

I wonder if it's possible to perform the PRP list page allocations and initialization in nvme_dma_buf_io_map() instead of the I/O path. The NVMe dma_pools are per-NUMA-node linked lists protected by spinlocks, making them significant CPU hotspots. If the dmabuf registration set up a contiguous DMA-coherent list of PRP entries for the pages of the registered buffer, any command using the dmabuf and requiring at most 1 PRP list page (i.e. length <= 2 MB) could just set its PRP list pointer to an offset into the dmabuf's PRP list.

Quoted 9 lines

I wonder if it's possible to perform the PRP list page allocations and initialization in nvme_dma_buf_io_map() instead of the I/O path. The NVMe dma_pools are per-NUMA-node linked lists protected by spinlocks, making them significant CPU hotspots. If the dmabuf registration set up a contiguous DMA-coherent list of PRP entries for the pages of the registered buffer, any command using the dmabuf and requiring at most 1 PRP list page (i.e. length <= 2 MB) could just set its PRP list pointer to an offset into the dmabuf's PRP list.

Possible to an extent, predecessor patches tried that, I left it for later to keep the series simpler. I guess chaining won't work in general case.
I mentioned this to Kanchan, Anuj and Nitesh before, they were up to playing with nvme optimisations in general, and since sgl from Anuj is ready, maybe they already have a prototype somewhere for that.

Signature · 1 line

Pavel Begunkov

Quoted 10 lines · Pavel Begunkov

Quoted 9 lines

I wonder if it's possible to perform the PRP list page allocations and initialization in nvme_dma_buf_io_map() instead of the I/O path. The NVMe dma_pools are per-NUMA-node linked lists protected by spinlocks, making them significant CPU hotspots. If the dmabuf registration set up a contiguous DMA-coherent list of PRP entries for the pages of the registered buffer, any command using the dmabuf and requiring at most 1 PRP list page (i.e. length <= 2 MB) could just set its PRP list pointer to an offset into the dmabuf's PRP list.

Quoted 7 lines

Possible to an extent, predecessor patches tried that, I left it for later to keep the series simpler. I guess chaining won't work in general case.
I mentioned this to Kanchan, Anuj and Nitesh before, they were up to playing with nvme optimisations in general, and since sgl from Anuj is ready, maybe they already have a prototype somewhere for that.

Right, I have prototyped the PRP list optimisation before. I will send it once the initial version of this series lands, to avoid adding new review churn while the rest of the series is pretty settled.