lkml.org 
[lkml]   [2022]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats
On Tue, May 10, 2022 at 7:07 AM Charan Teja Kalla
<quic_charante@quicinc.com> wrote:
>
> The dmabuf file uses get_next_ino()(through dma_buf_getfile() ->
> alloc_anon_inode()) to get an inode number and uses the same as a
> directory name under /sys/kernel/dmabuf/buffers/<ino>. This directory is
> used to collect the dmabuf stats and it is created through
> dma_buf_stats_setup(). At current, failure to create this directory
> entry can make the dma_buf_export() to fail.
>
> Now, as the get_next_ino() can definitely give a repetitive inode no
> causing the directory entry creation to fail with -EEXIST. This is a
> problem on the systems where dmabuf stats functionality is enabled on
> the production builds can make the dma_buf_export(), though the dmabuf
> memory is allocated successfully, to fail just because it couldn't
> create stats entry.
>
> This issue we are able to see on the snapdragon system within 13 days
> where there already exists a directory with inode no "122602" so
> dma_buf_stats_setup() failed with -EEXIST as it is trying to create
> the same directory entry.
>
> To make the directory entry as unique, append the unique_id for every
> inode. With this change the stats directory entries will be in the
> format of: /sys/kernel/dmabuf/buffers/<inode_number-unique_id>.
>
> Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>
> ---
> Changes in V2:
> -- Used the atomic64_t variable to generate a unique_id to be appended to inode
> to have an unique directory with name <inode_number-unique_id> -- Suggested by christian
> -- Updated the ABI documentation -- Identified by Greg.
> -- Massaged the commit log.
>
> Changes in V1:
> -- Used the inode->i_ctime->tv_secs as an id appended to inode to create the
> unique directory with name <inode_number-time_in_secs>.
> -- https://lore.kernel.org/all/1652178212-22383-1-git-send-email-quic_charante@quicinc.com/
>
> Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers | 10 +++++-----
> drivers/dma-buf/Kconfig | 6 +++---
> drivers/dma-buf/dma-buf-sysfs-stats.c | 8 +++++---
> 3 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> index 5d3bc99..9fffbd3 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> @@ -4,19 +4,19 @@ KernelVersion: v5.13
> Contact: Hridya Valsaraju <hridya@google.com>
> Description: The /sys/kernel/dmabuf/buffers directory contains a
> snapshot of the internal state of every DMA-BUF.
> - /sys/kernel/dmabuf/buffers/<inode_number> will contain the
> - statistics for the DMA-BUF with the unique inode number
> - <inode_number>
> + /sys/kernel/dmabuf/buffers/<inode_number-unique_id> will
> + contain the statistics for the DMA-BUF with the unique
> + pair <inode_number-unique_id>

Android userspace does have a dependency on this being an inode
number. Or at least, a single unsigned int. Not the end of the world,
but still... this will break.
https://cs.android.com/android/platform/superproject/+/master:system/memory/libmeminfo/libdmabufinfo/dmabuf_sysfs_stats.cpp;l=76-77;drc=6951984bbefb96423970b82005ae381065e36704

> Users: kernel memory tuning/debugging tools
>
> -What: /sys/kernel/dmabuf/buffers/<inode_number>/exporter_name
> +What: /sys/kernel/dmabuf/buffers/<inode_number-unique_id>/exporter_name
> Date: May 2021
> KernelVersion: v5.13
> Contact: Hridya Valsaraju <hridya@google.com>
> Description: This file is read-only and contains the name of the exporter of
> the DMA-BUF.
>
> -What: /sys/kernel/dmabuf/buffers/<inode_number>/size
> +What: /sys/kernel/dmabuf/buffers/<inode_number-unique_id>/size
> Date: May 2021
> KernelVersion: v5.13
> Contact: Hridya Valsaraju <hridya@google.com>
> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
> index 541efe0..5bcbdb1 100644
> --- a/drivers/dma-buf/Kconfig
> +++ b/drivers/dma-buf/Kconfig
> @@ -81,9 +81,9 @@ menuconfig DMABUF_SYSFS_STATS
> Choose this option to enable DMA-BUF sysfs statistics
> in location /sys/kernel/dmabuf/buffers.
>
> - /sys/kernel/dmabuf/buffers/<inode_number> will contain
> - statistics for the DMA-BUF with the unique inode number
> - <inode_number>.
> + /sys/kernel/dmabuf/buffers/<inode_number-unique_id> will contain
> + statistics for the DMA-BUF with the unique pair
> + <inode_number-unique_id>.
>
> source "drivers/dma-buf/heaps/Kconfig"
>
> diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-buf-sysfs-stats.c
> index 2bba0ba..29e9e23 100644
> --- a/drivers/dma-buf/dma-buf-sysfs-stats.c
> +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
> @@ -38,8 +38,8 @@
> *
> * The following stats are exposed by the interface:
> *
> - * * ``/sys/kernel/dmabuf/buffers/<inode_number>/exporter_name``
> - * * ``/sys/kernel/dmabuf/buffers/<inode_number>/size``
> + * * ``/sys/kernel/dmabuf/buffers/<inode_number-unique_id>/exporter_name``
> + * * ``/sys/kernel/dmabuf/buffers/<inode_number-unique_id>/size``
> *
> * The information in the interface can also be used to derive per-exporter
> * statistics. The data from the interface can be gathered on error conditions
> @@ -172,6 +172,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
> {
> struct dma_buf_sysfs_entry *sysfs_entry;
> int ret;
> + static atomic64_t unique_id = ATOMIC_INIT(0);
>
> if (!dmabuf || !dmabuf->file)
> return -EINVAL;
> @@ -192,7 +193,8 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>
> /* create the directory for buffer stats */
> ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_ktype, NULL,
> - "%lu", file_inode(dmabuf->file)->i_ino);
> + "%lu-%lu", file_inode(dmabuf->file)->i_ino,
> + atomic64_add_return(1, &unique_id));
> if (ret)
> goto err_sysfs_dmabuf;
>
> --
> 2.7.4
>

\
 
 \ /
  Last update: 2022-05-10 19:12    [W:0.124 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site