On Thu, 28 Jan 2021 at 17:23, Christian König <ckoenig.leichtzumerken@xxxxxxxxx> wrote: > > Am 28.01.21 um 12:00 schrieb Sumit Semwal: > > Hi Hridya, > > > > On Wed, 27 Jan 2021 at 17:36, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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/<inode_number>/exporter_name > >>> /sys/kernel/dmabuf/buffers/<inode_number>/size > >>> /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/device > >>> /sys/kernel/dmabuf/buffers/<inode_number>/attachments/<attach_uid>/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 <hridya@xxxxxxxxxx> > >>> Reported-by: kernel test robot <lkp@xxxxxxxxx> > > 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 need to > support this interface forever. Naturally. 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. Daniel, do you still have objections around adding this patch in? > > Regards, > Christian. Best, Sumit. > > > > >>> --- > >>> 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 > >> I don't know the dma-buf code at all, but from a sysfs/kobject point of > >> view, this patch looks good to me: > >> > >> Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Best, > > Sumit. > > _______________________________________________ > > Linaro-mm-sig mailing list > > Linaro-mm-sig@xxxxxxxxxxxxxxxx > > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig > -- Thanks and regards, Sumit Semwal Linaro Consumer Group - Tech Lead Linaro.org │ Open source software for ARM SoCs
