Re: [PATCH v2 v2 2/7] ntfs3: add namei tracepoints
liubaolin <[email protected]> Mon, 3 Aug 2026 09:50:30 +0800
| Newsgroups | dev.linux.lists.ntfs3,org.kernel.vger.linux-kernel,org.kernel.vger.linux-trace-kernel |
|---|---|
| Message-ID | <[email protected]> |
Dear Steve,
Thank you for your review. I will carefully recheck and modify all
patches according to your feedback, and submit the v3 version patch as
soon as possible. Thank you again for your feedback.
Best regards,
Baolin
在 2026/7/30 02:26, Steven Rostedt 写道:
> On Tue, 28 Jul 2026 17:44:21 +0800
> Baolin Liu <[email protected]> wrote:
>
>> diff --git a/include/trace/events/ntfs3.h b/include/trace/events/ntfs3.h
>> index d00f66e42ba8..041455edec76 100644
>> --- a/include/trace/events/ntfs3.h
>> +++ b/include/trace/events/ntfs3.h
>> @@ -80,6 +80,50 @@ TRACE_EVENT(ntfs3_log_replay,
>> __entry->size, __entry->initialized, __entry->err)
>> );
>>
>> +TRACE_EVENT(ntfs3_lookup,
>> + TP_PROTO(struct inode *dir, struct dentry *dentry),
>> + TP_ARGS(dir, dentry),
>> + TP_STRUCT__entry(
>> + __field(dev_t, dev)
>
> dev_t is 4 bytes, you should move it after parent_ino as on 64 bit
> machines, it will waste 4 bytes of space here.
>
>> + __field(unsigned long, parent_ino)
>> + __string(name, dentry->d_name.name)
>> + ),
>> + TP_fast_assign(
>> + __entry->dev = dir->i_sb->s_dev;
>> + __entry->parent_ino = dir->i_ino;
>> + __assign_str(name);
>> + ),
>> + TP_printk("dev=(%d,%d) parent=%lu name=%s",
>> + MAJOR(__entry->dev), MINOR(__entry->dev),
>> + __entry->parent_ino, __get_str(name))
>> +);
>> +
>> +TRACE_EVENT(ntfs3_rename,
>> + TP_PROTO(struct inode *dir, struct dentry *dentry,
>> + struct inode *new_dir, struct dentry *new_dentry),
>> + TP_ARGS(dir, dentry, new_dir, new_dentry),
>> + TP_STRUCT__entry(
>> + __field(dev_t, dev)
>
> Same here.
>
> In fact, you should audit the rest of your tracepoints to see where you can
> make the sizes more condensed.
>
> ~# cat /sys/kernel/tracing/events/ntfs3/ntfs3_rename/format
> name: ntfs3_rename
> ID: 1923
> format:
> field:unsigned short common_type; offset:0; size:2; signed:0;
> field:unsigned char common_flags; offset:2; size:1; signed:0;
> field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
> field:int common_pid; offset:4; size:4; signed:1;
>
> field:dev_t dev; offset:8; size:4; signed:0;
> field:unsigned long dir_ino; offset:16; size:8; signed:0;
> field:unsigned long new_dir_ino; offset:24; size:8; signed:0;
> field:unsigned long ino; offset:32; size:8; signed:0;
> field:__data_loc char[] old_name; offset:40; size:4; signed:0;
> field:__data_loc char[] new_name; offset:44; size:4; signed:0;
>
>
> Notice that dev is at offset 8 and size 4, where as dir_ino is at offset
> 16. That means there's 4 bytes between dev and dir_ino.
>
> Please look at your other trace events and see if there are other gaps you
> can close. I'm not going to audit all your tracepoints.
>
> Note that __string() is a 4 byte meta data, so have them act like 4 byte
> fields.
>
> -- Steve
>
>
>
>> + __field(unsigned long, dir_ino)
>> + __field(unsigned long, new_dir_ino)
>> + __field(unsigned long, ino)
>> + __string(old_name, dentry->d_name.name)
>> + __string(new_name, new_dentry->d_name.name)
>> + ),
>> + TP_fast_assign(
>> + __entry->dev = dir->i_sb->s_dev;
>> + __entry->dir_ino = dir->i_ino;
>> + __entry->new_dir_ino = new_dir->i_ino;
>> + __entry->ino = d_inode(dentry)->i_ino;
>> + __assign_str(old_name);
>> + __assign_str(new_name);
>> + ),
>> + TP_printk("dev=(%d,%d) dir=%lu new_dir=%lu ino=%lu old=%s new=%s",
>> + MAJOR(__entry->dev), MINOR(__entry->dev),
>> + __entry->dir_ino, __entry->new_dir_ino, __entry->ino,
>> + __get_str(old_name), __get_str(new_name))
>> +);
>> +
>> #endif /* _TRACE_NTFS3_H */
>>