On 5/7/25 19:36, T.J. Mercier wrote:
On Wed, May 7, 2025 at 1:15 AM Christian König christian.koenig@amd.com wrote:
On 5/7/25 02:10, T.J. Mercier 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
drivers/dma-buf/dma-buf.c | 64 ++++++++++++++++++++++++ include/linux/dma-buf.h | 3 ++ kernel/bpf/Makefile | 3 ++ kernel/bpf/dmabuf_iter.c | 102 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 172 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..9fee2788924e 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,68 @@ static void __dma_buf_list_del(struct dma_buf *dmabuf) mutex_unlock(&dmabuf_list_mutex); }
+/**
- get_first_dmabuf - begin iteration through global list of DMA-bufs
As far as I can see that looks really good.
The only thing I'm questioning a little bit is that the name get_first_dmabuf() just doesn't sound so well to me.
I'm a fan of keeping the object you work with first in the naming and it should somehow express that this iters over the global list of all buffers. Maybe something like dmabuf_get_first_globally or dmabuf_get_first_instance.
Feel free to add my rb if any of those suggestions are used, but I'm completely open other ideas as well.
Regards, Christian.
Yeah you're right. "first" is actually a little misleading too, since the most recently exported buffer will be at the list head, not the oldest buffer. But buffer age or ordering doesn't really matter here as long as we get through all of them.
So I'm thinking dma_buf_iter_begin() and dma_buf_iter_next() would be better names. Similar to seq_start / seq_next or bpf's iter_<type>_new / iter_<type>_next.
Yeah, dma_buf_iter_begin/next works for me as well.
Feel free to add my rb when you use those names.
Regards, Christian.
- 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 get_next_dmabuf(), or with dma_buf_put().
- Returns NULL If no active buffers are present.
- */
+struct dma_buf *get_first_dmabuf(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;+}
+/**
- get_next_dmabuf - continue iteration through global list of DMA-bufs
- @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 get_next_dmabuf(), or with dma_buf_put().
- Returns NULL If no additional active buffers are present.
- */
+struct dma_buf *get_next_dmabuf(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..1820f6db6e58 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -568,6 +568,9 @@ static inline void get_dma_buf(struct dma_buf *dmabuf) get_file(dmabuf->file); }
+struct dma_buf *get_first_dmabuf(void); +struct dma_buf *get_next_dmabuf(struct dma_buf *dmbuf);
/**
- dma_buf_is_dynamic - check if a DMA-buf uses dynamic mappings.
- @dmabuf: the DMA-buf to check
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..80bca8239c6d --- /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 get_first_dmabuf();+}
+static void *dmabuf_iter_seq_next(struct seq_file *seq, void *v, loff_t *pos) +{
struct dma_buf *dmabuf = v;++*pos;return get_next_dmabuf(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);