All of lore.kernel.org
 help / color / mirror / Atom feed
From: Charan Teja Kalla <quic_charante@quicinc.com>
To: <gregkh@linuxfoundation.org>, <christian.koenig@amd.com>,
	<sumit.semwal@linaro.org>, <hridya@google.com>,
	<daniel.vetter@ffwll.ch>, <tjmercier@google.com>
Cc: linaro-mm-sig@lists.linaro.org,
	Charan Teja Kalla <quic_charante@quicinc.com>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-media@vger.kernel.org
Subject: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats
Date: Tue, 10 May 2022 19:36:02 +0530	[thread overview]
Message-ID: <1652191562-18700-1-git-send-email-quic_charante@quicinc.com> (raw)

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


WARNING: multiple messages have this Message-ID (diff)
From: Charan Teja Kalla <quic_charante@quicinc.com>
To: <gregkh@linuxfoundation.org>, <christian.koenig@amd.com>,
	<sumit.semwal@linaro.org>, <hridya@google.com>,
	<daniel.vetter@ffwll.ch>, <tjmercier@google.com>
Cc: <linux-kernel@vger.kernel.org>, <linux-media@vger.kernel.org>,
	<dri-devel@lists.freedesktop.org>,
	<linaro-mm-sig@lists.linaro.org>,
	"Charan Teja Kalla" <quic_charante@quicinc.com>
Subject: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats
Date: Tue, 10 May 2022 19:36:02 +0530	[thread overview]
Message-ID: <1652191562-18700-1-git-send-email-quic_charante@quicinc.com> (raw)

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


             reply	other threads:[~2022-05-10 14:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10 14:06 Charan Teja Kalla [this message]
2022-05-10 14:06 ` [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats Charan Teja Kalla
2022-05-10 15:12 ` Christian König
2022-05-10 15:12   ` Christian König
2022-05-10 17:14   ` Charan Teja Kalla
2022-05-10 17:14     ` Charan Teja Kalla
2022-05-10 17:22     ` Christian König
2022-05-10 17:22       ` Christian König
2022-05-11  6:49       ` Charan Teja Kalla
2022-05-11  6:49         ` Charan Teja Kalla
2022-05-11  7:03         ` Christian König
2022-05-11  7:03           ` Christian König
2022-05-12 14:50           ` Charan Teja Kalla
2022-05-12 14:50             ` Charan Teja Kalla
2022-05-10 17:11 ` T.J. Mercier
2022-05-10 17:11   ` T.J. Mercier
2022-05-10 21:55 ` kernel test robot
2022-05-10 21:55   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1652191562-18700-1-git-send-email-quic_charante@quicinc.com \
    --to=quic_charante@quicinc.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hridya@google.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tjmercier@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.