Re: [PATCH] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Dec 10, 2020 at 1:06 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Dec 10, 2020 at 12:26:01PM +0100, Daniel Vetter wrote:
> > On Thu, Dec 10, 2020 at 11:55 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Dec 10, 2020 at 11:27:27AM +0100, Daniel Vetter wrote:
> > > > On Thu, Dec 10, 2020 at 11:10:45AM +0100, Greg KH wrote:
> > > > > On Thu, Dec 10, 2020 at 10:58:50AM +0100, Christian König wrote:
> > > > > > In general a good idea, but I have a few concern/comments here.
> > > > > >
> > > > > > Am 10.12.20 um 05:43 schrieb Hridya Valsaraju:
> > > > > > > This patch allows statistics to be enabled for each DMA-BUF in
> > > > > > > sysfs by enabling the config CONFIG_DMABUF_SYSFS_STATS.
> > > > > > >
> > > > > > > The following stats will be exposed by the interface:
> > > > > > >
> > > > > > > /sys/kernel/dmabuf/<inode_number>/exporter_name
> > > > > > > /sys/kernel/dmabuf/<inode_number>/size
> > > > > > > /sys/kernel/dmabuf/<inode_number>/dev_map_info
> > > > > > >
> > > > > > > The inode_number is unique for each DMA-BUF and was added earlier [1]
> > > > > > > in order to allow userspace to track DMA-BUF usage across different
> > > > > > > processes.
> > > > > > >
> > > > > > > Currently, this information is exposed in
> > > > > > > /sys/kernel/debug/dma_buf/bufinfo.
> > > > > > > However, since debugfs is considered unsafe to be mounted in production,
> > > > > > > it is being duplicated in sysfs.
> > > > > >
> > > > > > Mhm, this makes it part of the UAPI. What is the justification for this?
> > > > > >
> > > > > > In other words do we really need those debug information in a production
> > > > > > environment?
> > > > >
> > > > > Production environments seem to want to know who is using up memory :)
> > > >
> > > > This only shows shared memory, so it does smell a lot like $specific_issue
> > > > and we're designing a narrow solution for that and then have to carry it
> > > > forever.
> > >
> > > I think the "issue" is that this was a feature from ion that people
> > > "missed" in the dmabuf move.  Taking away the ability to see what kind
> > > of allocations were being made didn't make a lot of debugging tools
> > > happy :(
> >
> > If this is just for dma-heaps then why don't we add the stuff back
> > over there? It reinforces more that the android gpu stack and the
> > non-android gpu stack on linux are fairly different in fundamental
> > ways, but that's not really new.
>
> Back "over where"?
>
> dma-bufs are not only used for the graphics stack on android from what I
> can tell, so this shouldn't be a gpu-specific issue.

dma-buf heaps exist because android, mostly because google mandates
it. There's not a whole lot (meaning zero) of actually open gpu stacks
around that run on android and use dma-buf heaps like approved google
systems, largely because the gralloc implementation in mesa just
doesnt.

So if android needs some quick debug output in sysfs, we can just add
that in dma-buf heaps, for android only, problem solved. And much less
annoying review to make sure it actually fits into the wider ecosystem
because as-is (and I'm not seeing that chance anytime soon), dma-buf
heaps is for android only. dma-buf at large isn't, so merging a debug
output sysfs api that's just for android but misses a ton of the more
generic features and semantics of dma-buf is not great.
-Daniel

-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch