Re: 答复: 答复: [External Mail]Re: [PATCH v3] dma-buf: add some tracepoints to debug.

["Christian König" <christian.koenig@amd.com>]

On 11/27/25 16:35, 高翔 wrote:
>> Hui? Why would you add those symbols to any whitelist?
> 
> This is the mechanism of android gki. To use a tracepoint in the driver module, two points are required for a successful compilation:
> 1, export tracepint
> 2, add tracepoint to android whitelist.
> 
> But without exporting tracepoint, I can also use tracepoint_probe_register to solve it.

Yeah exporting the tracepoint is clearly not the correct approach.

See this is not used in a driver module, but the core kernel. Exporting the symbol would make only sense if drivers want to access it, but at least for upstream that is not the case.

So please just drop that.

Regards,
Christian.

> 
> 
>> The refcnt is manipulated outside of the DMA-buf code by the filesystem layer.
>> As far as I can see it is meaningless here.
> ok.
> 
> 
> 
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> *发件人:* Christian König <christian.koenig@amd.com>
> *发送时间:* 2025年11月27日 22:08:32
> *收件人:* 高翔; Xiang Gao; sumit.semwal@linaro.org; rostedt@goodmis.org; mhiramat@kernel.org
> *抄送:* linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; mathieu.desnoyers@efficios.com; dhowells@redhat.com; kuba@kernel.org; brauner@kernel.org; akpm@linux-foundation.org; linux-trace-kernel@vger.kernel.org
> *主题:* Re: 答复: [External Mail]Re: [PATCH v3] dma-buf: add some tracepoints to debug.
>  
> [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
> 
> On 11/27/25 15:05, 高翔 wrote:
>>> Exporting the tracepoints is unecessary as far as I can see
>>
>> android must export symbols before adding a whitelist.
> 
> Hui? Why would you add those symbols to any whitelist?
> 
>>
>>
>>> I can't come up with a reason why we should have the file refcount in the trace logs.
>>> Same for most other places.
>> I can directly observe how refcnt is changing.  Then it can be known where the dmabuf was finally released.
> 
> The refcnt is manipulated outside of the DMA-buf code by the filesystem layer.
> 
> As far as I can see it is meaningless here.
> 
> Regards,
> Christian.
> 
>>
>>
>>> Print the dev_name last, it's the external device which attaches to the DMA-buf.
>> ok.
>>
>>
>>> Additional to that it would be nice to know if the attachment is dynamic or not.
>> ok.
>>
>> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> *发件人:* Christian König <christian.koenig@amd.com>
>> *发送时间:* 2025年11月27日 17:49:09
>> *收件人:* Xiang Gao; sumit.semwal@linaro.org; rostedt@goodmis.org; mhiramat@kernel.org
>> *抄送:* linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; mathieu.desnoyers@efficios.com; dhowells@redhat.com; kuba@kernel.org; brauner@kernel.org; akpm@linux-foundation.org; linux-trace-kernel@vger.kernel.org; 高翔
>> *主题:* [External Mail]Re: [PATCH v3] dma-buf: add some tracepoints to debug.
>>
>> [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
>>
>> On 11/27/25 01:43, Xiang Gao wrote:
>>> From: gaoxiang17 <gaoxiang17@xiaomi.com>
>>>
>>> I want to track the status of dmabuf in real time in the production environment.
>>> But now we can only check it by traversing the fd in the process or dmabuf_list.
>>>
>>> For example:
>>>    binder:2962_2-2962    [005] ...1.   208.453940: dma_buf_export: exp_name=qcom,system name=(null) size=28672 ino=2580 f_refcnt=2
>>>    binder:2962_2-2962    [005] ...1.   208.453943: dma_buf_fd: exp_name=qcom,system name=(null) size=28672 ino=2580 fd=9 f_refcnt=2
>>>    binder:2962_2-2962    [005] ...1.   208.453977: dma_buf_mmap_internal: exp_name=qcom,system name=qcom,system size=28672 ino=2580 f_refcnt=4
>>>      kworker/5:2-194     [005] ...1.   208.460580: dma_buf_put: exp_name=qcom,system name=ab pid [8176] size=28672 ino=2580 f_refcnt=3
>>>     RenderThread-11305   [007] ...1.   208.599094: dma_buf_get: exp_name=qcom,system name=ab pid [8176] size=217088 ino=2579 fd=1114 f_refcnt=7
>>>     RenderThread-11305   [007] ...1.   208.599098: dma_buf_attach: dev_name=kgsl-3d0 exp_name=qcom,system name=ab pid [8176] size=217088 ino=2579 f_refcnt=7
>>>            <...>-14      [001] ...1.   208.726359: dma_buf_detach: dev_name=kgsl-3d0 exp_name=qcom,system name=ab pid [3317] size=217088 ino=2581 f_refcnt=3
>>>
>>> Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
>>> ---
>>>  drivers/dma-buf/dma-buf.c      |  57 ++++++++++-
>>>  include/trace/events/dma_buf.h | 166 +++++++++++++++++++++++++++++++++
>>>  2 files changed, 222 insertions(+), 1 deletion(-)
>>>  create mode 100644 include/trace/events/dma_buf.h
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 2bcf9ceca997..7cef816ddcac 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -35,6 +35,18 @@
>>>
>>>  #include "dma-buf-sysfs-stats.h"
>>>
>>> +#define CREATE_TRACE_POINTS
>>> +#include <trace/events/dma_buf.h>
>>> +
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_export);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_mmap_internal);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_mmap);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_put);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_attach);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_detach);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_fd);
>>> +EXPORT_TRACEPOINT_SYMBOL(dma_buf_get);
>>
>> Exporting the tracepoints is unecessary as far as I can see
>>
>>> +
>>>  static inline int is_dma_buf_file(struct file *);
>>>
>>>  static DEFINE_MUTEX(dmabuf_list_mutex);
>>> @@ -220,6 +232,11 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
>>>           dmabuf->size >> PAGE_SHIFT)
>>>               return -EINVAL;
>>>
>>> +     if (trace_dma_buf_mmap_internal_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_mmap_internal(dmabuf);
>>> +     }
>>> +
>>>       return dmabuf->ops->mmap(dmabuf, vma);
>>>  }
>>>
>>> @@ -745,6 +762,11 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>>>
>>>       __dma_buf_list_add(dmabuf);
>>>
>>> +     if (trace_dma_buf_export_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_export(dmabuf);
>>> +     }
>>> +
>>>       return dmabuf;
>>>
>>>  err_dmabuf:
>>> @@ -779,6 +801,11 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags)
>>>
>>>       fd_install(fd, dmabuf->file);
>>>
>>> +     if (trace_dma_buf_fd_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_fd(dmabuf, fd);
>>> +     }
>>> +
>>>       return fd;
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
>>> @@ -794,6 +821,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
>>>  struct dma_buf *dma_buf_get(int fd)
>>>  {
>>>       struct file *file;
>>> +     struct dma_buf *dmabuf;
>>>
>>>       file = fget(fd);
>>>
>>> @@ -805,7 +833,14 @@ struct dma_buf *dma_buf_get(int fd)
>>>               return ERR_PTR(-EINVAL);
>>>       }
>>>
>>> -     return file->private_data;
>>> +     dmabuf = file->private_data;
>>> +
>>> +     if (trace_dma_buf_get_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_get(dmabuf, fd);
>>> +     }
>>> +
>>> +     return dmabuf;
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(dma_buf_get, "DMA_BUF");
>>>
>>> @@ -825,6 +860,11 @@ void dma_buf_put(struct dma_buf *dmabuf)
>>>               return;
>>>
>>>       fput(dmabuf->file);
>>> +
>>> +     if (trace_dma_buf_put_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_put(dmabuf);
>>> +     }
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
>>>
>>> @@ -998,6 +1038,11 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_dynamic_attach, "DMA_BUF");
>>>  struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
>>>                                         struct device *dev)
>>>  {
>>> +     if (trace_dma_buf_attach_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_attach(dmabuf, dev);
>>> +     }
>>> +
>>>       return dma_buf_dynamic_attach(dmabuf, dev, NULL, NULL);
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(dma_buf_attach, "DMA_BUF");
>>> @@ -1023,6 +1068,11 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
>>>       if (dmabuf->ops->detach)
>>>               dmabuf->ops->detach(dmabuf, attach);
>>>
>>> +     if (trace_dma_buf_detach_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_detach(dmabuf, attach->dev);
>>> +     }
>>> +
>>>       kfree(attach);
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(dma_buf_detach, "DMA_BUF");
>>> @@ -1488,6 +1538,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
>>>       vma_set_file(vma, dmabuf->file);
>>>       vma->vm_pgoff = pgoff;
>>>
>>> +     if (trace_dma_buf_mmap_enabled()) {
>>> +             guard(spinlock)(&dmabuf->name_lock);
>>> +             trace_dma_buf_mmap(dmabuf);
>>> +     }
>>> +
>>>       return dmabuf->ops->mmap(dmabuf, vma);
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, "DMA_BUF");
>>> diff --git a/include/trace/events/dma_buf.h b/include/trace/events/dma_buf.h
>>> new file mode 100644
>>> index 000000000000..fe9da89bacd0
>>> --- /dev/null
>>> +++ b/include/trace/events/dma_buf.h
>>> @@ -0,0 +1,166 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#undef TRACE_SYSTEM
>>> +#define TRACE_SYSTEM dma_buf
>>> +
>>> +#if !defined(_TRACE_DMA_BUF_H) || defined(TRACE_HEADER_MULTI_READ)
>>> +#define _TRACE_DMA_BUF_H
>>> +
>>> +#include <linux/dma-buf.h>
>>> +#include <linux/tracepoint.h>
>>> +
>>> +DECLARE_EVENT_CLASS(dma_buf,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf),
>>> +
>>> +     TP_ARGS(dmabuf),
>>> +
>>> +     TP_STRUCT__entry(
>>> +             __string(exp_name, dmabuf->exp_name)
>>> +             __string(name, dmabuf->name)
>>> +             __field(size_t, size)
>>> +             __field(ino_t, ino)
>>> +             __field(long, f_refcnt)
>>> +     ),
>>> +
>>> +     TP_fast_assign(
>>> +             __assign_str(exp_name);
>>> +             __assign_str(name);
>>> +             __entry->size = dmabuf->size;
>>> +             __entry->ino = dmabuf->file->f_inode->i_ino;
>>> +             __entry->f_refcnt = file_count(dmabuf->file);
>>> +     ),
>>> +
>>> +     TP_printk("exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
>>> +               __get_str(exp_name),
>>> +               __get_str(name),
>>> +               __entry->size,
>>> +               __entry->ino,
>>> +               __entry->f_refcnt)
>>
>> I can't come up with a reason why we should have the file refcount in the trace logs.
>>
>> Same for most other places.
>>
>>> +);
>>> +
>>> +DECLARE_EVENT_CLASS(dma_buf_attach_dev,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf, struct device *dev),
>>> +
>>> +     TP_ARGS(dmabuf, dev),
>>> +
>>> +     TP_STRUCT__entry(
>>> +             __string(dname, dev_name(dev))
>>> +             __string(exp_name, dmabuf->exp_name)
>>> +             __string(name, dmabuf->name)
>>> +             __field(size_t, size)
>>> +             __field(ino_t, ino)
>>> +             __field(long, f_refcnt)
>>> +     ),
>>> +
>>> +     TP_fast_assign(
>>> +             __assign_str(dname);
>>> +             __assign_str(exp_name);
>>> +             __assign_str(name);
>>> +             __entry->size = dmabuf->size;
>>> +             __entry->ino = dmabuf->file->f_inode->i_ino;
>>> +             __entry->f_refcnt = file_count(dmabuf->file);
>>> +     ),
>>> +
>>> +     TP_printk("dev_name=%s exp_name=%s name=%s size=%zu ino=%lu f_refcnt=%ld",
>>> +               __get_str(dname),
>>> +               __get_str(exp_name),
>>> +               __get_str(name),
>>> +               __entry->size,
>>> +               __entry->ino,
>>> +               __entry->f_refcnt)
>>
>> Print the dev_name last, it's the external device which attaches to the DMA-buf.
>>
>> Additional to that it would be nice to know if the attachment is dynamic or not.
>>
>> Regards,
>> Christian.
>>
>>> +);
>>> +
>>> +DECLARE_EVENT_CLASS(dma_buf_fd,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf, int fd),
>>> +
>>> +     TP_ARGS(dmabuf, fd),
>>> +
>>> +     TP_STRUCT__entry(
>>> +             __string(exp_name, dmabuf->exp_name)
>>> +             __string(name, dmabuf->name)
>>> +             __field(size_t, size)
>>> +             __field(ino_t, ino)
>>> +             __field(int, fd)
>>> +             __field(long, f_refcnt)
>>> +     ),
>>> +
>>> +     TP_fast_assign(
>>> +             __assign_str(exp_name);
>>> +             __assign_str(name);
>>> +             __entry->size = dmabuf->size;
>>> +             __entry->ino = dmabuf->file->f_inode->i_ino;
>>> +             __entry->fd = fd;
>>> +             __entry->f_refcnt = file_count(dmabuf->file);
>>> +     ),
>>> +
>>> +     TP_printk("exp_name=%s name=%s size=%zu ino=%lu fd=%d f_refcnt=%ld",
>>> +               __get_str(exp_name),
>>> +               __get_str(name),
>>> +               __entry->size,
>>> +               __entry->ino,
>>> +               __entry->fd,
>>> +               __entry->f_refcnt)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf, dma_buf_export,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf),
>>> +
>>> +     TP_ARGS(dmabuf)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf, dma_buf_mmap_internal,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf),
>>> +
>>> +     TP_ARGS(dmabuf)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf, dma_buf_mmap,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf),
>>> +
>>> +     TP_ARGS(dmabuf)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf, dma_buf_put,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf),
>>> +
>>> +     TP_ARGS(dmabuf)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf_attach_dev, dma_buf_attach,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf, struct device *dev),
>>> +
>>> +     TP_ARGS(dmabuf, dev)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf_attach_dev, dma_buf_detach,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf, struct device *dev),
>>> +
>>> +     TP_ARGS(dmabuf, dev)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf_fd, dma_buf_fd,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf, int fd),
>>> +
>>> +     TP_ARGS(dmabuf, fd)
>>> +);
>>> +
>>> +DEFINE_EVENT(dma_buf_fd, dma_buf_get,
>>> +
>>> +     TP_PROTO(struct dma_buf *dmabuf, int fd),
>>> +
>>> +     TP_ARGS(dmabuf, fd)
>>> +);
>>> +
>>> +#endif /* _TRACE_DMA_BUF_H */
>>> +
>>> +/* This part must be outside protection */
>>> +#include <trace/define_trace.h>
>>
>