Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 22 Mar 2019 08:29:47 -0400
From:   Joel Fernandes <joel@joelfernandes.org>
To:     Chenbo Feng <fengc@google.com>
Cc:     linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-media@vger.kernel.org, kernel-team@android.com,
        Sumit Semwal <sumit.semwal@linaro.org>, erickreyes@google.com,
        Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [RFC v2 dma-buf 0/3] Improve the dma-buf tracking
Message-ID: <20190322122947.GA47151@google.com>
References: <20190322025135.118201-1-fengc@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190322025135.118201-1-fengc@google.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 21, 2019 at 07:51:32PM -0700, Chenbo Feng 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

Is there a better place to add bufinfo than debugfs, since debugfs is not
something Android may mount for non-debug uses in the future?

> 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.

There are a couple more entries shown in fdinfo per patch 3/3 so it is worth
mentioning those here as well.

Also, is there kselftests to add or modify? I suggest we add those since they
are always invaluable.

I'll review more soon. Currently traveling. I am especially curious how you
created the new inodes since I recently had to do so for another usecase as
well. thanks!

 - Joel

> 
> Change in v2:
> * Add a check to prevent changing dma-buf name when it is attached to
>   devices.
> * Fixed some compile warnings
> 
> 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
> 
>  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
>