Re: [PATCH] dma-buf: heaps: Set allocation limit for system heap

2021-08-03 Thread Hridya Valsaraju
On Mon, Aug 2, 2021 at 7:18 PM John Stultz  wrote:
>
> On Thu, Jul 22, 2021 at 12:07 PM Hridya Valsaraju  wrote:
> > This patch limits the size of total memory that can be requested in a
> > single allocation from the system heap. This would prevent a
> > buggy/malicious client from depleting system memory by requesting for an
> > extremely large allocation which might destabilize the system.
> >
> > The limit is set to half the size of the device's total RAM which is the
> > same as what was set by the deprecated ION system heap.
> >
> > Signed-off-by: Hridya Valsaraju 
>
> Seems sane to me, unless folks have better suggestions for allocation limits.
>
> Reviewed-by: John Stultz 

Thank you for taking a look John!

Regards,
Hridya

>
> thanks
> -john


Re: [PATCH 3/3] dma-buf: nuke SW_SYNC debugfs files

2021-07-30 Thread Hridya Valsaraju
On Thu, Jul 29, 2021 at 9:52 PM John Stultz  wrote:
>
> On Thu, Jul 29, 2021 at 12:24 AM Daniel Vetter  wrote:
> >
> > On Thu, Jul 29, 2021 at 09:03:30AM +0200, Christian König wrote:
> > > As we now knew controlling dma_fence synchronization from userspace is
> > > extremely dangerous and can not only deadlock drivers but trivially also 
> > > the
> > > whole kernel memory management.
> > >
> > > Entirely remove this option. We now have in kernel unit tests to exercise 
> > > the
> > > dma_fence framework and it's containers.
> > >
> > > Signed-off-by: Christian König 
> >
> > There's also igts for this, and Android heavily uses this. So I'm not sure
> > we can just nuke it.
>
> Eeeeh... I don't think that's actually the case anymore. As of
> android12-5.10 CONFIG_SW_SYNC is not turned on.
> Further, Android is disabling debugfs in their kernels as it exposes
> too much to userland.
>
> That said, there still are some references to it:
>   
> https://cs.android.com/android/platform/superproject/+/master:system/core/libsync/sync.c;l=416

Hello,

Like John mentioned, CONFIG_SW_SYNC is not turned on for the *-5.4 and
newer branches in the Android Common Kernel. The references in AOSP
are only in place to support devices with older kernels upgrading to
newer Android versions.

Regards,
Hridya

>
> But it looks like the actual users are only kselftest and igt?
>
> Adding Alistair, Hridya and Sandeep in case they have more context.
>
> thanks
> -john


[PATCH] dma-buf: heaps: Set allocation limit for system heap

2021-07-22 Thread Hridya Valsaraju
This patch limits the size of total memory that can be requested in a
single allocation from the system heap. This would prevent a
buggy/malicious client from depleting system memory by requesting for an
extremely large allocation which might destabilize the system.

The limit is set to half the size of the device's total RAM which is the
same as what was set by the deprecated ION system heap.

Signed-off-by: Hridya Valsaraju 
---
 drivers/dma-buf/heaps/system_heap.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma-buf/heaps/system_heap.c 
b/drivers/dma-buf/heaps/system_heap.c
index b7fbce66bcc0..099f5a8304b4 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -371,6 +371,12 @@ static struct dma_buf *system_heap_do_allocate(struct 
dma_heap *heap,
struct page *page, *tmp_page;
int i, ret = -ENOMEM;
 
+   if (len / PAGE_SIZE > totalram_pages() / 2) {
+   pr_err("pid %d requested too large an allocation(size %lu) from 
system heap\n",
+  current->pid, len);
+   return ERR_PTR(ret);
+   }
+
buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
if (!buffer)
return ERR_PTR(-ENOMEM);
-- 
2.32.0.432.gabb21c7263-goog



[PATCH] dma-buf: Delete the DMA-BUF attachment sysfs statistics

2021-07-10 Thread Hridya Valsaraju
The DMA-BUF attachment statistics form a subset of the DMA-BUF
sysfs statistics that recently merged to the drm-misc tree.
Since there has been a reported a performance regression due to the
overhead of sysfs directory creation/teardown during
dma_buf_attach()/dma_buf_detach(), this patch deletes the DMA-BUF
attachment statistics from sysfs.

Fixes: bdb8d06dfefd (dmabuf: Add the capability to expose DMA-BUF stats
in sysfs)
Signed-off-by: Hridya Valsaraju 
---

Hello all,

One of our partners recently reported a perf regression in a driver
which was being caused due to the overhead of setup/teardown of the
sysfs attachment statistics in the dma_buf_attach()/dma_buf_detach()
invocations. Since the driver's latency requirements were of the order
of microseconds(~100us), the overhead was significant.
Since this indicates that the solution might not work well for
all DMA-BUF importers, I think the right thing to do might be to delete
the same before it reaches upstream and becomes ABI :( I apologize for
the inconvenience.

This patch is based on the drm-misc-next branch. Please feel free to
let me know if you would prefer that I send a full revert and new patch that
adds the rest of the statistics.

Regards,
Hridya

 .../ABI/testing/sysfs-kernel-dmabuf-buffers   |  28 
 drivers/dma-buf/dma-buf-sysfs-stats.c | 140 +-
 drivers/dma-buf/dma-buf-sysfs-stats.h |  27 
 drivers/dma-buf/dma-buf.c |  16 --
 include/linux/dma-buf.h   |  17 ---
 5 files changed, 4 insertions(+), 224 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers 
b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
index a243984ed420..5d3bc997dc64 100644
--- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
+++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
@@ -22,31 +22,3 @@ KernelVersion:   v5.13
 Contact:   Hridya Valsaraju 
 Description:   This file is read-only and specifies the size of the DMA-BUF in
bytes.
-
-What:  /sys/kernel/dmabuf/buffers//attachments
-Date:  May 2021
-KernelVersion: v5.13
-Contact:   Hridya Valsaraju 
-Description:   This directory will contain subdirectories representing every
-   attachment of the DMA-BUF.
-
-What:  
/sys/kernel/dmabuf/buffers//attachments/
-Date:  May 2021
-KernelVersion: v5.13
-Contact:   Hridya Valsaraju 
-Description:   This directory will contain information on the attached device
-   and the number of current distinct device mappings.
-
-What:  
/sys/kernel/dmabuf/buffers//attachments//device
-Date:  May 2021
-KernelVersion: v5.13
-Contact:   Hridya Valsaraju 
-Description:   This file is read-only and is a symlink to the attached device's
-   sysfs entry.
-
-What:  
/sys/kernel/dmabuf/buffers//attachments//map_counter
-Date:  May 2021
-KernelVersion: v5.13
-Contact:   Hridya Valsaraju 
-Description:   This file is read-only and contains a map_counter indicating the
-   number of distinct device mappings of the attachment.
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c 
b/drivers/dma-buf/dma-buf-sysfs-stats.c
index a2638e84199c..053baadcada9 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -40,14 +40,11 @@
  *
  * * ``/sys/kernel/dmabuf/buffers//exporter_name``
  * * ``/sys/kernel/dmabuf/buffers//size``
- * * 
``/sys/kernel/dmabuf/buffers//attachments//device``
- * * 
``/sys/kernel/dmabuf/buffers//attachments//map_counter``
  *
- * The information in the interface can also be used to derive per-exporter and
- * per-device usage statistics. The data from the interface can be gathered
- * on error conditions or other important events to provide a snapshot of
- * DMA-BUF usage. It can also be collected periodically by telemetry to monitor
- * various metrics.
+ * 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
+ * or other important events to provide a snapshot of DMA-BUF usage.
+ * It can also be collected periodically by telemetry to monitor various 
metrics.
  *
  * Detailed documentation about the interface is present in
  * Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers.
@@ -121,120 +118,6 @@ static struct kobj_type dma_buf_ktype = {
.default_groups = dma_buf_stats_default_groups,
 };
 
-#define to_dma_buf_attach_entry_from_kobj(x) container_of(x, struct 
dma_buf_attach_sysfs_entry, kobj)
-
-struct dma_buf_attach_stats_attribute {
-   struct attribute attr;
-   ssize_t (*show)(struct dma_buf_attach_sysfs_entry *sysfs_entry,
-   struct dma_buf_attach_stats_attribute *attr, char *buf);
-};
-#define to_dma_buf_attach_stats_attr(x) container_of(x, struct 
dma_buf_attach_stats_attribute, attr)
-
-static ssize_t

Re: [PATCH v6] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-06-15 Thread Hridya Valsaraju
On Tue, Jun 15, 2021 at 2:49 AM Daniel Vetter  wrote:
>
> On Thu, Jun 03, 2021 at 02:47:51PM -0700, Hridya Valsaraju wrote:
> > Overview
> > 
> > The patch adds DMA-BUF statistics to /sys/kernel/dmabuf/buffers. It
> > 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/buffers//exporter_name
> > /sys/kernel/dmabuf/buffers//size
> > /sys/kernel/dmabuf/buffers//attachments//device
> > /sys/kernel/dmabuf/buffers//attachments//map_counter
> >
> > 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.
> >
> > Use Cases
> > =
> > The interface provides a way to gather DMA-BUF per-buffer statistics
> > from production devices. These statistics will be used to derive DMA-BUF
> > per-exporter stats and per-device usage stats for Android Bug reports.
> > The corresponding userspace changes can be found at [2].
> > Telemetry tools will also capture this information(along with other
> > memory metrics) periodically as well as on important events like a
> > foreground app kill (which might have been triggered by Low Memory
> > Killer). It will also contribute to provide a snapshot of the system
> > memory usage on other events such as OOM kills and Application Not
> > Responding events.
> >
> > Background
> > ==
> > Currently, there are two existing interfaces that provide information
> > about DMA-BUFs.
> > 1) /sys/kernel/debug/dma_buf/bufinfo
> > debugfs is however unsuitable to be mounted in production systems and
> > cannot be considered as an alternative to the sysfs interface being
> > proposed.
> > 2) proc//fdinfo/
> > The proc//fdinfo/ files expose information about DMA-BUF fds.
> > However, the existing procfs interfaces can only provide information
> > about the buffers for which processes hold fds or have the buffers
> > mmapped into their address space. Since the procfs interfaces alone
> > cannot provide a full picture of all DMA-BUFs in the system, there is
> > the need for an alternate interface to provide this information on
> > production systems.
> >
> > The patch contains the following major improvements over v1:
> > 1) Each attachment is represented by its own directory to allow creating
> > a symlink to the importing device and to also provide room for future
> > expansion.
> > 2) The number of distinct mappings of each attachment is exposed in a
> > separate file.
> > 3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
> > inorder to make the interface expandable in future.
> >
> > All of the improvements above are based on suggestions/feedback from
> > Daniel Vetter and Christian König.
> >
> > A shell script that can be run on a classic Linux environment to read
> > out the DMA-BUF statistics can be found at [3](suggested by John
> > Stultz).
> >
> > [1]: https://lore.kernel.org/patchwork/patch/1088791/
> > [2]: 
> > https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
> > [3]: 
> > https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734
> >
> > Reviewed-by: Greg Kroah-Hartman 
> > Signed-off-by: Hridya Valsaraju 
> > Reported-by: kernel test robot 
> > ---
> >
> > Hello Daniel,
> >
> > I have added the documentation as a DOC: overview section in the
> > dma-buf-sysfs-stats.c file as per your suggestion. Please do take a look
> > when you get a chance. Thanks in advance!
> >
> > Regards,
> > Hridya
> >
> > Change in v6:
> > -Moved documentation content from Documentation/driver-api/dma-buf.rst
> > to drivers/dma-buf/dma-buf-sysfs-stats.c as a DOC section and linked to
> > it from Documentation/driver-api/dma-buf.rst. Based on feedback from
> > Daniel Vetter.
> >
> > Change in v5:
> > -Added a section on DMA-BUF statistics to
> > Documentation/driver-api/dma-buf.rst. Organized the commit message to
> > clearly state the need for the new interface and provide the
> > background on why the existing means of DMA-BUF accounting will not
> > suffice. Based on feedback from Daniel Vetter.
> >
> > Changes in v4:
> > -Suppress uevents from kset creation to avoid waking up uevent listeners
> > on DMA-BUF export/release.
> >
> > Changes 

Re: [Linaro-mm-sig] [PATCH v3] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-02-04 Thread Hridya Valsaraju
On Wed, Feb 3, 2021 at 2:25 AM Daniel Vetter  wrote:
>
> On Mon, Feb 01, 2021 at 01:02:30PM -0800, Hridya Valsaraju wrote:
> > On Mon, Feb 1, 2021 at 10:37 AM Daniel Vetter  wrote:
> > >
> > > On Thu, Jan 28, 2021 at 1:03 PM Sumit Semwal  
> > > wrote:
> > > >
> > > > On Thu, 28 Jan 2021 at 17:23, Christian König
> > > >  wrote:
> > > > >
> > > > > Am 28.01.21 um 12:00 schrieb Sumit Semwal:
> > > > > > Hi Hridya,
> > > > > >
> > > > > > On Wed, 27 Jan 2021 at 17:36, Greg KH  
> > > > > > wrote:
> > > > > >> On Tue, Jan 26, 2021 at 12:42:36PM -0800, Hridya Valsaraju wrote:
> > > > > >>> 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/buffers//exporter_name
> > > > > >>> /sys/kernel/dmabuf/buffers//size
> > > > > >>> /sys/kernel/dmabuf/buffers//attachments//device
> > > > > >>> /sys/kernel/dmabuf/buffers//attachments//map_counter
> > > > > >>>
> > > > > >>> 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.
> > > > > >>>
> > > > > >>> This information will be used to derive DMA-BUF
> > > > > >>> per-exporter stats and per-device usage stats for Android Bug 
> > > > > >>> reports.
> > > > > >>> The corresponding userspace changes can be found at [2].
> > > > > >>> Telemetry tools will also capture this information(along with 
> > > > > >>> other
> > > > > >>> memory metrics) periodically as well as on important events like a
> > > > > >>> foreground app kill (which might have been triggered by Low Memory
> > > > > >>> Killer). It will also contribute to provide a snapshot of the 
> > > > > >>> system
> > > > > >>> memory usage on other events such as OOM kills and Application Not
> > > > > >>> Responding events.
> > > > > >>>
> > > > > >>> A shell script that can be run on a classic Linux environment to 
> > > > > >>> read
> > > > > >>> out the DMA-BUF statistics can be found at [3](suggested by John
> > > > > >>> Stultz).
> > > > > >>>
> > > > > >>> The patch contains the following improvements over the previous 
> > > > > >>> version:
> > > > > >>> 1) Each attachment is represented by its own directory to allow 
> > > > > >>> creating
> > > > > >>> a symlink to the importing device and to also provide room for 
> > > > > >>> future
> > > > > >>> expansion.
> > > > > >>> 2) The number of distinct mappings of each attachment is exposed 
> > > > > >>> in a
> > > > > >>> separate file.
> > > > > >>> 3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
> > > > > >>> inorder to make the interface expandable in future.
> > > > > >>>
> > > > > >>> All of the improvements above are based on suggestions/feedback 
> > > > > >>> from
> > > > > >>> Daniel Vetter and Christian König.
> > > > > >>>
> > > > > >>> [1]: https://lore.kernel.org/patchwork/patch/1088791/
> > > > > >>> [2]: 
> &g

Re: [Linaro-mm-sig] [PATCH v3] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-02-02 Thread Hridya Valsaraju
On Mon, Feb 1, 2021 at 10:37 AM Daniel Vetter  wrote:
>
> On Thu, Jan 28, 2021 at 1:03 PM Sumit Semwal  wrote:
> >
> > On Thu, 28 Jan 2021 at 17:23, Christian König
> >  wrote:
> > >
> > > Am 28.01.21 um 12:00 schrieb Sumit Semwal:
> > > > Hi Hridya,
> > > >
> > > > On Wed, 27 Jan 2021 at 17:36, Greg KH  
> > > > wrote:
> > > >> On Tue, Jan 26, 2021 at 12:42:36PM -0800, Hridya Valsaraju wrote:
> > > >>> 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/buffers//exporter_name
> > > >>> /sys/kernel/dmabuf/buffers//size
> > > >>> /sys/kernel/dmabuf/buffers//attachments//device
> > > >>> /sys/kernel/dmabuf/buffers//attachments//map_counter
> > > >>>
> > > >>> 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.
> > > >>>
> > > >>> This information will be used to derive DMA-BUF
> > > >>> per-exporter stats and per-device usage stats for Android Bug reports.
> > > >>> The corresponding userspace changes can be found at [2].
> > > >>> Telemetry tools will also capture this information(along with other
> > > >>> memory metrics) periodically as well as on important events like a
> > > >>> foreground app kill (which might have been triggered by Low Memory
> > > >>> Killer). It will also contribute to provide a snapshot of the system
> > > >>> memory usage on other events such as OOM kills and Application Not
> > > >>> Responding events.
> > > >>>
> > > >>> A shell script that can be run on a classic Linux environment to read
> > > >>> out the DMA-BUF statistics can be found at [3](suggested by John
> > > >>> Stultz).
> > > >>>
> > > >>> The patch contains the following improvements over the previous 
> > > >>> version:
> > > >>> 1) Each attachment is represented by its own directory to allow 
> > > >>> creating
> > > >>> a symlink to the importing device and to also provide room for future
> > > >>> expansion.
> > > >>> 2) The number of distinct mappings of each attachment is exposed in a
> > > >>> separate file.
> > > >>> 3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
> > > >>> inorder to make the interface expandable in future.
> > > >>>
> > > >>> All of the improvements above are based on suggestions/feedback from
> > > >>> Daniel Vetter and Christian König.
> > > >>>
> > > >>> [1]: https://lore.kernel.org/patchwork/patch/1088791/
> > > >>> [2]: 
> > > >>> https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
> > > >>> [3]: 
> > > >>> https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734
> > > >>>
> > > >>> Signed-off-by: Hridya Valsaraju 
> > > >>> Reported-by: kernel test robot 
> > > > Thanks for the patch!
> > > >
> > > > Christian: If you're satisfied with the explanation around not
> > > > directly embedding kobjects into the dma_buf and dma_buf_attachment
> > > > structs, then with Greg's r-b from sysfs PoV, I think we can merge it.
> > > > Please let me know if you feel otherwise!
> > >
> > >  From the technical side it looks clean to me, feel free to add my
> > > acked-by while pushing.
> > >
> > > But I would at least try to convince Daniel on the design. At least some
> > > of his concerns seems to be valid and keep in mind that we nee

Re: [Linaro-mm-sig] [PATCH v3] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-02-02 Thread Hridya Valsaraju
On Thu, Jan 28, 2021 at 6:35 AM Sumit Semwal  wrote:
>
> Hi Simon,
>
> On Thu, 28 Jan 2021 at 20:01, Simon Ser  wrote:
> >
> > On Thursday, January 28th, 2021 at 1:03 PM, Sumit Semwal 
> >  wrote:
> >
> > > Since he didn't comment over Hridya's last clarification about the
> > > tracepoints to track total GPU memory allocations being orthogonal to
> > > this series, I assumed he agreed with it.
> >
> > IIRC he's away this week. (I don't remember when he comes back.)
> >
> > > Daniel, do you still have objections around adding this patch in?
> >
> > (Adding him explicitly in CC)
> Thanks for doing this!
>
> Best,
> Sumit.

Thank you all for the help :)

Regards,
Hridya
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-01-27 Thread 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/buffers//exporter_name
/sys/kernel/dmabuf/buffers//size
/sys/kernel/dmabuf/buffers//attachments//device
/sys/kernel/dmabuf/buffers//attachments//map_counter

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.

This information will be used to derive DMA-BUF
per-exporter stats and per-device usage stats for Android Bug reports.
The corresponding userspace changes can be found at [2].
Telemetry tools will also capture this information(along with other
memory metrics) periodically as well as on important events like a
foreground app kill (which might have been triggered by Low Memory
Killer). It will also contribute to provide a snapshot of the system
memory usage on other events such as OOM kills and Application Not
Responding events.

A shell script that can be run on a classic Linux environment to read
out the DMA-BUF statistics can be found at [3](suggested by John
Stultz).

The patch contains the following improvements over the previous version:
1) Each attachment is represented by its own directory to allow creating
a symlink to the importing device and to also provide room for future
expansion.
2) The number of distinct mappings of each attachment is exposed in a
separate file.
3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
inorder to make the interface expandable in future.

All of the improvements above are based on suggestions/feedback from
Daniel Vetter and Christian König.

[1]: https://lore.kernel.org/patchwork/patch/1088791/
[2]: 
https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
[3]: 
https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734

Signed-off-by: Hridya Valsaraju 
Reported-by: kernel test robot 
---
Changes in v3:
Fix a warning reported by the kernel test robot.

Changes in v2:
-Move statistics to /sys/kernel/dmabuf/buffers in oder to allow addition
of other DMA-BUF-related sysfs stats in future. Based on feedback from
Daniel Vetter.
-Each attachment has its own directory to represent attaching devices as
symlinks and to introduce map_count as a separate file. Based on
feedback from Daniel Vetter and Christian König. Thank you both!
-Commit messages updated to point to userspace code in AOSP that will
read the DMA-BUF sysfs stats.


 .../ABI/testing/sysfs-kernel-dmabuf-buffers   |  52 
 drivers/dma-buf/Kconfig   |  11 +
 drivers/dma-buf/Makefile  |   1 +
 drivers/dma-buf/dma-buf-sysfs-stats.c | 285 ++
 drivers/dma-buf/dma-buf-sysfs-stats.h |  62 
 drivers/dma-buf/dma-buf.c |  37 +++
 include/linux/dma-buf.h   |  20 ++
 7 files changed, 468 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
 create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.c
 create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.h

diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers 
b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
new file mode 100644
index ..6f7c65209f07
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
@@ -0,0 +1,52 @@
+What:  /sys/kernel/dmabuf/buffers
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   The /sys/kernel/dmabuf/buffers directory contains a
+   snapshot of the internal state of every DMA-BUF.
+   /sys/kernel/dmabuf/buffers/ will contain the
+   statistics for the DMA-BUF with the unique inode number
+   
+Users: kernel memory tuning/debugging tools
+
+What:  /sys/kernel/dmabuf/buffers//exporter_name
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This file is read-only and contains the name of the exporter of
+   the DMA-BUF.
+
+What:  /sys/kernel/dmabuf/buffers//size
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This file is read-only and specifies the size of the DMA-BUF in
+   bytes.
+
+What:  /sys/kernel/dmabuf/buffers//attachments
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This directory will contain subdirectories representing every
+   attachment of the DMA-BUF.
+
+What:  
/sys/kernel/dmabuf/buffers//attachments/
+Date:  January 2021

Re: [Linaro-mm-sig] [PATCH v2] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-01-21 Thread Hridya Valsaraju
On Wed, Jan 20, 2021 at 4:22 AM Christian König
 wrote:
>
> Am 19.01.21 um 23:57 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/buffers//exporter_name
> > /sys/kernel/dmabuf/buffers//size
> > /sys/kernel/dmabuf/buffers//attachments//device
> > /sys/kernel/dmabuf/buffers//attachments//map_counter
> >
> > 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.
> >
> > This information will be used to derive DMA-BUF
> > per-exporter stats and per-device usage stats for Android Bug reports.
> > The corresponding userspace changes can be found at [2].
> > Telemetry tools will also capture this information(along with other
> > memory metrics) periodically as well as on important events like a
> > foreground app kill (which might have been triggered by Low Memory
> > Killer). It will also contribute to provide a snapshot of the system
> > memory usage on other events such as OOM kills and Application Not
> > Responding events.
> >
> > A shell script that can be run on a classic Linux environment to read
> > out the DMA-BUF statistics can be found at [3](suggested by John
> > Stultz).
> >
> > The patch contains the following improvements over the previous version:
> > 1) Each attachment is represented by its own directory to allow creating
> > a symlink to the importing device and to also provide room for future
> > expansion.
> > 2) The number of distinct mappings of each attachment is exposed in a
> > separate file.
> > 3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
> > inorder to make the interface expandable in future.
> >
> > All of the improvements above are based on suggestions/feedback from
> > Daniel Vetter and Christian König.
> >
> > [1]: https://lore.kernel.org/patchwork/patch/1088791/
> > [2]: 
> > https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
> > [3]: 
> > https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734
> >
> > Signed-off-by: Hridya Valsaraju 
> > ---
> > Changes in v2:
> > -Move statistics to /sys/kernel/dmabuf/buffers in oder to allow addition
> > of other DMA-BUF-related sysfs stats in future. Based on feedback from
> > Daniel Vetter.
> > -Each attachment has its own directory to represent attaching devices as
> > symlinks and to introduce map_count as a separate file. Based on
> > feedback from Daniel Vetter and Christian König. Thank you both!
> > -Commit messages updated to point to userspace code in AOSP that will
> > read the DMA-BUF sysfs stats.
> >
> >   .../ABI/testing/sysfs-kernel-dmabuf-buffers   |  52 
> >   drivers/dma-buf/Kconfig   |  11 +
> >   drivers/dma-buf/Makefile  |   1 +
> >   drivers/dma-buf/dma-buf-sysfs-stats.c | 283 ++
> >   drivers/dma-buf/dma-buf-sysfs-stats.h |  62 
> >   drivers/dma-buf/dma-buf.c |  37 +++
> >   include/linux/dma-buf.h   |  20 ++
> >   7 files changed, 466 insertions(+)
> >   create mode 100644 Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> >   create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.c
> >   create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers 
> > b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> > new file mode 100644
> > index ..6f7c65209f07
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> > @@ -0,0 +1,52 @@
> > +What:/sys/kernel/dmabuf/buffers
> > +Date:January 2021
> > +KernelVersion:   v5.12
> > +Contact: Hridya Valsaraju 
> > +Description: The /sys/kernel/dmabuf/buffers directory contains a
> > + snapshot of the internal state of every DMA-BUF.
> > + /sys/kernel/dmabuf/buffers/ will contain the
> > + statistics for the DMA-BUF with the unique inode number

Re: [Linaro-mm-sig] [PATCH v2] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-01-21 Thread Hridya Valsaraju
On Wed, Jan 20, 2021 at 4:42 AM Daniel Vetter  wrote:
>
> On Wed, Jan 20, 2021 at 1:22 PM Christian König
>  wrote:
> >
> > Am 19.01.21 um 23:57 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/buffers//exporter_name
> > > /sys/kernel/dmabuf/buffers//size
> > > /sys/kernel/dmabuf/buffers//attachments//device
> > > /sys/kernel/dmabuf/buffers//attachments//map_counter
> > >
> > > 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.
> > >
> > > This information will be used to derive DMA-BUF
> > > per-exporter stats and per-device usage stats for Android Bug reports.
> > > The corresponding userspace changes can be found at [2].
> > > Telemetry tools will also capture this information(along with other
> > > memory metrics) periodically as well as on important events like a
> > > foreground app kill (which might have been triggered by Low Memory
> > > Killer). It will also contribute to provide a snapshot of the system
> > > memory usage on other events such as OOM kills and Application Not
> > > Responding events.
> > >
> > > A shell script that can be run on a classic Linux environment to read
> > > out the DMA-BUF statistics can be found at [3](suggested by John
> > > Stultz).
> > >
> > > The patch contains the following improvements over the previous version:
> > > 1) Each attachment is represented by its own directory to allow creating
> > > a symlink to the importing device and to also provide room for future
> > > expansion.
> > > 2) The number of distinct mappings of each attachment is exposed in a
> > > separate file.
> > > 3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
> > > inorder to make the interface expandable in future.
> > >
> > > All of the improvements above are based on suggestions/feedback from
> > > Daniel Vetter and Christian König.
> > >
> > > [1]: https://lore.kernel.org/patchwork/patch/1088791/
> > > [2]: 
> > > https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
> > > [3]: 
> > > https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734
> > >
> > > Signed-off-by: Hridya Valsaraju 
> > > ---
> > > Changes in v2:
> > > -Move statistics to /sys/kernel/dmabuf/buffers in oder to allow addition
> > > of other DMA-BUF-related sysfs stats in future. Based on feedback from
> > > Daniel Vetter.
> > > -Each attachment has its own directory to represent attaching devices as
> > > symlinks and to introduce map_count as a separate file. Based on
> > > feedback from Daniel Vetter and Christian König. Thank you both!
> > > -Commit messages updated to point to userspace code in AOSP that will
> > > read the DMA-BUF sysfs stats.
> > >
> > >   .../ABI/testing/sysfs-kernel-dmabuf-buffers   |  52 
> > >   drivers/dma-buf/Kconfig   |  11 +
> > >   drivers/dma-buf/Makefile  |   1 +
> > >   drivers/dma-buf/dma-buf-sysfs-stats.c | 283 ++
> > >   drivers/dma-buf/dma-buf-sysfs-stats.h |  62 
> > >   drivers/dma-buf/dma-buf.c |  37 +++
> > >   include/linux/dma-buf.h   |  20 ++
> > >   7 files changed, 466 insertions(+)
> > >   create mode 100644 Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> > >   create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.c
> > >   create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.h
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers 
> > > b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> > > new file mode 100644
> > > index ..6f7c65209f07
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
> > > @@ -0,0 +1

[PATCH v2] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-01-21 Thread 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/buffers//exporter_name
/sys/kernel/dmabuf/buffers//size
/sys/kernel/dmabuf/buffers//attachments//device
/sys/kernel/dmabuf/buffers//attachments//map_counter

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.

This information will be used to derive DMA-BUF
per-exporter stats and per-device usage stats for Android Bug reports.
The corresponding userspace changes can be found at [2].
Telemetry tools will also capture this information(along with other
memory metrics) periodically as well as on important events like a
foreground app kill (which might have been triggered by Low Memory
Killer). It will also contribute to provide a snapshot of the system
memory usage on other events such as OOM kills and Application Not
Responding events.

A shell script that can be run on a classic Linux environment to read
out the DMA-BUF statistics can be found at [3](suggested by John
Stultz).

The patch contains the following improvements over the previous version:
1) Each attachment is represented by its own directory to allow creating
a symlink to the importing device and to also provide room for future
expansion.
2) The number of distinct mappings of each attachment is exposed in a
separate file.
3) The per-buffer statistics are now in /sys/kernel/dmabuf/buffers
inorder to make the interface expandable in future.

All of the improvements above are based on suggestions/feedback from
Daniel Vetter and Christian König.

[1]: https://lore.kernel.org/patchwork/patch/1088791/
[2]: 
https://android-review.googlesource.com/q/topic:%22dmabuf-sysfs%22+(status:open%20OR%20status:merged)
[3]: 
https://android-review.googlesource.com/c/platform/system/memory/libmeminfo/+/1549734

Signed-off-by: Hridya Valsaraju 
---
Changes in v2:
-Move statistics to /sys/kernel/dmabuf/buffers in oder to allow addition
of other DMA-BUF-related sysfs stats in future. Based on feedback from
Daniel Vetter.
-Each attachment has its own directory to represent attaching devices as
symlinks and to introduce map_count as a separate file. Based on
feedback from Daniel Vetter and Christian König. Thank you both!
-Commit messages updated to point to userspace code in AOSP that will
read the DMA-BUF sysfs stats.

 .../ABI/testing/sysfs-kernel-dmabuf-buffers   |  52 
 drivers/dma-buf/Kconfig   |  11 +
 drivers/dma-buf/Makefile  |   1 +
 drivers/dma-buf/dma-buf-sysfs-stats.c | 283 ++
 drivers/dma-buf/dma-buf-sysfs-stats.h |  62 
 drivers/dma-buf/dma-buf.c |  37 +++
 include/linux/dma-buf.h   |  20 ++
 7 files changed, 466 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
 create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.c
 create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.h

diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers 
b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
new file mode 100644
index ..6f7c65209f07
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
@@ -0,0 +1,52 @@
+What:  /sys/kernel/dmabuf/buffers
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   The /sys/kernel/dmabuf/buffers directory contains a
+   snapshot of the internal state of every DMA-BUF.
+   /sys/kernel/dmabuf/buffers/ will contain the
+   statistics for the DMA-BUF with the unique inode number
+   
+Users: kernel memory tuning/debugging tools
+
+What:  /sys/kernel/dmabuf/buffers//exporter_name
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This file is read-only and contains the name of the exporter of
+   the DMA-BUF.
+
+What:  /sys/kernel/dmabuf/buffers//size
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This file is read-only and specifies the size of the DMA-BUF in
+   bytes.
+
+What:  /sys/kernel/dmabuf/buffers//attachments
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This directory will contain subdirectories representing every
+   attachment of the DMA-BUF.
+
+What:  
/sys/kernel/dmabuf/buffers//attachments/
+Date:  January 2021
+KernelVersion: v5.12
+Contact:   Hridya Valsaraju 
+Description:   This directory will contain

Re: [PATCH] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2020-12-14 Thread Hridya Valsaraju
Thank you Christian!

On Fri, Dec 11, 2020 at 12:03 AM Christian König
 wrote:
>
> Am 10.12.20 um 23:41 schrieb Hridya Valsaraju:
> > Thanks again for the reviews!
> >
> > On Thu, Dec 10, 2020 at 3:03 AM Christian König
> >  wrote:
> >> Am 10.12.20 um 11:56 schrieb Greg KH:
> >>> On Thu, Dec 10, 2020 at 11:27:27AM +0100, Daniel Vetter wrote:
> >>>> 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//exporter_name
> >>>>>>> /sys/kernel/dmabuf//size
> >>>>>>> /sys/kernel/dmabuf//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.
> >>> I think the "issue" is that this was a feature from ion that people
> >>> "missed" in the dmabuf move.  Taking away the ability to see what kind
> >>> of allocations were being made didn't make a lot of debugging tools
> >>> happy :(
> >> Yeah, that is certainly a very valid concern.
> >>
> >>> But Hridya knows more, she's been dealing with the transition for a long
> >>> time now.
> > Currently, telemetry tools capture this information(along with other
> > memory metrics) periodically as well as on important events like a
> > foreground app kill (which might have been triggered by an LMK). We
> > would also like to get a snapshot of the system memory usage on other
> > events such as OOM kills and ANRs.
>
> That userspace tools are going to use those files directly is the
> justification you need for the stable UAPI provided by sysfs.
>
> Otherwise debugfs would be the way to go even when that is often disabled.
>
> Please change the commit message to reflect that.


Sounds good, I will make sure to include it in the commit message of
the next version.


>
> >>>> 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.
> >>> These aren't struct devices, so I don't understand the objection here.
> >>> Where else could these go in sysfs?
> >> Sure they are! Just take a look at an attachment:
> >>
> >> struct dma_buf_attachment {
> >>struct dma_buf *dmabuf;
> >>struct device *dev;
> >>
> >> This is the struct device which is importing the buffer and the patch in
> >> discussion is just printing the name of this device into sysfs.
> > I actually did not know that this is not ok to do. I will change it in
> > the next version of the patch to be sysfs links instead.
>
> Thanks, you need to restructure this patch a bit. But I agree with
> Daniel that links to the devices which are attached are more appropriate.
>
> I'm just not sure how we want to represent the map count for each
> attachment then, probably best to have that as separate file as well.


Yes, I can add the map count as a separate file. Thanks again!

Regards,
Hridya


>
> Regards,
> Christian.
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-team+unsubscr...@android.com.
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2020-12-11 Thread Hridya Valsaraju
Thank you for the reviews Greg, Christian and Daniel!


On Thu, Dec 10, 2020 at 1:59 AM 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//exporter_name
> > /sys/kernel/dmabuf//size
> > /sys/kernel/dmabuf//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?

Yes, we currently collect this information on production devices as well.

>
> >
> > This information is intended to help with root-causing
> > low-memory kills and the debugging/analysis of other memory-related issues.
> >
> > It will also be used to derive DMA-BUF
> > per-exporter stats and per-device usage stats for Android Bug reports.
> >
> > [1]: 
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1088791%2Fdata=04%7C01%7Cchristian.koenig%40amd.com%7C30a0e015502b4d20e18208d89cc63f1a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637431722574983797%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=RdGMvj5VsFUwJcVOuSPaLuAr4eI3CR1YOaznupmpTqg%3Dreserved=0
> >
> > Signed-off-by: Hridya Valsaraju 
> > ---
> >   Documentation/ABI/testing/sysfs-kernel-dmabuf |  32 
> >   drivers/dma-buf/Kconfig   |  11 ++
> >   drivers/dma-buf/Makefile  |   1 +
> >   drivers/dma-buf/dma-buf-sysfs-stats.c | 162 ++
> >   drivers/dma-buf/dma-buf-sysfs-stats.h |  37 
> >   drivers/dma-buf/dma-buf.c |  29 
> >   include/linux/dma-buf.h   |  13 ++
> >   7 files changed, 285 insertions(+)
> >   create mode 100644 Documentation/ABI/testing/sysfs-kernel-dmabuf
> >   create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.c
> >   create mode 100644 drivers/dma-buf/dma-buf-sysfs-stats.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf 
> > b/Documentation/ABI/testing/sysfs-kernel-dmabuf
> > new file mode 100644
> > index ..02d407d57aaa
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf
> > @@ -0,0 +1,32 @@
> > +What:/sys/kernel/dmabuf
> > +Date:November 2020
> > +KernelVersion:   v5.11
> > +Contact: Hridya Valsaraju 
> > +Description: The /sys/kernel/dmabuf directory contains a
> > + snapshot of the internal state of every DMA-BUF.
> > +     /sys/kernel/dmabuf/ will contain the
> > + statistics for the DMA-BUF with the unique inode number
> > + 
> > +Users:   kernel memory tuning/debugging tools
> > +
> > +What:/sys/kernel/dmabuf//exporter_name
> > +Date:November 2020
> > +KernelVersion:   v5.11
> > +Contact: Hridya Valsaraju 
> > +Description: This file is read-only and contains the name of the exporter 
> > of
> > + the DMA-BUF.
> > +
> > +What:/sys/kernel/dmabuf//size
> > +Dat: November 2020
> > +KernelVersion:   v5.11
> > +Contact: Hridya Valsaraju 
> > +Description: This file is read-only and specifies the size of the DMA-BUF 
> > in
> > + bytes.
> > +
> > +What:/sys/kernel/dmabuf//dev_map_info
> > +Dat: November 2020
> > +KernelVersion:   v5.11
> > +Contact: Hridya Valsaraju 
> > +Description: This file is read-only and lists the name of devices currently
> > + mapping the DMA-BUF in a space-separated format.
> > +
> > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
> > index 4f8224a6ac95..2fed26f14548 100644
> > --- a/drivers/dma-buf/Kconfig
> > +++ b/drivers/dma-buf/Kconfig
> > @@ -64,6 +64,17 @@ menuconfig DMABUF

Re: [PATCH] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2020-12-11 Thread Hridya Valsaraju
Thanks again for the reviews!

On Thu, Dec 10, 2020 at 3:03 AM Christian König
 wrote:
>
> Am 10.12.20 um 11:56 schrieb Greg KH:
> > On Thu, Dec 10, 2020 at 11:27:27AM +0100, Daniel Vetter wrote:
> >> 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//exporter_name
> >>>>> /sys/kernel/dmabuf//size
> >>>>> /sys/kernel/dmabuf//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.
> > I think the "issue" is that this was a feature from ion that people
> > "missed" in the dmabuf move.  Taking away the ability to see what kind
> > of allocations were being made didn't make a lot of debugging tools
> > happy :(
>
> Yeah, that is certainly a very valid concern.
>
> > But Hridya knows more, she's been dealing with the transition for a long
> > time now.

Currently, telemetry tools capture this information(along with other
memory metrics) periodically as well as on important events like a
foreground app kill (which might have been triggered by an LMK). We
would also like to get a snapshot of the system memory usage on other
events such as OOM kills and ANRs.

> >
> >> 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.
> > These aren't struct devices, so I don't understand the objection here.
> > Where else could these go in sysfs?
>
> Sure they are! Just take a look at an attachment:
>
> struct dma_buf_attachment {
>   struct dma_buf *dmabuf;
>   struct device *dev;
>
> This is the struct device which is importing the buffer and the patch in
> discussion is just printing the name of this device into sysfs.

I actually did not know that this is not ok to do. I will change it in
the next version of the patch to be sysfs links instead.

>
> >> 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.
> > Do we have the exporter device link in the dmabuf interface?  If so,
> > great, let's use that, but for some reason I didn't think it was there.
>
> Correct, since we don't really need a device as an exporter (it can just
> be a system heap as well) we only have a const char* as name for the
> exporter.

Yes, the file exporter_name prints out this information.

>
> >> 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.
> > There's a script somewhere that does this today, again, Hridya knows
> > more.

That is correct, we do have a tool in AOSP that gathers the
per-process DMA-BUF map stats from procfs.
https://android.googlesource.com/platform/system/memory/libmeminfo/+/refs/heads/master/libdmabufinfo/tools/dmabuf_dump.cpp

When I send the next revision of the patch, I will also include links
to AOSP CLs that show the usage for the sysfs files.

> >
> >> There's also 

Re: [PATCH] staging: ion: remove from the tree

2020-08-28 Thread Hridya Valsaraju
On Thu, Aug 27, 2020 at 10:17 AM Greg Kroah-Hartman
 wrote:
>
> On Thu, Aug 27, 2020 at 10:31:41PM +0530, Amit Pundir wrote:
> > On Thu, 27 Aug 2020 at 21:34, Greg Kroah-Hartman
> >  wrote:
> > >
> > > On Thu, Aug 27, 2020 at 09:31:27AM -0400, Laura Abbott wrote:
> > > > On 8/27/20 8:36 AM, Greg Kroah-Hartman wrote:
> > > > > The ION android code has long been marked to be removed, now that we
> > > > > dma-buf support merged into the real part of the kernel.
> > > > >
> > > > > It was thought that we could wait to remove the ion kernel at a later
> > > > > time, but as the out-of-tree Android fork of the ion code has diverged
> > > > > quite a bit, and any Android device using the ion interface uses that
> > > > > forked version and not this in-tree version, the in-tree copy of the
> > > > > code is abandonded and not used by anyone.
> > > > >
> > > > > Combine this abandoned codebase with the need to make changes to it in
> > > > > order to keep the kernel building properly, which then causes merge
> > > > > issues when merging those changes into the out-of-tree Android code, 
> > > > > and
> > > > > you end up with two different groups of people (the in-kernel-tree
> > > > > developers, and the Android kernel developers) who are both annoyed at
> > > > > the current situation.  Because of this problem, just drop the 
> > > > > in-kernel
> > > > > copy of the ion code now, as it's not used, and is only causing 
> > > > > problems
> > > > > for everyone involved.
> > > > >
> > > > > Cc: "Arve Hjønnevåg" 
> > > > > Cc: "Christian König" 
> > > > > Cc: Christian Brauner 
> > > > > Cc: Christoph Hellwig 
> > > > > Cc: Hridya Valsaraju 
> > > > > Cc: Joel Fernandes 
> > > > > Cc: John Stultz 
> > > > > Cc: Laura Abbott 
> > > > > Cc: Martijn Coenen 
> > > > > Cc: Shuah Khan 
> > > > > Cc: Sumit Semwal 
> > > > > Cc: Suren Baghdasaryan 
> > > > > Cc: Todd Kjos 
> > > > > Cc: de...@driverdev.osuosl.org
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Cc: linaro-mm-...@lists.linaro.org
> > > > > Signed-off-by: Greg Kroah-Hartman 
> > > >
> > > > We discussed this at the Android MC on Monday and the plan was to
> > > > remove it after the next LTS release.
> > >
> > > I know it was discussed, my point is that it is actually causing
> > > problems now (with developers who want to change the internal kernel api
> > > hitting issues, and newbies trying to clean up code in ways that isn't
> > > exactly optimal wasting maintainer cycles), and that anyone who uses
> > > this code, is not actually using this version of the code.  Everyone who
> > > relies on ion right now, is using the version that is in the Android
> > > common kernel tree, which has diverged from this in-kernel way quite a
> > > bit now for the reason that we didn't want to take any of those new
> > > features in the in-kernel version.
> > >
> > > So this is a problem that we have caused by just wanting to wait, no one
> > > is using this code, combined with it causing problems for the upstream
> > > developers.
> > >
> > > There is nothing "magic" about the last kernel of the year that requires
> > > this code to sit here until then.  At that point in time, all users
> > > will, again, be using the forked Android kernel version, and if we
> > > delete this now here, that fork can remain just fine, with the added
> > > benifit of it reducing developer workloads here in-kernel.
> > >
> > > So why wait?
> >
> > Hi,
> >
> > I don't know what is the right thing to do here. I just want to
> > highlight that AOSP's audio (codec2) HAL depends on the ION system
> > heap and it will break AOSP for people who boot mainline on their
> > devices, even for just testing purpose like we do in Linaro. Right now
> > we need only 1 (Android specific out-of-tree) patch to boot AOSP with
> > mainline and Sumit is already trying to upstream that vma naming
> > patch. Removal of in-kernel ION, will just add more to that delta.
>
> As AOSP will continue to rely on ION after December of this year, all
> you are doing is postponing the inevitable a few more months.
>
> Push back on the Android team to fix up the code to not use ION, they
> know this needs to happen.
>

Hi all,

We are currently working with the codec2 team to transition codec2 to
use libdmabufheap instead of libion. It will definitely happen during
the Android S timeframe.

Thanks,
Hridya

> thanks,
>
> greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel