Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 10 Dec 2020 11:27:27 +0100
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     Christian =?iso-8859-1?Q?K=F6nig?= <christian.koenig@amd.com>,
        surenb@google.com, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
        Hridya Valsaraju <hridya@google.com>, kernel-team@android.com,
        linux-media@vger.kernel.org
Subject: Re: [PATCH] dmabuf: Add the capability to expose DMA-BUF stats in
 sysfs
Message-ID: <20201210102727.GE401619@phenom.ffwll.local>
Mail-Followup-To: Greg KH <gregkh@linuxfoundation.org>,
        Christian =?iso-8859-1?Q?K=F6nig?= <christian.koenig@amd.com>,
        surenb@google.com, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
        Hridya Valsaraju <hridya@google.com>, kernel-team@android.com,
        linux-media@vger.kernel.org
References: <20201210044400.1080308-1-hridya@google.com>
 <b5adfe46-8615-5821-d092-2b93feed5b79@amd.com>
 <X9H0JREcdxDsMtLX@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <X9H0JREcdxDsMtLX@kroah.com>
Precedence: bulk

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.

E.g. why is the list of attachments not a sysfs link? That's how we
usually expose struct device * pointers in sysfs to userspace, not as a
list of things.

Furthermore we don't have the exporter device covered anywhere, how is
that tracked? Yes Android just uses ion for all shared buffers, but that's
not how all of linux userspace works.

Then I guess there's the mmaps, you can fish them out of procfs. A tool
which collects all that information might be useful, just as demonstration
of how this is all supposed to be used.

Finally we have kernel internal mappings too. Not tracked.

There's also some things to make sure we're at least having thought about
how other things fit in here. E.d. dma_resv attached to the dma-buf
matters in general a lot. It doesn't matter on Android because
everything's pinned all the time anyway.

Also I thought sysfs was one value one file, dumping an entire list into
dev_info_map with properties we'll need to extend (once you care about
dma_resv you also want to know which attachments are dynamic) does not
smell like sysfs design at all.

So yeah, why? worksformeonandroidweneeditthere not good enough for uapi of
something this core to how the gpu stack works on linux in general, at
least imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch