Re: [RFC dma-buf 0/3] Improve the dma-buf tracking

[Daniel Vetter]

On Thu, Mar 14, 2019 at 6:49 PM Sumit Semwal <sumit.semwal@xxxxxxxxxx> wrote:
>
> Hello Chenbo,Thank you for your RFC series.
>
> On Wed, 27 Feb 2019 at 09:24, Chenbo Feng <fengc@xxxxxxxxxx> wrote:
> >
> > Currently, all dma-bufs share the same anonymous inode. While we can count
> > how many dma-buf fds or mappings a process has, we can't get the size of
> > the backing buffers or tell if two entries point to the same dma-buf. And
> > in debugfs, we can get a per-buffer breakdown of size and reference count,
> > but can't tell which processes are actually holding the references to each
> > buffer.
> >
> > To resolve the issue above and provide better method for userspace to track
> > the dma-buf usage across different processes, the following changes are
> > proposed in dma-buf kernel side. First of all, replace the singleton inode
> > inside the dma-buf subsystem with a mini-filesystem, and assign each
> > dma-buf a unique inode out of this filesystem.  With this change, calling
> > stat(2) on each entry gives the caller a unique ID (st_ino), the buffer's
> > size (st_size), and even the number of pages assigned to each dma-buffer.
> > Secoundly, add the inode information to /sys/kernel/debug/dma_buf/bufinfo
> > so in the case where a buffer is mmap()ed into a processâs address space
> > but all remaining fds have been closed, we can still get the dma-buf
> > information and try to accociate it with the process by searching the
> > proc/pid/maps and looking for the corresponding inode number exposed in
> > dma-buf debug fs. Thirdly, created an ioctl to assign names to dma-bufs
> > which lets userspace assign short names (e.g., "CAMERA") to buffers. This
> > information can be extremely helpful for tracking and accounting shared
> > buffers based on their usage and original purpose. Last but not least, add
> > dma-buf information to /proc/pid/fdinfo by adding a show_fdinfo() handler
> > to dma_buf_file_operations. The handler will print the file_count and name
> > of each buffer.
>
> In general, I think I like the idea as it contributes to a much more
> relevant usage analysis of dma-buf backed buffers.
> I will get to doing a more detailed review soon, but immediately, we
> might want to think a bit about the get/set_name IOCTLS - do we need
> to think of disallowing multiple renaming of buffers once they start
> getting used? It could otherwise make the whole metrics a lot
> confused?
>
> >
> > Greg Hackmann (3):
> >   dma-buf: give each buffer a full-fledged inode
> >   dma-buf: add DMA_BUF_{GET,SET}_NAME ioctls
> >   dma-buf: add show_fdinfo handler

I'm not seeing the patches, so just quick comment here: Some drivers
(at least vc4) already have the concept of buffer names. Would be neat
to integrate this between dma-buf and drm_gem in prime.

Aside from that sounds like a good idea overall, but I can't see any details.
-Daniel

> >
> >  drivers/dma-buf/dma-buf.c    | 121 ++++++++++++++++++++++++++++++++---
> >  include/linux/dma-buf.h      |   5 +-
> >  include/uapi/linux/dma-buf.h |   4 ++
> >  include/uapi/linux/magic.h   |   1 +
> >  4 files changed, 122 insertions(+), 9 deletions(-)
> >
> > --
> > 2.21.0.rc2.261.ga7da99ff1b-goog
> >
>
> Best,
> Sumit.
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch