ok, thanks. ________________________________ 发件人: Steven Rostedt <[email protected]> 发送时间: 2025年11月26日 9:39:24 收件人: Xiang Gao 抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; 高翔 主题: [External Mail]Re: [PATCH v2] dma-buf: add some tracepoints to debug.
[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给[email protected]进行反馈 On Wed, 26 Nov 2025 00:29:49 +0800 Xiang Gao <[email protected]> wrote: > +++ b/include/trace/events/dma_buf.h > @@ -0,0 +1,281 @@ > +/* 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> > + > +TRACE_EVENT(dma_buf_export, > + > + 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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); The above isn't doing what you think it's doing. The name is assigned before this by the above __string(name, dmabuf->name). You really shouldn't be taking any locks in a tracepoint. A tracepoint is a callback, that isn't called most of the time. You could be hiding very hard to find deadlocks by taking a lock in a tracepoint callback. You need to take the lock around the tracepoint call itself where it is called in the code. Not in the TRACE_EVENT. You may need to have something like: @@ -220,6 +223,8 @@ 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->namelock); + trace_dma_buf_mmap_internal(dmabuf); + } + return dmabuf->ops->mmap(dmabuf, vma); } The "trace_dma_buf_mmap_internal_enabled()" is a static branch, where it is either a nop or a jump to the tracing code. It's not a normal conditional branch. It acts the same as tracepoints themselves do. > + __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) > +); Below seems to be a lot of very similar TRACE_EVENT()s. A TRACE_EVENT() is literally defined as: #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \ DECLARE_EVENT_CLASS(name, \ PARAMS(proto), \ PARAMS(args), \ PARAMS(tstruct), \ PARAMS(assign), \ PARAMS(print)); \ DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args)); That is, it is both a DECLARE_EVENT_CLASS() and a DEFINE_EVENT(). You can make one DECLARE_EVENT_CLASS() and use many DEFINE_EVENT()s with it. Each DECLARE_EVENT_CLASS() takes up around 4 to 5 kilobytes of memory. Each DEFINE_EVENT() takes around 300 bytes to 1K of memory. The more DEFINE_EVENT()s you use with a single DECLARE_EVENT_CLASS(), the more memory you save. Please try to do that. -- Steve > + > +TRACE_EVENT(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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __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) > +); > + > +TRACE_EVENT(dma_buf_mmap_internal, > + > + 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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __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) > +); > + > +TRACE_EVENT(dma_buf_mmap, > + > + 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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __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) > +); > + > +TRACE_EVENT(dma_buf_attach, > + > + 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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __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) > +); > + > +TRACE_EVENT(dma_buf_detach, > + > + 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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __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) > +); > + > +TRACE_EVENT(dma_buf_get, > + > + TP_PROTO(int fd, struct file *file), > + > + TP_ARGS(fd, file), > + > + TP_STRUCT__entry( > + __string(exp_name, ((struct dma_buf > *)file->private_data)->exp_name) > + __string(name, ((struct dma_buf *)file->private_data)->name) > + __field(size_t, size) > + __field(ino_t, ino) > + __field(int, fd) > + __field(long, f_refcnt) > + ), > + > + TP_fast_assign( > + struct dma_buf *dmabuf = (struct dma_buf *)file->private_data; > + > + __assign_str(exp_name); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __entry->size = dmabuf->size; > + __entry->ino = dmabuf->file->f_inode->i_ino; > + __entry->fd = fd; > + __entry->f_refcnt = file_count(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) > +); > + > +TRACE_EVENT(dma_buf_put, > + > + 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); > + spin_lock(&dmabuf->name_lock); > + __assign_str(name); > + spin_unlock(&dmabuf->name_lock); > + __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) > +); > + > +#endif /* _TRACE_DMA_BUF_H */ > + > +/* This part must be outside protection */ > +#include <trace/define_trace.h>
