Re: [PATCH v2] btrfs: trace use file_inode(file)->i_sb to calculate fs_info

[Filipe Manana <fdmanana@kernel.org>]

On Thu, Feb 19, 2026 at 1:50 AM Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
>
> If overlay is used on top of btrfs, dentry->d_sb translates to overlay's
> super block and fsid assignment will lead to a crash.
>
> Use file_inode(file)->i_sb to always get btrfs_sb.
>
> Changes since v1:
>   Changed subject to include trace
>   Use file_inode() to get inode pointer

Information about what changes between patch versions doesn't go into
the change log, it goes below the line marked as "---", as that's
irrelevant information to have in git, it's only useful for patch
reviews.

This subject:

"btrfs: trace use file_inode(file)->i_sb to calculate fs_info"

is also odd, using a C expression, not saying where (which trace
event) and not saying what problem are we fixing but rather how are we
fixing the problem.

I suggest something much more clear and concise such as:

"btrfs: fix a crash in the trace event btrfs_sync_file()"

One further comment below.

>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> Reviewed-by: Boris Burkov <boris@bur.io>
> ---
>  include/trace/events/btrfs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
> index 125bdc166bfe..92118df217b4 100644
> --- a/include/trace/events/btrfs.h
> +++ b/include/trace/events/btrfs.h
> @@ -770,9 +770,9 @@ TRACE_EVENT(btrfs_sync_file,
>
>         TP_fast_assign(
>                 const struct dentry *dentry = file->f_path.dentry;

Shouldn't we also use file_dentry(file) here?

I think we should, otherwise we get the same bug that was fixed in:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=de17e793b104d690e1d007dfc5cb6b4f649598ca

> -               const struct inode *inode = d_inode(dentry);
> +               const struct inode *inode = file_inode(file);
>
> -               TP_fast_assign_fsid(btrfs_sb(file->f_path.dentry->d_sb));
> +               TP_fast_assign_fsid(btrfs_sb(inode->i_sb));
>                 __entry->ino            = btrfs_ino(BTRFS_I(inode));
>                 __entry->parent         = btrfs_ino(BTRFS_I(d_inode(dentry->d_parent)));

And here, why didn't you replace d_inode() with file_inode() like above?

Thanks.

>                 __entry->datasync       = datasync;
> --
> 2.53.0
>
>
> --
> Goldwyn
>