[PATCH bpf-next v4 0/5] Replace CONFIG_DMABUF_SYSFS_STATS with BPF

[PATCH bpf-next v4 0/5] Replace CONFIG_DMABUF_SYSFS_STATS with BPF

[T.J. Mercier]

Until CONFIG_DMABUF_SYSFS_STATS was added [1] it was only possible to
perform per-buffer accounting with debugfs which is not suitable for
production environments. Eventually we discovered the overhead with
per-buffer sysfs file creation/removal was significantly impacting
allocation and free times, and exacerbated kernfs lock contention. [2]
dma_buf_stats_setup() is responsible for 39% of single-page buffer
creation duration, or 74% of single-page dma_buf_export() duration when
stressing dmabuf allocations and frees.

I prototyped a change from per-buffer to per-exporter statistics with a
RCU protected list of exporter allocations that accommodates most (but
not all) of our use-cases and avoids almost all of the sysfs overhead.
While that adds less overhead than per-buffer sysfs, and less even than
the maintenance of the dmabuf debugfs_list, it's still *additional*
overhead on top of the debugfs_list and doesn't give us per-buffer info.

This series uses the existing dmabuf debugfs_list to implement a BPF
dmabuf iterator, which adds no overhead to buffer allocation/free and
provides per-buffer info. The list has been moved outside of
CONFIG_DEBUG_FS scope so that it is always populated. The BPF program
loaded by userspace that extracts per-buffer information gets to define
its own interface which avoids the lack of ABI stability with debugfs.

This will allow us to replace our use of CONFIG_DMABUF_SYSFS_STATS, and
the plan is to remove it from the kernel after the next longterm stable
release.

[1] https://lore.kernel.org/linux-media/20201210044400.1080308-1-hridya@google.com
[2] https://lore.kernel.org/all/20220516171315.2400578-1-tjmercier@google.com

v1: https://lore.kernel.org/all/20250414225227.3642618-1-tjmercier@google.com
v1 -> v2:
Make the DMA buffer list independent of CONFIG_DEBUG_FS per Christian König
Add CONFIG_DMA_SHARED_BUFFER check to kernel/bpf/Makefile per kernel test robot
Use BTF_ID_LIST_SINGLE instead of BTF_ID_LIST_GLOBAL_SINGLE per Song Liu
Fixup comment style, mixing code/declarations, and use ASSERT_OK_FD in selftest per Song Liu
Add BPF_ITER_RESCHED feature to bpf_dmabuf_reg_info per Alexei Starovoitov
Add open-coded iterator and selftest per Alexei Starovoitov
Add a second test buffer from the system dmabuf heap to selftests
Use the BPF program we'll use in production for selftest per Alexei Starovoitov
  https://r.android.com/c/platform/system/bpfprogs/+/3616123/2/dmabufIter.c
  https://r.android.com/c/platform/system/memory/libmeminfo/+/3614259/1/libdmabufinfo/dmabuf_bpf_stats.cpp
v2: https://lore.kernel.org/all/20250504224149.1033867-1-tjmercier@google.com
v2 -> v3:
Rebase onto bpf-next/master
Move get_next_dmabuf() into drivers/dma-buf/dma-buf.c, along with the
  new get_first_dmabuf(). This avoids having to expose the dmabuf list
  and mutex to the rest of the kernel, and keeps the dmabuf mutex
  operations near each other in the same file. (Christian König)
Add Christian's RB to dma-buf: Rename debugfs symbols
Drop RFC: dma-buf: Remove DMA-BUF statistics
v3: https://lore.kernel.org/all/20250507001036.2278781-1-tjmercier@google.com
v3 -> v4:
Fix selftest BPF program comment style (not kdoc) per Alexei Starovoitov
Fix dma-buf.c kdoc comment style per Alexei Starovoitov
Rename get_first_dmabuf / get_next_dmabuf to dma_buf_iter_begin /
  dma_buf_iter_next per Christian König
Add Christian's RB to bpf: Add dmabuf iterator

T.J. Mercier (5):
  dma-buf: Rename debugfs symbols
  bpf: Add dmabuf iterator
  bpf: Add open coded dmabuf iterator
  selftests/bpf: Add test for dmabuf_iter
  selftests/bpf: Add test for open coded dmabuf_iter

 drivers/dma-buf/dma-buf.c                     |  98 +++++--
 include/linux/dma-buf.h                       |   4 +-
 kernel/bpf/Makefile                           |   3 +
 kernel/bpf/dmabuf_iter.c                      | 149 ++++++++++
 kernel/bpf/helpers.c                          |   5 +
 .../testing/selftests/bpf/bpf_experimental.h  |   5 +
 tools/testing/selftests/bpf/config            |   3 +
 .../selftests/bpf/prog_tests/dmabuf_iter.c    | 258 ++++++++++++++++++
 .../testing/selftests/bpf/progs/dmabuf_iter.c |  91 ++++++
 9 files changed, 594 insertions(+), 22 deletions(-)
 create mode 100644 kernel/bpf/dmabuf_iter.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/dmabuf_iter.c


base-commit: 43745d11bfd9683abdf08ad7a5cc403d6a9ffd15
-- 
2.49.0.1015.ga840276032-goog

[PATCH bpf-next v4 1/5] dma-buf: Rename debugfs symbols

[T.J. Mercier]

Rename the debugfs list and mutex so it's clear they are now usable
without the need for CONFIG_DEBUG_FS. The list will always be populated
to support the creation of a BPF iterator for dmabufs.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-buf.c | 40 +++++++++++++++------------------------
 include/linux/dma-buf.h   |  2 --
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 5baa83b85515..8d151784e302 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -35,35 +35,25 @@
 
 static inline int is_dma_buf_file(struct file *);
 
-#if IS_ENABLED(CONFIG_DEBUG_FS)
-static DEFINE_MUTEX(debugfs_list_mutex);
-static LIST_HEAD(debugfs_list);
+static DEFINE_MUTEX(dmabuf_list_mutex);
+static LIST_HEAD(dmabuf_list);
 
-static void __dma_buf_debugfs_list_add(struct dma_buf *dmabuf)
+static void __dma_buf_list_add(struct dma_buf *dmabuf)
 {
-	mutex_lock(&debugfs_list_mutex);
-	list_add(&dmabuf->list_node, &debugfs_list);
-	mutex_unlock(&debugfs_list_mutex);
+	mutex_lock(&dmabuf_list_mutex);
+	list_add(&dmabuf->list_node, &dmabuf_list);
+	mutex_unlock(&dmabuf_list_mutex);
 }
 
-static void __dma_buf_debugfs_list_del(struct dma_buf *dmabuf)
+static void __dma_buf_list_del(struct dma_buf *dmabuf)
 {
 	if (!dmabuf)
 		return;
 
-	mutex_lock(&debugfs_list_mutex);
+	mutex_lock(&dmabuf_list_mutex);
 	list_del(&dmabuf->list_node);
-	mutex_unlock(&debugfs_list_mutex);
+	mutex_unlock(&dmabuf_list_mutex);
 }
-#else
-static void __dma_buf_debugfs_list_add(struct dma_buf *dmabuf)
-{
-}
-
-static void __dma_buf_debugfs_list_del(struct dma_buf *dmabuf)
-{
-}
-#endif
 
 static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
 {
@@ -115,7 +105,7 @@ static int dma_buf_file_release(struct inode *inode, struct file *file)
 	if (!is_dma_buf_file(file))
 		return -EINVAL;
 
-	__dma_buf_debugfs_list_del(file->private_data);
+	__dma_buf_list_del(file->private_data);
 
 	return 0;
 }
@@ -689,7 +679,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 	file->f_path.dentry->d_fsdata = dmabuf;
 	dmabuf->file = file;
 
-	__dma_buf_debugfs_list_add(dmabuf);
+	__dma_buf_list_add(dmabuf);
 
 	return dmabuf;
 
@@ -1630,7 +1620,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
 	size_t size = 0;
 	int ret;
 
-	ret = mutex_lock_interruptible(&debugfs_list_mutex);
+	ret = mutex_lock_interruptible(&dmabuf_list_mutex);
 
 	if (ret)
 		return ret;
@@ -1639,7 +1629,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
 	seq_printf(s, "%-8s\t%-8s\t%-8s\t%-8s\texp_name\t%-8s\tname\n",
 		   "size", "flags", "mode", "count", "ino");
 
-	list_for_each_entry(buf_obj, &debugfs_list, list_node) {
+	list_for_each_entry(buf_obj, &dmabuf_list, list_node) {
 
 		ret = dma_resv_lock_interruptible(buf_obj->resv, NULL);
 		if (ret)
@@ -1676,11 +1666,11 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
 
 	seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size);
 
-	mutex_unlock(&debugfs_list_mutex);
+	mutex_unlock(&dmabuf_list_mutex);
 	return 0;
 
 error_unlock:
-	mutex_unlock(&debugfs_list_mutex);
+	mutex_unlock(&dmabuf_list_mutex);
 	return ret;
 }
 
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 36216d28d8bd..8ff4add71f88 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -370,10 +370,8 @@ struct dma_buf {
 	 */
 	struct module *owner;
 
-#if IS_ENABLED(CONFIG_DEBUG_FS)
 	/** @list_node: node for dma_buf accounting and debugging. */
 	struct list_head list_node;
-#endif
 
 	/** @priv: exporter specific private data for this buffer object. */
 	void *priv;
-- 
2.49.0.1015.ga840276032-goog

[PATCH bpf-next v4 2/5] bpf: Add dmabuf iterator

[T.J. Mercier]

The dmabuf iterator traverses the list of all DMA buffers.

DMA buffers are refcounted through their associated struct file. A
reference is taken on each buffer as the list is iterated to ensure each
buffer persists for the duration of the bpf program execution without
holding the list mutex.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-buf.c |  68 +++++++++++++++++++++++++
 include/linux/dma-buf.h   |   2 +
 kernel/bpf/Makefile       |   3 ++
 kernel/bpf/dmabuf_iter.c  | 102 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 175 insertions(+)
 create mode 100644 kernel/bpf/dmabuf_iter.c

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 8d151784e302..6b59506a1b94 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -19,7 +19,9 @@
 #include <linux/anon_inodes.h>
 #include <linux/export.h>
 #include <linux/debugfs.h>
+#include <linux/list.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/seq_file.h>
 #include <linux/sync_file.h>
 #include <linux/poll.h>
@@ -55,6 +57,72 @@ static void __dma_buf_list_del(struct dma_buf *dmabuf)
 	mutex_unlock(&dmabuf_list_mutex);
 }
 
+/**
+ * dma_buf_iter_begin - begin iteration through global list of all DMA buffers
+ *
+ * Returns the first buffer in the global list of DMA-bufs that's not in the
+ * process of being destroyed. Increments that buffer's reference count to
+ * prevent buffer destruction. Callers must release the reference, either by
+ * continuing iteration with dma_buf_iter_next(), or with dma_buf_put().
+ *
+ * Return:
+ * * First buffer from global list, with refcount elevated
+ * * NULL if no active buffers are present
+ */
+struct dma_buf *dma_buf_iter_begin(void)
+{
+	struct dma_buf *ret = NULL, *dmabuf;
+
+	/*
+	 * The list mutex does not protect a dmabuf's refcount, so it can be
+	 * zeroed while we are iterating. We cannot call get_dma_buf() since the
+	 * caller may not already own a reference to the buffer.
+	 */
+	mutex_lock(&dmabuf_list_mutex);
+	list_for_each_entry(dmabuf, &dmabuf_list, list_node) {
+		if (file_ref_get(&dmabuf->file->f_ref)) {
+			ret = dmabuf;
+			break;
+		}
+	}
+	mutex_unlock(&dmabuf_list_mutex);
+	return ret;
+}
+
+/**
+ * dma_buf_iter_next - continue iteration through global list of all DMA buffers
+ * @dmabuf:	[in]	pointer to dma_buf
+ *
+ * Decrements the reference count on the provided buffer. Returns the next
+ * buffer from the remainder of the global list of DMA-bufs with its reference
+ * count incremented. Callers must release the reference, either by continuing
+ * iteration with dma_buf_iter_next(), or with dma_buf_put().
+ *
+ * Return:
+ * * Next buffer from global list, with refcount elevated
+ * * NULL if no additional active buffers are present
+ */
+struct dma_buf *dma_buf_iter_next(struct dma_buf *dmabuf)
+{
+	struct dma_buf *ret = NULL;
+
+	/*
+	 * The list mutex does not protect a dmabuf's refcount, so it can be
+	 * zeroed while we are iterating. We cannot call get_dma_buf() since the
+	 * caller may not already own a reference to the buffer.
+	 */
+	mutex_lock(&dmabuf_list_mutex);
+	dma_buf_put(dmabuf);
+	list_for_each_entry_continue(dmabuf, &dmabuf_list, list_node) {
+		if (file_ref_get(&dmabuf->file->f_ref)) {
+			ret = dmabuf;
+			break;
+		}
+	}
+	mutex_unlock(&dmabuf_list_mutex);
+	return ret;
+}
+
 static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
 {
 	struct dma_buf *dmabuf;
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 8ff4add71f88..7af2ea839f58 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -634,4 +634,6 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map);
 void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map);
 int dma_buf_vmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map);
 void dma_buf_vunmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map);
+struct dma_buf *dma_buf_iter_begin(void);
+struct dma_buf *dma_buf_iter_next(struct dma_buf *dmbuf);
 #endif /* __DMA_BUF_H__ */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 70502f038b92..3a335c50e6e3 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -53,6 +53,9 @@ obj-$(CONFIG_BPF_SYSCALL) += relo_core.o
 obj-$(CONFIG_BPF_SYSCALL) += btf_iter.o
 obj-$(CONFIG_BPF_SYSCALL) += btf_relocate.o
 obj-$(CONFIG_BPF_SYSCALL) += kmem_cache_iter.o
+ifeq ($(CONFIG_DMA_SHARED_BUFFER),y)
+obj-$(CONFIG_BPF_SYSCALL) += dmabuf_iter.o
+endif
 
 CFLAGS_REMOVE_percpu_freelist.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_bpf_lru_list.o = $(CC_FLAGS_FTRACE)
diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
new file mode 100644
index 000000000000..96b4ba7f0b2c
--- /dev/null
+++ b/kernel/bpf/dmabuf_iter.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2025 Google LLC */
+#include <linux/bpf.h>
+#include <linux/btf_ids.h>
+#include <linux/dma-buf.h>
+#include <linux/kernel.h>
+#include <linux/seq_file.h>
+
+BTF_ID_LIST_SINGLE(bpf_dmabuf_btf_id, struct, dma_buf)
+DEFINE_BPF_ITER_FUNC(dmabuf, struct bpf_iter_meta *meta, struct dma_buf *dmabuf)
+
+static void *dmabuf_iter_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	if (*pos)
+		return NULL;
+
+	return dma_buf_iter_begin();
+}
+
+static void *dmabuf_iter_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+	struct dma_buf *dmabuf = v;
+
+	++*pos;
+
+	return dma_buf_iter_next(dmabuf);
+}
+
+struct bpf_iter__dmabuf {
+	__bpf_md_ptr(struct bpf_iter_meta *, meta);
+	__bpf_md_ptr(struct dma_buf *, dmabuf);
+};
+
+static int __dmabuf_seq_show(struct seq_file *seq, void *v, bool in_stop)
+{
+	struct bpf_iter_meta meta = {
+		.seq = seq,
+	};
+	struct bpf_iter__dmabuf ctx = {
+		.meta = &meta,
+		.dmabuf = v,
+	};
+	struct bpf_prog *prog = bpf_iter_get_info(&meta, in_stop);
+
+	if (prog)
+		return bpf_iter_run_prog(prog, &ctx);
+
+	return 0;
+}
+
+static int dmabuf_iter_seq_show(struct seq_file *seq, void *v)
+{
+	return __dmabuf_seq_show(seq, v, false);
+}
+
+static void dmabuf_iter_seq_stop(struct seq_file *seq, void *v)
+{
+	struct dma_buf *dmabuf = v;
+
+	if (dmabuf)
+		dma_buf_put(dmabuf);
+}
+
+static const struct seq_operations dmabuf_iter_seq_ops = {
+	.start	= dmabuf_iter_seq_start,
+	.next	= dmabuf_iter_seq_next,
+	.stop	= dmabuf_iter_seq_stop,
+	.show	= dmabuf_iter_seq_show,
+};
+
+static void bpf_iter_dmabuf_show_fdinfo(const struct bpf_iter_aux_info *aux,
+					struct seq_file *seq)
+{
+	seq_puts(seq, "dmabuf iter\n");
+}
+
+static const struct bpf_iter_seq_info dmabuf_iter_seq_info = {
+	.seq_ops		= &dmabuf_iter_seq_ops,
+	.init_seq_private	= NULL,
+	.fini_seq_private	= NULL,
+	.seq_priv_size		= 0,
+};
+
+static struct bpf_iter_reg bpf_dmabuf_reg_info = {
+	.target			= "dmabuf",
+	.feature                = BPF_ITER_RESCHED,
+	.show_fdinfo		= bpf_iter_dmabuf_show_fdinfo,
+	.ctx_arg_info_size	= 1,
+	.ctx_arg_info		= {
+		{ offsetof(struct bpf_iter__dmabuf, dmabuf),
+		  PTR_TO_BTF_ID_OR_NULL },
+	},
+	.seq_info		= &dmabuf_iter_seq_info,
+};
+
+static int __init dmabuf_iter_init(void)
+{
+	bpf_dmabuf_reg_info.ctx_arg_info[0].btf_id = bpf_dmabuf_btf_id[0];
+	return bpf_iter_reg_target(&bpf_dmabuf_reg_info);
+}
+
+late_initcall(dmabuf_iter_init);
-- 
2.49.0.1015.ga840276032-goog

[PATCH bpf-next v4 3/5] bpf: Add open coded dmabuf iterator

[T.J. Mercier]

This open coded iterator allows for more flexibility when creating BPF
programs. It can support output in formats other than text. With an open
coded iterator, a single BPF program can traverse multiple kernel data
structures (now including dmabufs), allowing for more efficient analysis
of kernel data compared to multiple reads from procfs, sysfs, or
multiple traditional BPF iterator invocations.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
---
 kernel/bpf/dmabuf_iter.c | 47 ++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/helpers.c     |  5 +++++
 2 files changed, 52 insertions(+)

diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
index 96b4ba7f0b2c..8049bdbc9efc 100644
--- a/kernel/bpf/dmabuf_iter.c
+++ b/kernel/bpf/dmabuf_iter.c
@@ -100,3 +100,50 @@ static int __init dmabuf_iter_init(void)
 }
 
 late_initcall(dmabuf_iter_init);
+
+struct bpf_iter_dmabuf {
+	/* opaque iterator state; having __u64 here allows to preserve correct
+	 * alignment requirements in vmlinux.h, generated from BTF
+	 */
+	__u64 __opaque[1];
+} __aligned(8);
+
+/* Non-opaque version of bpf_iter_dmabuf */
+struct bpf_iter_dmabuf_kern {
+	struct dma_buf *dmabuf;
+} __aligned(8);
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it)
+{
+	struct bpf_iter_dmabuf_kern *kit = (void *)it;
+
+	BUILD_BUG_ON(sizeof(*kit) > sizeof(*it));
+	BUILD_BUG_ON(__alignof__(*kit) != __alignof__(*it));
+
+	kit->dmabuf = NULL;
+	return 0;
+}
+
+__bpf_kfunc struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it)
+{
+	struct bpf_iter_dmabuf_kern *kit = (void *)it;
+
+	if (kit->dmabuf)
+		kit->dmabuf = dma_buf_iter_next(kit->dmabuf);
+	else
+		kit->dmabuf = dma_buf_iter_begin();
+
+	return kit->dmabuf;
+}
+
+__bpf_kfunc void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it)
+{
+	struct bpf_iter_dmabuf_kern *kit = (void *)it;
+
+	if (kit->dmabuf)
+		dma_buf_put(kit->dmabuf);
+}
+
+__bpf_kfunc_end_defs();
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 78cefb41266a..39fe63016868 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3346,6 +3346,11 @@ BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLE
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_local_irq_save)
 BTF_ID_FLAGS(func, bpf_local_irq_restore)
+#ifdef CONFIG_DMA_SHARED_BUFFER
+BTF_ID_FLAGS(func, bpf_iter_dmabuf_new, KF_ITER_NEW | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_iter_dmabuf_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_iter_dmabuf_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
+#endif
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
-- 
2.49.0.1015.ga840276032-goog

[PATCH bpf-next v4 4/5] selftests/bpf: Add test for dmabuf_iter

[T.J. Mercier]

This test creates a udmabuf, and a dmabuf from the system dmabuf heap,
and uses a BPF program that prints dmabuf metadata with the new
dmabuf_iter to verify they can be found.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
---
 tools/testing/selftests/bpf/config            |   3 +
 .../selftests/bpf/prog_tests/dmabuf_iter.c    | 224 ++++++++++++++++++
 .../testing/selftests/bpf/progs/dmabuf_iter.c |  53 +++++
 3 files changed, 280 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/dmabuf_iter.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index c378d5d07e02..2bdff2f3285f 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -22,6 +22,8 @@ CONFIG_CRYPTO_AES=y
 CONFIG_DEBUG_INFO=y
 CONFIG_DEBUG_INFO_BTF=y
 CONFIG_DEBUG_INFO_DWARF4=y
+CONFIG_DMABUF_HEAPS=y
+CONFIG_DMABUF_HEAPS_SYSTEM=y
 CONFIG_DUMMY=y
 CONFIG_DYNAMIC_FTRACE=y
 CONFIG_FPROBE=y
@@ -106,6 +108,7 @@ CONFIG_SECURITY=y
 CONFIG_SECURITYFS=y
 CONFIG_SYN_COOKIES=y
 CONFIG_TEST_BPF=m
+CONFIG_UDMABUF=y
 CONFIG_USERFAULTFD=y
 CONFIG_VSOCKETS=y
 CONFIG_VXLAN=y
diff --git a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
new file mode 100644
index 000000000000..35745f4ce0f8
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Google */
+
+#include <test_progs.h>
+#include <bpf/libbpf.h>
+#include <bpf/btf.h>
+#include "dmabuf_iter.skel.h"
+
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+#include <linux/udmabuf.h>
+
+static int memfd, udmabuf;
+static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] = "udmabuf_test_buffer_for_iter";
+static size_t udmabuf_test_buffer_size;
+static int sysheap_dmabuf;
+static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] = "sysheap_test_buffer_for_iter";
+static size_t sysheap_test_buffer_size;
+
+static int create_udmabuf(void)
+{
+	struct udmabuf_create create;
+	int dev_udmabuf;
+
+	udmabuf_test_buffer_size = 10 * getpagesize();
+
+	if (!ASSERT_LE(sizeof(udmabuf_test_buffer_name), DMA_BUF_NAME_LEN, "NAMETOOLONG"))
+		return 1;
+
+	memfd = memfd_create("memfd_test", MFD_ALLOW_SEALING);
+	if (!ASSERT_OK_FD(memfd, "memfd_create"))
+		return 1;
+
+	if (!ASSERT_OK(ftruncate(memfd, udmabuf_test_buffer_size), "ftruncate"))
+		return 1;
+
+	if (!ASSERT_OK(fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK), "seal"))
+		return 1;
+
+	dev_udmabuf = open("/dev/udmabuf", O_RDONLY);
+	if (!ASSERT_OK_FD(dev_udmabuf, "open udmabuf"))
+		return 1;
+
+	create.memfd = memfd;
+	create.flags = UDMABUF_FLAGS_CLOEXEC;
+	create.offset = 0;
+	create.size = udmabuf_test_buffer_size;
+
+	udmabuf = ioctl(dev_udmabuf, UDMABUF_CREATE, &create);
+	close(dev_udmabuf);
+	if (!ASSERT_OK_FD(udmabuf, "udmabuf_create"))
+		return 1;
+
+	if (!ASSERT_OK(ioctl(udmabuf, DMA_BUF_SET_NAME_B, udmabuf_test_buffer_name), "name"))
+		return 1;
+
+	return 0;
+}
+
+static int create_sys_heap_dmabuf(void)
+{
+	sysheap_test_buffer_size = 20 * getpagesize();
+
+	struct dma_heap_allocation_data data = {
+		.len = sysheap_test_buffer_size,
+		.fd = 0,
+		.fd_flags = O_RDWR | O_CLOEXEC,
+		.heap_flags = 0,
+	};
+	int heap_fd, ret;
+
+	if (!ASSERT_LE(sizeof(sysheap_test_buffer_name), DMA_BUF_NAME_LEN, "NAMETOOLONG"))
+		return 1;
+
+	heap_fd = open("/dev/dma_heap/system", O_RDONLY);
+	if (!ASSERT_OK_FD(heap_fd, "open dma heap"))
+		return 1;
+
+	ret = ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &data);
+	close(heap_fd);
+	if (!ASSERT_OK(ret, "syheap alloc"))
+		return 1;
+
+	sysheap_dmabuf = data.fd;
+
+	if (!ASSERT_OK(ioctl(sysheap_dmabuf, DMA_BUF_SET_NAME_B, sysheap_test_buffer_name), "name"))
+		return 1;
+
+	return 0;
+}
+
+static int create_test_buffers(void)
+{
+	int ret;
+
+	ret = create_udmabuf();
+	if (ret)
+		return ret;
+
+	return create_sys_heap_dmabuf();
+}
+
+static void destroy_test_buffers(void)
+{
+	close(udmabuf);
+	close(memfd);
+	close(sysheap_dmabuf);
+}
+
+enum Fields { INODE, SIZE, NAME, EXPORTER, FIELD_COUNT };
+struct DmabufInfo {
+	unsigned long inode;
+	unsigned long size;
+	char name[DMA_BUF_NAME_LEN];
+	char exporter[32];
+};
+
+static bool check_dmabuf_info(const struct DmabufInfo *bufinfo,
+			      unsigned long size,
+			      const char *name, const char *exporter)
+{
+	return size == bufinfo->size &&
+	       !strcmp(name, bufinfo->name) &&
+	       !strcmp(exporter, bufinfo->exporter);
+}
+
+static void subtest_dmabuf_iter_check_default_iter(struct dmabuf_iter *skel)
+{
+	bool found_test_sysheap_dmabuf = false;
+	bool found_test_udmabuf = false;
+	struct DmabufInfo bufinfo;
+	size_t linesize = 0;
+	char *line = NULL;
+	FILE *iter_file;
+	int iter_fd, f = INODE;
+
+	iter_fd = bpf_iter_create(bpf_link__fd(skel->links.dmabuf_collector));
+	ASSERT_OK_FD(iter_fd, "iter_create");
+
+	iter_file = fdopen(iter_fd, "r");
+	ASSERT_OK_PTR(iter_file, "fdopen");
+
+	while (getline(&line, &linesize, iter_file) != -1) {
+		if (f % FIELD_COUNT == INODE) {
+			ASSERT_EQ(sscanf(line, "%ld", &bufinfo.inode), 1,
+				  "read inode");
+		} else if (f % FIELD_COUNT == SIZE) {
+			ASSERT_EQ(sscanf(line, "%ld", &bufinfo.size), 1,
+				  "read size");
+		} else if (f % FIELD_COUNT == NAME) {
+			ASSERT_EQ(sscanf(line, "%s", bufinfo.name), 1,
+				  "read name");
+		} else if (f % FIELD_COUNT == EXPORTER) {
+			ASSERT_EQ(sscanf(line, "%31s", bufinfo.exporter), 1,
+				  "read exporter");
+
+			if (check_dmabuf_info(&bufinfo,
+					      sysheap_test_buffer_size,
+					      sysheap_test_buffer_name,
+					      "system"))
+				found_test_sysheap_dmabuf = true;
+			else if (check_dmabuf_info(&bufinfo,
+						   udmabuf_test_buffer_size,
+						   udmabuf_test_buffer_name,
+						   "udmabuf"))
+				found_test_udmabuf = true;
+		}
+		++f;
+	}
+
+	ASSERT_EQ(f % FIELD_COUNT, INODE, "number of fields");
+
+	ASSERT_TRUE(found_test_sysheap_dmabuf, "found_test_sysheap_dmabuf");
+	ASSERT_TRUE(found_test_udmabuf, "found_test_udmabuf");
+
+	free(line);
+	fclose(iter_file);
+	close(iter_fd);
+}
+
+void test_dmabuf_iter(void)
+{
+	struct dmabuf_iter *skel = NULL;
+	char buf[256];
+	int iter_fd;
+
+	skel = dmabuf_iter__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "dmabuf_iter__open_and_load"))
+		return;
+
+	if (!ASSERT_OK(create_test_buffers(), "create_buffers"))
+		goto destroy;
+
+	if (!ASSERT_OK(dmabuf_iter__attach(skel), "skel_attach"))
+		goto destroy;
+
+	iter_fd = bpf_iter_create(bpf_link__fd(skel->links.dmabuf_collector));
+	if (!ASSERT_OK_FD(iter_fd, "iter_create"))
+		goto destroy;
+
+	while (read(iter_fd, buf, sizeof(buf)) > 0)
+		; /* Read out all contents */
+
+	/* Next reads should return 0 */
+	ASSERT_EQ(read(iter_fd, buf, sizeof(buf)), 0, "read");
+
+	if (test__start_subtest("default_iter"))
+		subtest_dmabuf_iter_check_default_iter(skel);
+
+	close(iter_fd);
+
+destroy:
+	destroy_test_buffers();
+	dmabuf_iter__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/dmabuf_iter.c b/tools/testing/selftests/bpf/progs/dmabuf_iter.c
new file mode 100644
index 000000000000..d654b4f64cfa
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/dmabuf_iter.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Google LLC */
+#include <vmlinux.h>
+#include <bpf/bpf_core_read.h>
+#include <bpf/bpf_helpers.h>
+
+/* From uapi/linux/dma-buf.h */
+#define DMA_BUF_NAME_LEN 32
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * Fields output by this iterator are delimited by newlines. Convert any
+ * newlines in user-provided printed strings to spaces.
+ */
+static void sanitize_string(char *src, size_t size)
+{
+	for (char *c = src; c && (size_t)(c-src) < size; ++c)
+		if (*c == '\n')
+			*c = ' ';
+}
+
+SEC("iter/dmabuf")
+int dmabuf_collector(struct bpf_iter__dmabuf *ctx)
+{
+	const struct dma_buf *dmabuf = ctx->dmabuf;
+	struct seq_file *seq = ctx->meta->seq;
+	unsigned long inode = 0;
+	size_t size;
+	const char *pname, *exporter;
+	char name[DMA_BUF_NAME_LEN] = {'\0'};
+
+	if (!dmabuf)
+		return 0;
+
+	if (BPF_CORE_READ_INTO(&inode, dmabuf, file, f_inode, i_ino) ||
+	    bpf_core_read(&size, sizeof(size), &dmabuf->size) ||
+	    bpf_core_read(&pname, sizeof(pname), &dmabuf->name) ||
+	    bpf_core_read(&exporter, sizeof(exporter), &dmabuf->exp_name))
+		return 1;
+
+	/* Buffers are not required to be named */
+	if (pname) {
+		if (bpf_probe_read_kernel(name, sizeof(name), pname))
+			return 1;
+
+		/* Name strings can be provided by userspace */
+		sanitize_string(name, sizeof(name));
+	}
+
+	BPF_SEQ_PRINTF(seq, "%lu\n%llu\n%s\n%s\n", inode, size, name, exporter);
+	return 0;
+}
-- 
2.49.0.1015.ga840276032-goog

[PATCH bpf-next v4 5/5] selftests/bpf: Add test for open coded dmabuf_iter

[T.J. Mercier]

Use the same test buffers as the traditional iterator and a new BPF map
to verify the test buffers can be found with the open coded dmabuf
iterator.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
---
 .../testing/selftests/bpf/bpf_experimental.h  |  5 ++
 .../selftests/bpf/prog_tests/dmabuf_iter.c    | 52 +++++++++++++++----
 .../testing/selftests/bpf/progs/dmabuf_iter.c | 38 ++++++++++++++
 3 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 6535c8ae3c46..5e512a1d09d1 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -591,4 +591,9 @@ extern int bpf_iter_kmem_cache_new(struct bpf_iter_kmem_cache *it) __weak __ksym
 extern struct kmem_cache *bpf_iter_kmem_cache_next(struct bpf_iter_kmem_cache *it) __weak __ksym;
 extern void bpf_iter_kmem_cache_destroy(struct bpf_iter_kmem_cache *it) __weak __ksym;
 
+struct bpf_iter_dmabuf;
+extern int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it) __weak __ksym;
+extern struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it) __weak __ksym;
+extern void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it) __weak __ksym;
+
 #endif
diff --git a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
index 35745f4ce0f8..c8230a080ef3 100644
--- a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
@@ -26,10 +26,11 @@ static int sysheap_dmabuf;
 static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] = "sysheap_test_buffer_for_iter";
 static size_t sysheap_test_buffer_size;
 
-static int create_udmabuf(void)
+static int create_udmabuf(int map_fd)
 {
 	struct udmabuf_create create;
 	int dev_udmabuf;
+	bool f = false;
 
 	udmabuf_test_buffer_size = 10 * getpagesize();
 
@@ -63,10 +64,10 @@ static int create_udmabuf(void)
 	if (!ASSERT_OK(ioctl(udmabuf, DMA_BUF_SET_NAME_B, udmabuf_test_buffer_name), "name"))
 		return 1;
 
-	return 0;
+	return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name, &f, BPF_ANY);
 }
 
-static int create_sys_heap_dmabuf(void)
+static int create_sys_heap_dmabuf(int map_fd)
 {
 	sysheap_test_buffer_size = 20 * getpagesize();
 
@@ -77,6 +78,7 @@ static int create_sys_heap_dmabuf(void)
 		.heap_flags = 0,
 	};
 	int heap_fd, ret;
+	bool f = false;
 
 	if (!ASSERT_LE(sizeof(sysheap_test_buffer_name), DMA_BUF_NAME_LEN, "NAMETOOLONG"))
 		return 1;
@@ -95,18 +97,18 @@ static int create_sys_heap_dmabuf(void)
 	if (!ASSERT_OK(ioctl(sysheap_dmabuf, DMA_BUF_SET_NAME_B, sysheap_test_buffer_name), "name"))
 		return 1;
 
-	return 0;
+	return bpf_map_update_elem(map_fd, sysheap_test_buffer_name, &f, BPF_ANY);
 }
 
-static int create_test_buffers(void)
+static int create_test_buffers(int map_fd)
 {
 	int ret;
 
-	ret = create_udmabuf();
+	ret = create_udmabuf(map_fd);
 	if (ret)
 		return ret;
 
-	return create_sys_heap_dmabuf();
+	return create_sys_heap_dmabuf(map_fd);
 }
 
 static void destroy_test_buffers(void)
@@ -187,17 +189,46 @@ static void subtest_dmabuf_iter_check_default_iter(struct dmabuf_iter *skel)
 	close(iter_fd);
 }
 
+static void subtest_dmabuf_iter_check_open_coded(struct dmabuf_iter *skel, int map_fd)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	char key[DMA_BUF_NAME_LEN];
+	int err, fd;
+	bool found;
+
+	/* No need to attach it, just run it directly */
+	fd = bpf_program__fd(skel->progs.iter_dmabuf_for_each);
+
+	err = bpf_prog_test_run_opts(fd, &topts);
+	if (!ASSERT_OK(err, "test_run_opts err"))
+		return;
+	if (!ASSERT_OK(topts.retval, "test_run_opts retval"))
+		return;
+
+	if (!ASSERT_OK(bpf_map_get_next_key(map_fd, NULL, key), "get next key"))
+		return;
+
+	do {
+		ASSERT_OK(bpf_map_lookup_elem(map_fd, key, &found), "lookup");
+		ASSERT_TRUE(found, "found test buffer");
+	} while (bpf_map_get_next_key(map_fd, key, key));
+}
+
 void test_dmabuf_iter(void)
 {
 	struct dmabuf_iter *skel = NULL;
+	int iter_fd, map_fd;
 	char buf[256];
-	int iter_fd;
 
 	skel = dmabuf_iter__open_and_load();
 	if (!ASSERT_OK_PTR(skel, "dmabuf_iter__open_and_load"))
 		return;
 
-	if (!ASSERT_OK(create_test_buffers(), "create_buffers"))
+	map_fd = bpf_map__fd(skel->maps.testbuf_hash);
+	if (!ASSERT_OK_FD(map_fd, "map_fd"))
+		goto destroy_skel;
+
+	if (!ASSERT_OK(create_test_buffers(map_fd), "create_buffers"))
 		goto destroy;
 
 	if (!ASSERT_OK(dmabuf_iter__attach(skel), "skel_attach"))
@@ -215,10 +246,13 @@ void test_dmabuf_iter(void)
 
 	if (test__start_subtest("default_iter"))
 		subtest_dmabuf_iter_check_default_iter(skel);
+	if (test__start_subtest("open_coded"))
+		subtest_dmabuf_iter_check_open_coded(skel, map_fd);
 
 	close(iter_fd);
 
 destroy:
 	destroy_test_buffers();
+destroy_skel:
 	dmabuf_iter__destroy(skel);
 }
diff --git a/tools/testing/selftests/bpf/progs/dmabuf_iter.c b/tools/testing/selftests/bpf/progs/dmabuf_iter.c
index d654b4f64cfa..cfdcf4b1c636 100644
--- a/tools/testing/selftests/bpf/progs/dmabuf_iter.c
+++ b/tools/testing/selftests/bpf/progs/dmabuf_iter.c
@@ -9,6 +9,13 @@
 
 char _license[] SEC("license") = "GPL";
 
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(key_size, DMA_BUF_NAME_LEN);
+	__type(value, bool);
+	__uint(max_entries, 5);
+} testbuf_hash SEC(".maps");
+
 /*
  * Fields output by this iterator are delimited by newlines. Convert any
  * newlines in user-provided printed strings to spaces.
@@ -51,3 +58,34 @@ int dmabuf_collector(struct bpf_iter__dmabuf *ctx)
 	BPF_SEQ_PRINTF(seq, "%lu\n%llu\n%s\n%s\n", inode, size, name, exporter);
 	return 0;
 }
+
+SEC("syscall")
+int iter_dmabuf_for_each(const void *ctx)
+{
+	struct dma_buf *d;
+
+	bpf_for_each(dmabuf, d) {
+		char name[DMA_BUF_NAME_LEN];
+		const char *pname;
+		bool *found;
+
+		if (bpf_core_read(&pname, sizeof(pname), &d->name))
+			return 1;
+
+		/* Buffers are not required to be named */
+		if (!pname)
+			continue;
+
+		if (bpf_probe_read_kernel(name, sizeof(name), pname))
+			return 1;
+
+		found = bpf_map_lookup_elem(&testbuf_hash, name);
+		if (found) {
+			bool t = true;
+
+			bpf_map_update_elem(&testbuf_hash, name, &t, BPF_EXIST);
+		}
+	}
+
+	return 0;
+}
-- 
2.49.0.1015.ga840276032-goog

Re: [PATCH bpf-next v4 1/5] dma-buf: Rename debugfs symbols

[Song Liu]

On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
>
> Rename the debugfs list and mutex so it's clear they are now usable
> without the need for CONFIG_DEBUG_FS. The list will always be populated
> to support the creation of a BPF iterator for dmabufs.
>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>

Acked-by: Song Liu <song@kernel.org>

Re: [PATCH bpf-next v4 2/5] bpf: Add dmabuf iterator

[Song Liu]

On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
>
> The dmabuf iterator traverses the list of all DMA buffers.
>
> DMA buffers are refcounted through their associated struct file. A
> reference is taken on each buffer as the list is iterated to ensure each
> buffer persists for the duration of the bpf program execution without
> holding the list mutex.
>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>

Acked-by: Song Liu <song@kernel.org>

With one nitpick below.

> ---
[...]
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 8ff4add71f88..7af2ea839f58 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -634,4 +634,6 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map);
>  void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map);
>  int dma_buf_vmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map);
>  void dma_buf_vunmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map);
> +struct dma_buf *dma_buf_iter_begin(void);
> +struct dma_buf *dma_buf_iter_next(struct dma_buf *dmbuf);
>  #endif /* __DMA_BUF_H__ */
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index 70502f038b92..3a335c50e6e3 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -53,6 +53,9 @@ obj-$(CONFIG_BPF_SYSCALL) += relo_core.o
>  obj-$(CONFIG_BPF_SYSCALL) += btf_iter.o
>  obj-$(CONFIG_BPF_SYSCALL) += btf_relocate.o
>  obj-$(CONFIG_BPF_SYSCALL) += kmem_cache_iter.o
> +ifeq ($(CONFIG_DMA_SHARED_BUFFER),y)
> +obj-$(CONFIG_BPF_SYSCALL) += dmabuf_iter.o
> +endif
>
>  CFLAGS_REMOVE_percpu_freelist.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_bpf_lru_list.o = $(CC_FLAGS_FTRACE)
> diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> new file mode 100644
> index 000000000000..96b4ba7f0b2c
> --- /dev/null
> +++ b/kernel/bpf/dmabuf_iter.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2025 Google LLC */
> +#include <linux/bpf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/dma-buf.h>
> +#include <linux/kernel.h>
> +#include <linux/seq_file.h>
> +
> +BTF_ID_LIST_SINGLE(bpf_dmabuf_btf_id, struct, dma_buf)
> +DEFINE_BPF_ITER_FUNC(dmabuf, struct bpf_iter_meta *meta, struct dma_buf *dmabuf)

nit: It is better to move these two lines later, to where they
are about to be used.

> +
> +static void *dmabuf_iter_seq_start(struct seq_file *seq, loff_t *pos)
> +{
> +       if (*pos)
> +               return NULL;
> +
> +       return dma_buf_iter_begin();
> +}
> +
[...]

Re: [PATCH bpf-next v4 3/5] bpf: Add open coded dmabuf iterator

[Song Liu]

On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
>
> This open coded iterator allows for more flexibility when creating BPF
> programs. It can support output in formats other than text. With an open
> coded iterator, a single BPF program can traverse multiple kernel data
> structures (now including dmabufs), allowing for more efficient analysis
> of kernel data compared to multiple reads from procfs, sysfs, or
> multiple traditional BPF iterator invocations.
>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>

Acked-by: Song Liu <song@kernel.org>

With one nitpick below:

> ---
>  kernel/bpf/dmabuf_iter.c | 47 ++++++++++++++++++++++++++++++++++++++++
>  kernel/bpf/helpers.c     |  5 +++++
>  2 files changed, 52 insertions(+)
>
> diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> index 96b4ba7f0b2c..8049bdbc9efc 100644
> --- a/kernel/bpf/dmabuf_iter.c
> +++ b/kernel/bpf/dmabuf_iter.c
> @@ -100,3 +100,50 @@ static int __init dmabuf_iter_init(void)
>  }
>
>  late_initcall(dmabuf_iter_init);
> +
> +struct bpf_iter_dmabuf {
> +       /* opaque iterator state; having __u64 here allows to preserve correct
> +        * alignment requirements in vmlinux.h, generated from BTF
> +        */

nit: comment style.

> +       __u64 __opaque[1];
> +} __aligned(8);
> +
> +/* Non-opaque version of bpf_iter_dmabuf */
> +struct bpf_iter_dmabuf_kern {
> +       struct dma_buf *dmabuf;
> +} __aligned(8);
> +
[...]

Re: [PATCH bpf-next v4 4/5] selftests/bpf: Add test for dmabuf_iter

[Song Liu]

On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
> new file mode 100644
> index 000000000000..35745f4ce0f8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2025 Google */
> +
> +#include <test_progs.h>
> +#include <bpf/libbpf.h>
> +#include <bpf/btf.h>
> +#include "dmabuf_iter.skel.h"
> +
> +#include <fcntl.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +#include <linux/udmabuf.h>
> +
> +static int memfd, udmabuf;

Global fds are weird. AFAICT, we don't really need them
to be global? If we really need them to be global, please
initialize them to -1, just in case we close(0) by accident.

> +static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] = "udmabuf_test_buffer_for_iter";
> +static size_t udmabuf_test_buffer_size;
> +static int sysheap_dmabuf;
> +static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] = "sysheap_test_buffer_for_iter";
> +static size_t sysheap_test_buffer_size;

Re: [PATCH bpf-next v4 0/5] Replace CONFIG_DMABUF_SYSFS_STATS with BPF

[Christian König]

Feel free to add my Acked-by to the patches which don't have my rb yet.

And ping me when I should upstream this through drm-misc-next, but if you want to upstream this through some other branch then that is fine with me as well.

Regards,
Christian.

On 5/8/25 20:20, T.J. Mercier wrote:
> Until CONFIG_DMABUF_SYSFS_STATS was added [1] it was only possible to
> perform per-buffer accounting with debugfs which is not suitable for
> production environments. Eventually we discovered the overhead with
> per-buffer sysfs file creation/removal was significantly impacting
> allocation and free times, and exacerbated kernfs lock contention. [2]
> dma_buf_stats_setup() is responsible for 39% of single-page buffer
> creation duration, or 74% of single-page dma_buf_export() duration when
> stressing dmabuf allocations and frees.
> 
> I prototyped a change from per-buffer to per-exporter statistics with a
> RCU protected list of exporter allocations that accommodates most (but
> not all) of our use-cases and avoids almost all of the sysfs overhead.
> While that adds less overhead than per-buffer sysfs, and less even than
> the maintenance of the dmabuf debugfs_list, it's still *additional*
> overhead on top of the debugfs_list and doesn't give us per-buffer info.
> 
> This series uses the existing dmabuf debugfs_list to implement a BPF
> dmabuf iterator, which adds no overhead to buffer allocation/free and
> provides per-buffer info. The list has been moved outside of
> CONFIG_DEBUG_FS scope so that it is always populated. The BPF program
> loaded by userspace that extracts per-buffer information gets to define
> its own interface which avoids the lack of ABI stability with debugfs.
> 
> This will allow us to replace our use of CONFIG_DMABUF_SYSFS_STATS, and
> the plan is to remove it from the kernel after the next longterm stable
> release.
> 
> [1] https://lore.kernel.org/linux-media/20201210044400.1080308-1-hridya@google.com
> [2] https://lore.kernel.org/all/20220516171315.2400578-1-tjmercier@google.com
> 
> v1: https://lore.kernel.org/all/20250414225227.3642618-1-tjmercier@google.com
> v1 -> v2:
> Make the DMA buffer list independent of CONFIG_DEBUG_FS per Christian König
> Add CONFIG_DMA_SHARED_BUFFER check to kernel/bpf/Makefile per kernel test robot
> Use BTF_ID_LIST_SINGLE instead of BTF_ID_LIST_GLOBAL_SINGLE per Song Liu
> Fixup comment style, mixing code/declarations, and use ASSERT_OK_FD in selftest per Song Liu
> Add BPF_ITER_RESCHED feature to bpf_dmabuf_reg_info per Alexei Starovoitov
> Add open-coded iterator and selftest per Alexei Starovoitov
> Add a second test buffer from the system dmabuf heap to selftests
> Use the BPF program we'll use in production for selftest per Alexei Starovoitov
>   https://r.android.com/c/platform/system/bpfprogs/+/3616123/2/dmabufIter.c
>   https://r.android.com/c/platform/system/memory/libmeminfo/+/3614259/1/libdmabufinfo/dmabuf_bpf_stats.cpp
> v2: https://lore.kernel.org/all/20250504224149.1033867-1-tjmercier@google.com
> v2 -> v3:
> Rebase onto bpf-next/master
> Move get_next_dmabuf() into drivers/dma-buf/dma-buf.c, along with the
>   new get_first_dmabuf(). This avoids having to expose the dmabuf list
>   and mutex to the rest of the kernel, and keeps the dmabuf mutex
>   operations near each other in the same file. (Christian König)
> Add Christian's RB to dma-buf: Rename debugfs symbols
> Drop RFC: dma-buf: Remove DMA-BUF statistics
> v3: https://lore.kernel.org/all/20250507001036.2278781-1-tjmercier@google.com
> v3 -> v4:
> Fix selftest BPF program comment style (not kdoc) per Alexei Starovoitov
> Fix dma-buf.c kdoc comment style per Alexei Starovoitov
> Rename get_first_dmabuf / get_next_dmabuf to dma_buf_iter_begin /
>   dma_buf_iter_next per Christian König
> Add Christian's RB to bpf: Add dmabuf iterator
> 
> T.J. Mercier (5):
>   dma-buf: Rename debugfs symbols
>   bpf: Add dmabuf iterator
>   bpf: Add open coded dmabuf iterator
>   selftests/bpf: Add test for dmabuf_iter
>   selftests/bpf: Add test for open coded dmabuf_iter
> 
>  drivers/dma-buf/dma-buf.c                     |  98 +++++--
>  include/linux/dma-buf.h                       |   4 +-
>  kernel/bpf/Makefile                           |   3 +
>  kernel/bpf/dmabuf_iter.c                      | 149 ++++++++++
>  kernel/bpf/helpers.c                          |   5 +
>  .../testing/selftests/bpf/bpf_experimental.h  |   5 +
>  tools/testing/selftests/bpf/config            |   3 +
>  .../selftests/bpf/prog_tests/dmabuf_iter.c    | 258 ++++++++++++++++++
>  .../testing/selftests/bpf/progs/dmabuf_iter.c |  91 ++++++
>  9 files changed, 594 insertions(+), 22 deletions(-)
>  create mode 100644 kernel/bpf/dmabuf_iter.c
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
>  create mode 100644 tools/testing/selftests/bpf/progs/dmabuf_iter.c
> 
> 
> base-commit: 43745d11bfd9683abdf08ad7a5cc403d6a9ffd15

Re: [PATCH bpf-next v4 0/5] Replace CONFIG_DMABUF_SYSFS_STATS with BPF

[T.J. Mercier]

On Thu, May 8, 2025 at 11:04 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Feel free to add my Acked-by to the patches which don't have my rb yet.
>
> And ping me when I should upstream this through drm-misc-next, but if you want to upstream this through some other branch then that is fine with me as well.

Thanks Christian. Alexei mentioned he was willing to take the series
through bpf-next here:
https://lore.kernel.org/all/CAADnVQLqv-ZpoQEhk2UwvSZorSLcjgF7qLD76oHguH5-GcSXxA@mail.gmail.com/

I think it makes sense to send the CONFIG_DMABUF_SYSFS_STATS removal
through drm-misc-next though, so I'll resend that as a standalone
patch whenever I hear about the next longterm stable release.

>
> Regards,
> Christian.
>
> On 5/8/25 20:20, T.J. Mercier wrote:
> > Until CONFIG_DMABUF_SYSFS_STATS was added [1] it was only possible to
> > perform per-buffer accounting with debugfs which is not suitable for
> > production environments. Eventually we discovered the overhead with
> > per-buffer sysfs file creation/removal was significantly impacting
> > allocation and free times, and exacerbated kernfs lock contention. [2]
> > dma_buf_stats_setup() is responsible for 39% of single-page buffer
> > creation duration, or 74% of single-page dma_buf_export() duration when
> > stressing dmabuf allocations and frees.
> >
> > I prototyped a change from per-buffer to per-exporter statistics with a
> > RCU protected list of exporter allocations that accommodates most (but
> > not all) of our use-cases and avoids almost all of the sysfs overhead.
> > While that adds less overhead than per-buffer sysfs, and less even than
> > the maintenance of the dmabuf debugfs_list, it's still *additional*
> > overhead on top of the debugfs_list and doesn't give us per-buffer info.
> >
> > This series uses the existing dmabuf debugfs_list to implement a BPF
> > dmabuf iterator, which adds no overhead to buffer allocation/free and
> > provides per-buffer info. The list has been moved outside of
> > CONFIG_DEBUG_FS scope so that it is always populated. The BPF program
> > loaded by userspace that extracts per-buffer information gets to define
> > its own interface which avoids the lack of ABI stability with debugfs.
> >
> > This will allow us to replace our use of CONFIG_DMABUF_SYSFS_STATS, and
> > the plan is to remove it from the kernel after the next longterm stable
> > release.
> >
> > [1] https://lore.kernel.org/linux-media/20201210044400.1080308-1-hridya@google.com
> > [2] https://lore.kernel.org/all/20220516171315.2400578-1-tjmercier@google.com
> >
> > v1: https://lore.kernel.org/all/20250414225227.3642618-1-tjmercier@google.com
> > v1 -> v2:
> > Make the DMA buffer list independent of CONFIG_DEBUG_FS per Christian König
> > Add CONFIG_DMA_SHARED_BUFFER check to kernel/bpf/Makefile per kernel test robot
> > Use BTF_ID_LIST_SINGLE instead of BTF_ID_LIST_GLOBAL_SINGLE per Song Liu
> > Fixup comment style, mixing code/declarations, and use ASSERT_OK_FD in selftest per Song Liu
> > Add BPF_ITER_RESCHED feature to bpf_dmabuf_reg_info per Alexei Starovoitov
> > Add open-coded iterator and selftest per Alexei Starovoitov
> > Add a second test buffer from the system dmabuf heap to selftests
> > Use the BPF program we'll use in production for selftest per Alexei Starovoitov
> >   https://r.android.com/c/platform/system/bpfprogs/+/3616123/2/dmabufIter.c
> >   https://r.android.com/c/platform/system/memory/libmeminfo/+/3614259/1/libdmabufinfo/dmabuf_bpf_stats.cpp
> > v2: https://lore.kernel.org/all/20250504224149.1033867-1-tjmercier@google.com
> > v2 -> v3:
> > Rebase onto bpf-next/master
> > Move get_next_dmabuf() into drivers/dma-buf/dma-buf.c, along with the
> >   new get_first_dmabuf(). This avoids having to expose the dmabuf list
> >   and mutex to the rest of the kernel, and keeps the dmabuf mutex
> >   operations near each other in the same file. (Christian König)
> > Add Christian's RB to dma-buf: Rename debugfs symbols
> > Drop RFC: dma-buf: Remove DMA-BUF statistics
> > v3: https://lore.kernel.org/all/20250507001036.2278781-1-tjmercier@google.com
> > v3 -> v4:
> > Fix selftest BPF program comment style (not kdoc) per Alexei Starovoitov
> > Fix dma-buf.c kdoc comment style per Alexei Starovoitov
> > Rename get_first_dmabuf / get_next_dmabuf to dma_buf_iter_begin /
> >   dma_buf_iter_next per Christian König
> > Add Christian's RB to bpf: Add dmabuf iterator
> >
> > T.J. Mercier (5):
> >   dma-buf: Rename debugfs symbols
> >   bpf: Add dmabuf iterator
> >   bpf: Add open coded dmabuf iterator
> >   selftests/bpf: Add test for dmabuf_iter
> >   selftests/bpf: Add test for open coded dmabuf_iter
> >
> >  drivers/dma-buf/dma-buf.c                     |  98 +++++--
> >  include/linux/dma-buf.h                       |   4 +-
> >  kernel/bpf/Makefile                           |   3 +
> >  kernel/bpf/dmabuf_iter.c                      | 149 ++++++++++
> >  kernel/bpf/helpers.c                          |   5 +
> >  .../testing/selftests/bpf/bpf_experimental.h  |   5 +
> >  tools/testing/selftests/bpf/config            |   3 +
> >  .../selftests/bpf/prog_tests/dmabuf_iter.c    | 258 ++++++++++++++++++
> >  .../testing/selftests/bpf/progs/dmabuf_iter.c |  91 ++++++
> >  9 files changed, 594 insertions(+), 22 deletions(-)
> >  create mode 100644 kernel/bpf/dmabuf_iter.c
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/dmabuf_iter.c
> >
> >
> > base-commit: 43745d11bfd9683abdf08ad7a5cc403d6a9ffd15
>

Re: [PATCH bpf-next v4 2/5] bpf: Add dmabuf iterator

[T.J. Mercier]

On Thu, May 8, 2025 at 5:27 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
> >
> > The dmabuf iterator traverses the list of all DMA buffers.
> >
> > DMA buffers are refcounted through their associated struct file. A
> > reference is taken on each buffer as the list is iterated to ensure each
> > buffer persists for the duration of the bpf program execution without
> > holding the list mutex.
> >
> > Signed-off-by: T.J. Mercier <tjmercier@google.com>
> > Reviewed-by: Christian König <christian.koenig@amd.com>
>
> Acked-by: Song Liu <song@kernel.org>
>
> With one nitpick below.

Thanks!

> > ---
> [...]
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 8ff4add71f88..7af2ea839f58 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -634,4 +634,6 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map);
> >  void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map);
> >  int dma_buf_vmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map);
> >  void dma_buf_vunmap_unlocked(struct dma_buf *dmabuf, struct iosys_map *map);
> > +struct dma_buf *dma_buf_iter_begin(void);
> > +struct dma_buf *dma_buf_iter_next(struct dma_buf *dmbuf);
> >  #endif /* __DMA_BUF_H__ */
> > diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> > index 70502f038b92..3a335c50e6e3 100644
> > --- a/kernel/bpf/Makefile
> > +++ b/kernel/bpf/Makefile
> > @@ -53,6 +53,9 @@ obj-$(CONFIG_BPF_SYSCALL) += relo_core.o
> >  obj-$(CONFIG_BPF_SYSCALL) += btf_iter.o
> >  obj-$(CONFIG_BPF_SYSCALL) += btf_relocate.o
> >  obj-$(CONFIG_BPF_SYSCALL) += kmem_cache_iter.o
> > +ifeq ($(CONFIG_DMA_SHARED_BUFFER),y)
> > +obj-$(CONFIG_BPF_SYSCALL) += dmabuf_iter.o
> > +endif
> >
> >  CFLAGS_REMOVE_percpu_freelist.o = $(CC_FLAGS_FTRACE)
> >  CFLAGS_REMOVE_bpf_lru_list.o = $(CC_FLAGS_FTRACE)
> > diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> > new file mode 100644
> > index 000000000000..96b4ba7f0b2c
> > --- /dev/null
> > +++ b/kernel/bpf/dmabuf_iter.c
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (c) 2025 Google LLC */
> > +#include <linux/bpf.h>
> > +#include <linux/btf_ids.h>
> > +#include <linux/dma-buf.h>
> > +#include <linux/kernel.h>
> > +#include <linux/seq_file.h>
> > +
> > +BTF_ID_LIST_SINGLE(bpf_dmabuf_btf_id, struct, dma_buf)
> > +DEFINE_BPF_ITER_FUNC(dmabuf, struct bpf_iter_meta *meta, struct dma_buf *dmabuf)
>
> nit: It is better to move these two lines later, to where they
> are about to be used.

I've moved them both to just before dmabuf_iter_init() farther down.

Re: [PATCH bpf-next v4 3/5] bpf: Add open coded dmabuf iterator

[T.J. Mercier]

On Thu, May 8, 2025 at 5:28 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
> >
> > This open coded iterator allows for more flexibility when creating BPF
> > programs. It can support output in formats other than text. With an open
> > coded iterator, a single BPF program can traverse multiple kernel data
> > structures (now including dmabufs), allowing for more efficient analysis
> > of kernel data compared to multiple reads from procfs, sysfs, or
> > multiple traditional BPF iterator invocations.
> >
> > Signed-off-by: T.J. Mercier <tjmercier@google.com>
>
> Acked-by: Song Liu <song@kernel.org>
>
> With one nitpick below:
>
> > ---
> >  kernel/bpf/dmabuf_iter.c | 47 ++++++++++++++++++++++++++++++++++++++++
> >  kernel/bpf/helpers.c     |  5 +++++
> >  2 files changed, 52 insertions(+)
> >
> > diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> > index 96b4ba7f0b2c..8049bdbc9efc 100644
> > --- a/kernel/bpf/dmabuf_iter.c
> > +++ b/kernel/bpf/dmabuf_iter.c
> > @@ -100,3 +100,50 @@ static int __init dmabuf_iter_init(void)
> >  }
> >
> >  late_initcall(dmabuf_iter_init);
> > +
> > +struct bpf_iter_dmabuf {
> > +       /* opaque iterator state; having __u64 here allows to preserve correct
> > +        * alignment requirements in vmlinux.h, generated from BTF
> > +        */
>
> nit: comment style.

Added a leading /*

(This is copied from task_iter.c, which currently has the same style.)


> > +       __u64 __opaque[1];
> > +} __aligned(8);
> > +
> > +/* Non-opaque version of bpf_iter_dmabuf */
> > +struct bpf_iter_dmabuf_kern {
> > +       struct dma_buf *dmabuf;
> > +} __aligned(8);
> > +
> [...]

Re: [PATCH bpf-next v4 4/5] selftests/bpf: Add test for dmabuf_iter

[T.J. Mercier]

On Thu, May 8, 2025 at 5:36 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
> [...]
> > diff --git a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
> > new file mode 100644
> > index 000000000000..35745f4ce0f8
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
> > @@ -0,0 +1,224 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2025 Google */
> > +
> > +#include <test_progs.h>
> > +#include <bpf/libbpf.h>
> > +#include <bpf/btf.h>
> > +#include "dmabuf_iter.skel.h"
> > +
> > +#include <fcntl.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/mman.h>
> > +#include <unistd.h>
> > +
> > +#include <linux/dma-buf.h>
> > +#include <linux/dma-heap.h>
> > +#include <linux/udmabuf.h>
> > +
> > +static int memfd, udmabuf;
>
> Global fds are weird. AFAICT, we don't really need them
> to be global? If we really need them to be global, please
> initialize them to -1, just in case we close(0) by accident.

Hmm, no we don't really need them to be global but I didn't really
want to pass all these variables around to all the setup and test
functions. The fd lifetimes are nearly the whole program lifetime
anyways, and just need to exist without actually being used for
anything. I'll add the -1 initialization as you suggest. If udmabuf
creation failed, we would have done a close(0) in
destroy_test_buffers() on the sysheap_dmabuf fd.


> > +static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] = "udmabuf_test_buffer_for_iter";
> > +static size_t udmabuf_test_buffer_size;
> > +static int sysheap_dmabuf;
> > +static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] = "sysheap_test_buffer_for_iter";
> > +static size_t sysheap_test_buffer_size;

Re: [PATCH bpf-next v4 5/5] selftests/bpf: Add test for open coded dmabuf_iter

[Song Liu]

On Thu, May 8, 2025 at 11:21 AM T.J. Mercier <tjmercier@google.com> wrote:
>
> Use the same test buffers as the traditional iterator and a new BPF map
> to verify the test buffers can be found with the open coded dmabuf
> iterator.

The way we split 4/5 and 5/5 makes the code tricker to follow. I guess
the motivation is to back port default iter along to older kernels. But I
think we can still make the code cleaner.

>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>
> ---
[...]

>
> -static int create_udmabuf(void)
> +static int create_udmabuf(int map_fd)
>  {
>         struct udmabuf_create create;
>         int dev_udmabuf;
> +       bool f = false;
>
>         udmabuf_test_buffer_size = 10 * getpagesize();
>
> @@ -63,10 +64,10 @@ static int create_udmabuf(void)
>         if (!ASSERT_OK(ioctl(udmabuf, DMA_BUF_SET_NAME_B, udmabuf_test_buffer_name), "name"))
>                 return 1;
>
> -       return 0;
> +       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name, &f, BPF_ANY);

We don't really need this bpf_map_update_elem() inside
create_udmabuf(), right?

>  }
>
> -static int create_sys_heap_dmabuf(void)
> +static int create_sys_heap_dmabuf(int map_fd)
>  {
>         sysheap_test_buffer_size = 20 * getpagesize();
>
> @@ -77,6 +78,7 @@ static int create_sys_heap_dmabuf(void)
>                 .heap_flags = 0,
>         };
>         int heap_fd, ret;
> +       bool f = false;
>
>         if (!ASSERT_LE(sizeof(sysheap_test_buffer_name), DMA_BUF_NAME_LEN, "NAMETOOLONG"))
>                 return 1;
> @@ -95,18 +97,18 @@ static int create_sys_heap_dmabuf(void)
>         if (!ASSERT_OK(ioctl(sysheap_dmabuf, DMA_BUF_SET_NAME_B, sysheap_test_buffer_name), "name"))
>                 return 1;
>
> -       return 0;
> +       return bpf_map_update_elem(map_fd, sysheap_test_buffer_name, &f, BPF_ANY);

Same for this bpf_map_update_elem(), we can call this directly from
create_test_buffers().

>  }
>
> -static int create_test_buffers(void)
> +static int create_test_buffers(int map_fd)
>  {
>         int ret;
>
> -       ret = create_udmabuf();
> +       ret = create_udmabuf(map_fd);
>         if (ret)
>                 return ret;
>
> -       return create_sys_heap_dmabuf();
> +       return create_sys_heap_dmabuf(map_fd);

Personally, I would prefer we just merge all the logic of
create_udmabuf() and create_sys_heap_dmabuf()
into create_test_buffers().

>  }
>
[...]
> +
>  void test_dmabuf_iter(void)
>  {
>         struct dmabuf_iter *skel = NULL;
> +       int iter_fd, map_fd;
>         char buf[256];
> -       int iter_fd;
>
>         skel = dmabuf_iter__open_and_load();
>         if (!ASSERT_OK_PTR(skel, "dmabuf_iter__open_and_load"))
>                 return;
>
> -       if (!ASSERT_OK(create_test_buffers(), "create_buffers"))
> +       map_fd = bpf_map__fd(skel->maps.testbuf_hash);
> +       if (!ASSERT_OK_FD(map_fd, "map_fd"))
> +               goto destroy_skel;
> +
> +       if (!ASSERT_OK(create_test_buffers(map_fd), "create_buffers"))
>                 goto destroy;
>
>         if (!ASSERT_OK(dmabuf_iter__attach(skel), "skel_attach"))
> @@ -215,10 +246,13 @@ void test_dmabuf_iter(void)
>
>         if (test__start_subtest("default_iter"))
>                 subtest_dmabuf_iter_check_default_iter(skel);
> +       if (test__start_subtest("open_coded"))
> +               subtest_dmabuf_iter_check_open_coded(skel, map_fd);
>
>         close(iter_fd);
>
>  destroy:
>         destroy_test_buffers();
> +destroy_skel:
>         dmabuf_iter__destroy(skel);
>  }
[...]

Re: [PATCH bpf-next v4 4/5] selftests/bpf: Add test for dmabuf_iter

[Song Liu]

On Thu, May 8, 2025 at 11:20 AM T.J. Mercier <tjmercier@google.com> wrote:
[...]

> +
> +void test_dmabuf_iter(void)
> +{
> +       struct dmabuf_iter *skel = NULL;
> +       char buf[256];
> +       int iter_fd;
> +
> +       skel = dmabuf_iter__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "dmabuf_iter__open_and_load"))
> +               return;
> +
> +       if (!ASSERT_OK(create_test_buffers(), "create_buffers"))
> +               goto destroy;
> +
> +       if (!ASSERT_OK(dmabuf_iter__attach(skel), "skel_attach"))
> +               goto destroy;

From here...

> +       iter_fd = bpf_iter_create(bpf_link__fd(skel->links.dmabuf_collector));
> +       if (!ASSERT_OK_FD(iter_fd, "iter_create"))
> +               goto destroy;
> +
> +       while (read(iter_fd, buf, sizeof(buf)) > 0)
> +               ; /* Read out all contents */
> +
> +       /* Next reads should return 0 */
> +       ASSERT_EQ(read(iter_fd, buf, sizeof(buf)), 0, "read");

to here, can be a separate subtest. Then iter_fd can be moved to
that subtest.

> +
> +       if (test__start_subtest("default_iter"))
> +               subtest_dmabuf_iter_check_default_iter(skel);
> +
> +       close(iter_fd);
> +
> +destroy:
> +       destroy_test_buffers();
> +       dmabuf_iter__destroy(skel);
[...]

Re: [PATCH bpf-next v4 5/5] selftests/bpf: Add test for open coded dmabuf_iter

[T.J. Mercier]

On Fri, May 9, 2025 at 11:46 AM Song Liu <song@kernel.org> wrote:
>
> On Thu, May 8, 2025 at 11:21 AM T.J. Mercier <tjmercier@google.com> wrote:
> >
> > Use the same test buffers as the traditional iterator and a new BPF map
> > to verify the test buffers can be found with the open coded dmabuf
> > iterator.
>
> The way we split 4/5 and 5/5 makes the code tricker to follow. I guess
> the motivation is to back port default iter along to older kernels. But I
> think we can still make the code cleaner.
>
> >
> > Signed-off-by: T.J. Mercier <tjmercier@google.com>
> > ---
> [...]
>
> >
> > -static int create_udmabuf(void)
> > +static int create_udmabuf(int map_fd)
> >  {
> >         struct udmabuf_create create;
> >         int dev_udmabuf;
> > +       bool f = false;
> >
> >         udmabuf_test_buffer_size = 10 * getpagesize();
> >
> > @@ -63,10 +64,10 @@ static int create_udmabuf(void)
> >         if (!ASSERT_OK(ioctl(udmabuf, DMA_BUF_SET_NAME_B, udmabuf_test_buffer_name), "name"))
> >                 return 1;
> >
> > -       return 0;
> > +       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name, &f, BPF_ANY);
>
> We don't really need this bpf_map_update_elem() inside
> create_udmabuf(), right?
>
> >  }
> >
> > -static int create_sys_heap_dmabuf(void)
> > +static int create_sys_heap_dmabuf(int map_fd)
> >  {
> >         sysheap_test_buffer_size = 20 * getpagesize();
> >
> > @@ -77,6 +78,7 @@ static int create_sys_heap_dmabuf(void)
> >                 .heap_flags = 0,
> >         };
> >         int heap_fd, ret;
> > +       bool f = false;
> >
> >         if (!ASSERT_LE(sizeof(sysheap_test_buffer_name), DMA_BUF_NAME_LEN, "NAMETOOLONG"))
> >                 return 1;
> > @@ -95,18 +97,18 @@ static int create_sys_heap_dmabuf(void)
> >         if (!ASSERT_OK(ioctl(sysheap_dmabuf, DMA_BUF_SET_NAME_B, sysheap_test_buffer_name), "name"))
> >                 return 1;
> >
> > -       return 0;
> > +       return bpf_map_update_elem(map_fd, sysheap_test_buffer_name, &f, BPF_ANY);
>
> Same for this bpf_map_update_elem(), we can call this directly from
> create_test_buffers().
>
> >  }
> >
> > -static int create_test_buffers(void)
> > +static int create_test_buffers(int map_fd)
> >  {
> >         int ret;
> >
> > -       ret = create_udmabuf();
> > +       ret = create_udmabuf(map_fd);
> >         if (ret)
> >                 return ret;
> >
> > -       return create_sys_heap_dmabuf();
> > +       return create_sys_heap_dmabuf(map_fd);
>
> Personally, I would prefer we just merge all the logic of
> create_udmabuf() and create_sys_heap_dmabuf()
> into create_test_buffers().

That's a lot of different stuff to put in one place. How about
returning file descriptors from the buffer create functions while
having them clean up after themselves:

-static int memfd, udmabuf;
+static int udmabuf;
 static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] =
"udmabuf_test_buffer_for_iter";
 static size_t udmabuf_test_buffer_size;
 static int sysheap_dmabuf;
 static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] =
"sysheap_test_buffer_for_iter";
 static size_t sysheap_test_buffer_size;

-static int create_udmabuf(int map_fd)
+static int create_udmabuf(void)
 {
        struct udmabuf_create create;
-       int dev_udmabuf;
-       bool f = false;
+       int dev_udmabuf, memfd, udmabuf;

        udmabuf_test_buffer_size = 10 * getpagesize();

        if (!ASSERT_LE(sizeof(udmabuf_test_buffer_name),
DMA_BUF_NAME_LEN, "NAMETOOLONG"))
-               return 1;
+               return -1;

        memfd = memfd_create("memfd_test", MFD_ALLOW_SEALING);
        if (!ASSERT_OK_FD(memfd, "memfd_create"))
-               return 1;
+               return -1;

        if (!ASSERT_OK(ftruncate(memfd, udmabuf_test_buffer_size), "ftruncate"))
-               return 1;
+               goto close_memfd;

        if (!ASSERT_OK(fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK), "seal"))
-               return 1;
+               goto close_memfd;

        dev_udmabuf = open("/dev/udmabuf", O_RDONLY);
        if (!ASSERT_OK_FD(dev_udmabuf, "open udmabuf"))
-               return 1;
+               goto close_memfd;

        create.memfd = memfd;
        create.flags = UDMABUF_FLAGS_CLOEXEC;
@@ -59,15 +58,21 @@ static int create_udmabuf(int map_fd)
        udmabuf = ioctl(dev_udmabuf, UDMABUF_CREATE, &create);
        close(dev_udmabuf);
        if (!ASSERT_OK_FD(udmabuf, "udmabuf_create"))
-               return 1;
+               goto close_memfd;

        if (!ASSERT_OK(ioctl(udmabuf, DMA_BUF_SET_NAME_B,
udmabuf_test_buffer_name), "name"))
-               return 1;
+               goto close_udmabuf;
+
+       return udmabuf;

-       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name,
&f, BPF_ANY);
+close_udmabuf:
+       close(udmabuf);
+close_memfd:
+       close(memfd);
+       return -1;
 }

-static int create_sys_heap_dmabuf(int map_fd)
+static int create_sys_heap_dmabuf(void)
 {
        sysheap_test_buffer_size = 20 * getpagesize();

@@ -78,43 +83,46 @@ static int create_sys_heap_dmabuf(int map_fd)
                .heap_flags = 0,
        };
        int heap_fd, ret;
-       bool f = false;

        if (!ASSERT_LE(sizeof(sysheap_test_buffer_name),
DMA_BUF_NAME_LEN, "NAMETOOLONG"))
-               return 1;
+               return -1;

        heap_fd = open("/dev/dma_heap/system", O_RDONLY);
        if (!ASSERT_OK_FD(heap_fd, "open dma heap"))
-               return 1;
+               return -1;

        ret = ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &data);
        close(heap_fd);
        if (!ASSERT_OK(ret, "syheap alloc"))
-               return 1;
+               return -1;

-       sysheap_dmabuf = data.fd;
+       if (!ASSERT_OK(ioctl(data.fd, DMA_BUF_SET_NAME_B,
sysheap_test_buffer_name), "name"))
+               goto close_sysheap_dmabuf;

-       if (!ASSERT_OK(ioctl(sysheap_dmabuf, DMA_BUF_SET_NAME_B,
sysheap_test_buffer_name), "name"))
-               return 1;
+       return data.fd;

-       return bpf_map_update_elem(map_fd, sysheap_test_buffer_name,
&f, BPF_ANY);
+close_sysheap_dmabuf:
+       close(data.fd);
+       return -1;
 }

 static int create_test_buffers(int map_fd)
 {
-       int ret;
+       bool f = false;
+
+       udmabuf = create_udmabuf();
+       sysheap_dmabuf = create_sys_heap_dmabuf();

-       ret = create_udmabuf(map_fd);
-       if (ret)
-               return ret;
+       if (udmabuf < 0 || sysheap_dmabuf < 0)
+               return -1;

-       return create_sys_heap_dmabuf(map_fd);
+       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name,
&f, BPF_ANY) ||
+              bpf_map_update_elem(map_fd, sysheap_test_buffer_name,
&f, BPF_ANY);
 }

 static void destroy_test_buffers(void)
 {
        close(udmabuf);
-       close(memfd);
        close(sysheap_dmabuf);
 }

Re: [PATCH bpf-next v4 5/5] selftests/bpf: Add test for open coded dmabuf_iter

[Song Liu]

On Fri, May 9, 2025 at 2:43 PM T.J. Mercier <tjmercier@google.com> wrote:
>
[...]
> >
> > Personally, I would prefer we just merge all the logic of
> > create_udmabuf() and create_sys_heap_dmabuf()
> > into create_test_buffers().
>
> That's a lot of different stuff to put in one place. How about
> returning file descriptors from the buffer create functions while
> having them clean up after themselves:

I do like this version better. Some nitpicks though.

>
> -static int memfd, udmabuf;
> +static int udmabuf;

About this, and ...

>  static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] =
> "udmabuf_test_buffer_for_iter";
>  static size_t udmabuf_test_buffer_size;
>  static int sysheap_dmabuf;
>  static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] =
> "sysheap_test_buffer_for_iter";
>  static size_t sysheap_test_buffer_size;
>
> -static int create_udmabuf(int map_fd)
> +static int create_udmabuf(void)
>  {
>         struct udmabuf_create create;
> -       int dev_udmabuf;
> -       bool f = false;
> +       int dev_udmabuf, memfd, udmabuf;
.. here.

It is not ideal to have a global udmabuf and a local udmabuf.
If we want the global version, let's rename the local one.

[...]

>
>  static int create_test_buffers(int map_fd)
>  {
> -       int ret;
> +       bool f = false;
> +
> +       udmabuf = create_udmabuf();
> +       sysheap_dmabuf = create_sys_heap_dmabuf();
>
> -       ret = create_udmabuf(map_fd);
> -       if (ret)
> -               return ret;
> +       if (udmabuf < 0 || sysheap_dmabuf < 0)
> +               return -1;

We also need destroy_test_buffers() on the error path here,
or at the caller.

>
> -       return create_sys_heap_dmabuf(map_fd);
> +       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name,
> &f, BPF_ANY) ||
> +              bpf_map_update_elem(map_fd, sysheap_test_buffer_name,
> &f, BPF_ANY);
>  }
>
>  static void destroy_test_buffers(void)
>  {
>         close(udmabuf);
> -       close(memfd);
>         close(sysheap_dmabuf);

For the two global fds, let's reset them to -1 right after close().

Thanks,
Song

Re: [PATCH bpf-next v4 5/5] selftests/bpf: Add test for open coded dmabuf_iter

[T.J. Mercier]

On Fri, May 9, 2025 at 2:58 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, May 9, 2025 at 2:43 PM T.J. Mercier <tjmercier@google.com> wrote:
> >
> [...]
> > >
> > > Personally, I would prefer we just merge all the logic of
> > > create_udmabuf() and create_sys_heap_dmabuf()
> > > into create_test_buffers().
> >
> > That's a lot of different stuff to put in one place. How about
> > returning file descriptors from the buffer create functions while
> > having them clean up after themselves:
>
> I do like this version better. Some nitpicks though.
>
> >
> > -static int memfd, udmabuf;
> > +static int udmabuf;
>
> About this, and ...
>
> >  static const char udmabuf_test_buffer_name[DMA_BUF_NAME_LEN] =
> > "udmabuf_test_buffer_for_iter";
> >  static size_t udmabuf_test_buffer_size;
> >  static int sysheap_dmabuf;
> >  static const char sysheap_test_buffer_name[DMA_BUF_NAME_LEN] =
> > "sysheap_test_buffer_for_iter";
> >  static size_t sysheap_test_buffer_size;
> >
> > -static int create_udmabuf(int map_fd)
> > +static int create_udmabuf(void)
> >  {
> >         struct udmabuf_create create;
> > -       int dev_udmabuf;
> > -       bool f = false;
> > +       int dev_udmabuf, memfd, udmabuf;
> .. here.
>
> It is not ideal to have a global udmabuf and a local udmabuf.
> If we want the global version, let's rename the local one.

Ok let me change up the name of the aliasing variable to local_udmabuf.

> [...]
>
> >
> >  static int create_test_buffers(int map_fd)
> >  {
> > -       int ret;
> > +       bool f = false;
> > +
> > +       udmabuf = create_udmabuf();
> > +       sysheap_dmabuf = create_sys_heap_dmabuf();
> >
> > -       ret = create_udmabuf(map_fd);
> > -       if (ret)
> > -               return ret;
> > +       if (udmabuf < 0 || sysheap_dmabuf < 0)
> > +               return -1;
>
> We also need destroy_test_buffers() on the error path here,
> or at the caller.

The caller does currently check to decide if it should bother running
the tests or not, and calls destroy_test_buffers() if not.

> > -       return create_sys_heap_dmabuf(map_fd);
> > +       return bpf_map_update_elem(map_fd, udmabuf_test_buffer_name,
> > &f, BPF_ANY) ||
> > +              bpf_map_update_elem(map_fd, sysheap_test_buffer_name,
> > &f, BPF_ANY);
> >  }
> >
> >  static void destroy_test_buffers(void)
> >  {
> >         close(udmabuf);
> > -       close(memfd);
> >         close(sysheap_dmabuf);
>
> For the two global fds, let's reset them to -1 right after close().
>
> Thanks,
> Song

Will do, thanks.