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 */
>>