On Tue, May 10, 2022 at 7:07 AM Charan Teja Kalla <quic_charante@xxxxxxxxxxx> 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@xxxxxxxxxxx> > --- > 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@xxxxxxxxxxx/ > > 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@xxxxxxxxxx> > 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@xxxxxxxxxx> > 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@xxxxxxxxxx> > 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 >
