Re: [PATCH v3 v3 5/7] ntfs3: add allocation tracepoints

Dear Steven,
    Thanks for your review. You are right, I will fix it as suggested in v4.


Thanks,
Baolin

在 2026/8/12 04:21, Steven Rostedt 写道:
> Also Sashiko had something to say about this patch: https://sashiko.dev/#/patchset/20260807010354.2277156-8-liubaolin12138%40163.com
> 
>> --- Patch [5]: [PATCH v3 v3 5/7] ntfs3: add allocation tracepoints ---
>> commit 0592c540bc564c346cb524c624f6ee4a5768406b
>> Author: Baolin Liu <[email protected]>
>>
>> ntfs3: add allocation tracepoints
>>
>> This commit adds trace events for ntfs3 allocation operations. It introduces
>> events for attr_allocate_clusters, attr_set_size_ex, and attr_data_get_block
>> to help observe cluster allocation, size changes, and block mapping activity.
>>
>>> diff --git a/include/trace/events/ntfs3.h b/include/trace/events/ntfs3.h
>>> index 4ac05ed29e49d..d16dd127ff2e5 100644
>>> --- a/include/trace/events/ntfs3.h
>>> +++ b/include/trace/events/ntfs3.h
>> [ ... ]
>>> +TRACE_EVENT(ntfs3_attr_set_size_ex,
>>> +	TP_PROTO(struct inode *inode, u32 type, u64 new_size, bool keep_prealloc,
>>> +		 bool no_da),
>>> +	TP_ARGS(inode, type, new_size, keep_prealloc, no_da),
>>> +	TP_STRUCT__entry(
>>> +		__field(loff_t, old_size)
>>> +		__field(u64, new_size)
>>> +		__field(unsigned long, ino)
>>> +		__field(dev_t, dev)
>>> +		__field(u32, type)
>>> +		__field(bool, keep_prealloc)
>>> +		__field(bool, no_da)
>>> +	),
>>> +	TP_fast_assign(
>>> +		__entry->old_size = i_size_read(inode);
>>
>> Will this actually record the previous size during truncate operations?
>>
>> When an application truncates a file, the VFS size is often updated before
>> calling into the filesystem. For instance, looking at how ntfs_setattr()
>> handles this:
>>
>> fs/ntfs3/file.c:ntfs_setattr() {
>>          ...
>>          truncate_setsize(inode, newsize);
>>          
>>          err = ntfs_set_size(inode, newsize);
>>          ...
>> }
>>
>> Since truncate_setsize() updates the VFS inode size before attr_set_size_ex()
>> and this tracepoint are reached, won't i_size_read(inode) just return the
>> newly truncated size, resulting in old_size equaling new_size?
>>
>> Could the actual old size be passed as an explicit parameter to the
>> tracepoint instead of sampling it here?
>>
>>> +		__entry->new_size = new_size;
>>> +		__entry->ino = inode->i_ino;
>>> +		__entry->dev = inode->i_sb->s_dev;
>>> +		__entry->type = type;
>>> +		__entry->keep_prealloc = keep_prealloc;
>>> +		__entry->no_da = no_da;
>>> +	),
>> [ ... ]
> 
> -- Steve