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

[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 <liubaolin@xxxxxxxxxx>
> 
> 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