On Tue, Apr 22, 2025 at 4:01 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:

On Tue, Apr 22, 2025 at 12:57 PM T.J. Mercier tjmercier@google.com wrote:

...

On Mon, Apr 21, 2025 at 4:39 PM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:

...

On Mon, Apr 21, 2025 at 1:40 PM T.J. Mercier tjmercier@google.com wrote:

...

...

...

new file mode 100644 index 000000000000..b4b8be1d6aa4 --- /dev/null +++ b/kernel/bpf/dmabuf_iter.c

Maybe we should add this file to drivers/dma-buf. I would like to hear other folks thoughts on this.

This is fine with me, and would save us the extra CONFIG_DMA_SHARED_BUFFER check that's currently needed in kernel/bpf/Makefile but would require checking CONFIG_BPF instead. Sumit / Christian any objections to moving the dmabuf bpf iterator implementation into drivers/dma-buf?

The driver directory would need to 'depends on BPF_SYSCALL'. Are you sure you want this? imo kernel/bpf/ is fine for this.

I don't have a strong preference so either way is fine with me. The main difference I see is maintainership.

...

You also probably want .feature = BPF_ITER_RESCHED in bpf_dmabuf_reg_info.

Thank you, this looks like a good idea.

...

Also have you considered open coded iterator for dmabufs? Would it help with the interface to user space?

I read through the open coded iterator patches, and it looks like they would be slightly more efficient by avoiding seq_file overhead. As far as the interface to userspace, for the purpose of replacing what's currently exposed by CONFIG_DMABUF_SYSFS_STATS I don't think there is a difference. However it looks like if I were to try to replace all of our userspace analysis of dmabufs with a single bpf program then an open coded iterator would make that much easier. I had not considered attempting that.

One problem I see with open coded iterators is that support is much more recent (2023 vs 2020). We support longterm stable kernels (back to 5.4 currently but probably 5.10 by the time this would be used), so it seems like it would be harder to backport the kernel support for an open-coded iterator that far since it only goes back as far as 6.6 now. Actually it doesn't look like it is possible while also maintaining the stable ABI we provide to device vendors. Which means we couldn't get rid of the dmabuf sysfs stats userspace dependency until 6.1 EOL in Dec. 2027. :\ So I'm in favor of a traditional bpf iterator here for now.

Fair enough, but please implement both and backport only the old style pinned iterator.

Ok, will do.

The code will be mostly shared between them. bpf_iter_dmabuf_new/_next will be more flexible with more options to return data to user space. Like android can invent their own binary format. Pack into it in a bpf prog, send to bpf ringbuf and unmarshal efficiently in user space. Instead of being limited to text output that pinned iterators are supposed to do usually.

Also a neat idea!

You can do binary with bpf_seq_write() too, but it's rare.

Also please provide full bpf prog that you'll use in production in a selftest instead of trivial: +SEC("iter/dmabuf") +int dmabuf_collector(struct bpf_iter__dmabuf *ctx)

just to make sure it's tested end to end and future changes won't break it.

The final bpf program should be something pretty close to that, but I'll start working on the AOSP side as well so I can put up patches.

pw-bot: cr