| Message ID | 20250118200330.21020-1-devosruben6@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | smb: client: fix order of arguments of tracepoints | expand |
Good catch.
Looking at your patch, I noticed one trace point was missing from the
original patch:
trace_smb3_query_wsl_ea_compound_enter
commit ea41367b2a602f602ea6594fc4a310520dcc64f4
Author: Paulo Alcantara <pc@manguebit.com>
Date: Sun Jan 28 01:12:01 2024 -0300
smb: client: introduce SMB2_OP_QUERY_WSL_EA
On Sat, Jan 18, 2025 at 2:04 PM Ruben Devos <devosruben6@gmail.com> wrote:
>
> The tracepoints based on smb3_inf_compound_*_class have tcon id and
> session id swapped around. This results in incorrect output in
> `trace-cmd report`.
>
> Fix the order of arguments to resolve this issue. The trace-cmd output
> below shows the before and after of the smb3_delete_enter and
> smb3_delete_done events as an example. The smb3_cmd_* events show the
> correct session and tcon id for reference.
>
> Also fix tracepoint set -> get in the SMB2_OP_GET_REPARSE case.
>
> BEFORE:
> rm-2211 [001] ..... 1839.550888: smb3_delete_enter: xid=281 sid=0x5 tid=0x3d path=\hello2.txt
> rm-2211 [001] ..... 1839.550894: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61
> rm-2211 [001] ..... 1839.550896: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62
> rm-2211 [001] ..... 1839.552091: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61
> rm-2211 [001] ..... 1839.552093: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62
> rm-2211 [001] ..... 1839.552103: smb3_delete_done: xid=281 sid=0x5 tid=0x3d
>
> AFTER:
> rm-2501 [001] ..... 3237.656110: smb3_delete_enter: xid=88 sid=0x1ac0000000041 tid=0x5 path=\hello2.txt
> rm-2501 [001] ..... 3237.656122: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84
> rm-2501 [001] ..... 3237.656123: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85
> rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84
> rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85
> rm-2501 [001] ..... 3237.657922: smb3_delete_done: xid=88 sid=0x1ac0000000041 tid=0x5
>
> Signed-off-by: Ruben Devos <devosruben6@gmail.com>
> ---
> fs/smb/client/dir.c | 6 +--
> fs/smb/client/smb2inode.c | 108 +++++++++++++++++++-------------------
> 2 files changed, 57 insertions(+), 57 deletions(-)
>
> diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> index 864b194dbaa0..1822493dd084 100644
> --- a/fs/smb/client/dir.c
> +++ b/fs/smb/client/dir.c
> @@ -627,7 +627,7 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode,
> goto mknod_out;
> }
>
> - trace_smb3_mknod_enter(xid, tcon->ses->Suid, tcon->tid, full_path);
> + trace_smb3_mknod_enter(xid, tcon->tid, tcon->ses->Suid, full_path);
>
> rc = tcon->ses->server->ops->make_node(xid, inode, direntry, tcon,
> full_path, mode,
> @@ -635,9 +635,9 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode,
>
> mknod_out:
> if (rc)
> - trace_smb3_mknod_err(xid, tcon->ses->Suid, tcon->tid, rc);
> + trace_smb3_mknod_err(xid, tcon->tid, tcon->ses->Suid, rc);
> else
> - trace_smb3_mknod_done(xid, tcon->ses->Suid, tcon->tid);
> + trace_smb3_mknod_done(xid, tcon->tid, tcon->ses->Suid);
>
> free_dentry_path(page);
> free_xid(xid);
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index a55f0044d30b..274672755c19 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -298,8 +298,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_query_info_compound_enter(xid, ses->Suid,
> - tcon->tid, full_path);
> + trace_smb3_query_info_compound_enter(xid, tcon->tid,
> + ses->Suid, full_path);
> break;
> case SMB2_OP_POSIX_QUERY_INFO:
> rqst[num_rqst].rq_iov = &vars->qi_iov;
> @@ -334,18 +334,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_posix_query_info_compound_enter(xid, ses->Suid,
> - tcon->tid, full_path);
> + trace_smb3_posix_query_info_compound_enter(xid, tcon->tid,
> + ses->Suid, full_path);
> break;
> case SMB2_OP_DELETE:
> - trace_smb3_delete_enter(xid, ses->Suid, tcon->tid, full_path);
> + trace_smb3_delete_enter(xid, tcon->tid, ses->Suid, full_path);
> break;
> case SMB2_OP_MKDIR:
> /*
> * Directories are created through parameters in the
> * SMB2_open() call.
> */
> - trace_smb3_mkdir_enter(xid, ses->Suid, tcon->tid, full_path);
> + trace_smb3_mkdir_enter(xid, tcon->tid, ses->Suid, full_path);
> break;
> case SMB2_OP_RMDIR:
> rqst[num_rqst].rq_iov = &vars->si_iov[0];
> @@ -363,7 +363,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> smb2_set_next_command(tcon, &rqst[num_rqst]);
> smb2_set_related(&rqst[num_rqst++]);
> - trace_smb3_rmdir_enter(xid, ses->Suid, tcon->tid, full_path);
> + trace_smb3_rmdir_enter(xid, tcon->tid, ses->Suid, full_path);
> break;
> case SMB2_OP_SET_EOF:
> rqst[num_rqst].rq_iov = &vars->si_iov[0];
> @@ -398,7 +398,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_set_eof_enter(xid, ses->Suid, tcon->tid, full_path);
> + trace_smb3_set_eof_enter(xid, tcon->tid, ses->Suid, full_path);
> break;
> case SMB2_OP_SET_INFO:
> rqst[num_rqst].rq_iov = &vars->si_iov[0];
> @@ -429,8 +429,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_set_info_compound_enter(xid, ses->Suid,
> - tcon->tid, full_path);
> + trace_smb3_set_info_compound_enter(xid, tcon->tid,
> + ses->Suid, full_path);
> break;
> case SMB2_OP_RENAME:
> rqst[num_rqst].rq_iov = &vars->si_iov[0];
> @@ -469,7 +469,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_rename_enter(xid, ses->Suid, tcon->tid, full_path);
> + trace_smb3_rename_enter(xid, tcon->tid, ses->Suid, full_path);
> break;
> case SMB2_OP_HARDLINK:
> rqst[num_rqst].rq_iov = &vars->si_iov[0];
> @@ -496,7 +496,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> smb2_set_next_command(tcon, &rqst[num_rqst]);
> smb2_set_related(&rqst[num_rqst++]);
> - trace_smb3_hardlink_enter(xid, ses->Suid, tcon->tid, full_path);
> + trace_smb3_hardlink_enter(xid, tcon->tid, ses->Suid, full_path);
> break;
> case SMB2_OP_SET_REPARSE:
> rqst[num_rqst].rq_iov = vars->io_iov;
> @@ -523,8 +523,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_set_reparse_compound_enter(xid, ses->Suid,
> - tcon->tid, full_path);
> + trace_smb3_set_reparse_compound_enter(xid, tcon->tid,
> + ses->Suid, full_path);
> break;
> case SMB2_OP_GET_REPARSE:
> rqst[num_rqst].rq_iov = vars->io_iov;
> @@ -549,8 +549,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> goto finished;
> }
> num_rqst++;
> - trace_smb3_get_reparse_compound_enter(xid, ses->Suid,
> - tcon->tid, full_path);
> + trace_smb3_get_reparse_compound_enter(xid, tcon->tid,
> + ses->Suid, full_path);
> break;
> case SMB2_OP_QUERY_WSL_EA:
> rqst[num_rqst].rq_iov = &vars->ea_iov;
> @@ -656,11 +656,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> }
> SMB2_query_info_free(&rqst[num_rqst++]);
> if (rc)
> - trace_smb3_query_info_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_query_info_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> else
> - trace_smb3_query_info_compound_done(xid, ses->Suid,
> - tcon->tid);
> + trace_smb3_query_info_compound_done(xid, tcon->tid,
> + ses->Suid);
> break;
> case SMB2_OP_POSIX_QUERY_INFO:
> idata = in_iov[i].iov_base;
> @@ -683,15 +683,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
>
> SMB2_query_info_free(&rqst[num_rqst++]);
> if (rc)
> - trace_smb3_posix_query_info_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_posix_query_info_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> else
> - trace_smb3_posix_query_info_compound_done(xid, ses->Suid,
> - tcon->tid);
> + trace_smb3_posix_query_info_compound_done(xid, tcon->tid,
> + ses->Suid);
> break;
> case SMB2_OP_DELETE:
> if (rc)
> - trace_smb3_delete_err(xid, ses->Suid, tcon->tid, rc);
> + trace_smb3_delete_err(xid, tcon->tid, ses->Suid, rc);
> else {
> /*
> * If dentry (hence, inode) is NULL, lease break is going to
> @@ -699,59 +699,59 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> */
> if (inode)
> cifs_mark_open_handles_for_deleted_file(inode, full_path);
> - trace_smb3_delete_done(xid, ses->Suid, tcon->tid);
> + trace_smb3_delete_done(xid, tcon->tid, ses->Suid);
> }
> break;
> case SMB2_OP_MKDIR:
> if (rc)
> - trace_smb3_mkdir_err(xid, ses->Suid, tcon->tid, rc);
> + trace_smb3_mkdir_err(xid, tcon->tid, ses->Suid, rc);
> else
> - trace_smb3_mkdir_done(xid, ses->Suid, tcon->tid);
> + trace_smb3_mkdir_done(xid, tcon->tid, ses->Suid);
> break;
> case SMB2_OP_HARDLINK:
> if (rc)
> - trace_smb3_hardlink_err(xid, ses->Suid, tcon->tid, rc);
> + trace_smb3_hardlink_err(xid, tcon->tid, ses->Suid, rc);
> else
> - trace_smb3_hardlink_done(xid, ses->Suid, tcon->tid);
> + trace_smb3_hardlink_done(xid, tcon->tid, ses->Suid);
> SMB2_set_info_free(&rqst[num_rqst++]);
> break;
> case SMB2_OP_RENAME:
> if (rc)
> - trace_smb3_rename_err(xid, ses->Suid, tcon->tid, rc);
> + trace_smb3_rename_err(xid, tcon->tid, ses->Suid, rc);
> else
> - trace_smb3_rename_done(xid, ses->Suid, tcon->tid);
> + trace_smb3_rename_done(xid, tcon->tid, ses->Suid);
> SMB2_set_info_free(&rqst[num_rqst++]);
> break;
> case SMB2_OP_RMDIR:
> if (rc)
> - trace_smb3_rmdir_err(xid, ses->Suid, tcon->tid, rc);
> + trace_smb3_rmdir_err(xid, tcon->tid, ses->Suid, rc);
> else
> - trace_smb3_rmdir_done(xid, ses->Suid, tcon->tid);
> + trace_smb3_rmdir_done(xid, tcon->tid, ses->Suid);
> SMB2_set_info_free(&rqst[num_rqst++]);
> break;
> case SMB2_OP_SET_EOF:
> if (rc)
> - trace_smb3_set_eof_err(xid, ses->Suid, tcon->tid, rc);
> + trace_smb3_set_eof_err(xid, tcon->tid, ses->Suid, rc);
> else
> - trace_smb3_set_eof_done(xid, ses->Suid, tcon->tid);
> + trace_smb3_set_eof_done(xid, tcon->tid, ses->Suid);
> SMB2_set_info_free(&rqst[num_rqst++]);
> break;
> case SMB2_OP_SET_INFO:
> if (rc)
> - trace_smb3_set_info_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_set_info_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> else
> - trace_smb3_set_info_compound_done(xid, ses->Suid,
> - tcon->tid);
> + trace_smb3_set_info_compound_done(xid, tcon->tid,
> + ses->Suid);
> SMB2_set_info_free(&rqst[num_rqst++]);
> break;
> case SMB2_OP_SET_REPARSE:
> if (rc) {
> - trace_smb3_set_reparse_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_set_reparse_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> } else {
> - trace_smb3_set_reparse_compound_done(xid, ses->Suid,
> - tcon->tid);
> + trace_smb3_set_reparse_compound_done(xid, tcon->tid,
> + ses->Suid);
> }
> SMB2_ioctl_free(&rqst[num_rqst++]);
> break;
> @@ -764,18 +764,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> rbuf = reparse_buf_ptr(iov);
> if (IS_ERR(rbuf)) {
> rc = PTR_ERR(rbuf);
> - trace_smb3_set_reparse_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_get_reparse_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> } else {
> idata->reparse.tag = le32_to_cpu(rbuf->ReparseTag);
> - trace_smb3_set_reparse_compound_done(xid, ses->Suid,
> - tcon->tid);
> + trace_smb3_get_reparse_compound_done(xid, tcon->tid,
> + ses->Suid);
> }
> memset(iov, 0, sizeof(*iov));
> resp_buftype[i + 1] = CIFS_NO_BUFFER;
> } else {
> - trace_smb3_set_reparse_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_get_reparse_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> }
> SMB2_ioctl_free(&rqst[num_rqst++]);
> break;
> @@ -792,11 +792,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> }
> }
> if (!rc) {
> - trace_smb3_query_wsl_ea_compound_done(xid, ses->Suid,
> - tcon->tid);
> + trace_smb3_query_wsl_ea_compound_done(xid, tcon->tid,
> + ses->Suid);
> } else {
> - trace_smb3_query_wsl_ea_compound_err(xid, ses->Suid,
> - tcon->tid, rc);
> + trace_smb3_query_wsl_ea_compound_err(xid, tcon->tid,
> + ses->Suid, rc);
> }
> SMB2_query_info_free(&rqst[num_rqst++]);
> break;
> --
> 2.48.1
>
>
Here is a minor patch to add the missing tracepoint (see attached).
Let me know if any thoughts, or other obviousb missing tracepoints
smb3: add missing tracepoint for querying wsl EAs
We had tracepoints for the return code for querying WSL EAs
(trace_smb3_query_wsl_ea_compound_err and
trace_smb3_query_wsl_ea_compound_done) but were missing one for
trace_smb3_query_wsl_ea_compound_enter.
Fixes: ea41367b2a60 ("smb: client: introduce SMB2_OP_QUERY_WSL_EA")
On Sat, Jan 18, 2025 at 10:38 PM Steve French <smfrench@gmail.com> wrote:
>
> Good catch.
>
> Looking at your patch, I noticed one trace point was missing from the
> original patch:
>
> trace_smb3_query_wsl_ea_compound_enter
>
> commit ea41367b2a602f602ea6594fc4a310520dcc64f4
> Author: Paulo Alcantara <pc@manguebit.com>
> Date: Sun Jan 28 01:12:01 2024 -0300
>
> smb: client: introduce SMB2_OP_QUERY_WSL_EA
>
> On Sat, Jan 18, 2025 at 2:04 PM Ruben Devos <devosruben6@gmail.com> wrote:
> >
> > The tracepoints based on smb3_inf_compound_*_class have tcon id and
> > session id swapped around. This results in incorrect output in
> > `trace-cmd report`.
> >
> > Fix the order of arguments to resolve this issue. The trace-cmd output
> > below shows the before and after of the smb3_delete_enter and
> > smb3_delete_done events as an example. The smb3_cmd_* events show the
> > correct session and tcon id for reference.
> >
> > Also fix tracepoint set -> get in the SMB2_OP_GET_REPARSE case.
> >
> > BEFORE:
> > rm-2211 [001] ..... 1839.550888: smb3_delete_enter: xid=281 sid=0x5 tid=0x3d path=\hello2.txt
> > rm-2211 [001] ..... 1839.550894: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61
> > rm-2211 [001] ..... 1839.550896: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62
> > rm-2211 [001] ..... 1839.552091: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61
> > rm-2211 [001] ..... 1839.552093: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62
> > rm-2211 [001] ..... 1839.552103: smb3_delete_done: xid=281 sid=0x5 tid=0x3d
> >
> > AFTER:
> > rm-2501 [001] ..... 3237.656110: smb3_delete_enter: xid=88 sid=0x1ac0000000041 tid=0x5 path=\hello2.txt
> > rm-2501 [001] ..... 3237.656122: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84
> > rm-2501 [001] ..... 3237.656123: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85
> > rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84
> > rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85
> > rm-2501 [001] ..... 3237.657922: smb3_delete_done: xid=88 sid=0x1ac0000000041 tid=0x5
> >
> > Signed-off-by: Ruben Devos <devosruben6@gmail.com>
> > ---
> > fs/smb/client/dir.c | 6 +--
> > fs/smb/client/smb2inode.c | 108 +++++++++++++++++++-------------------
> > 2 files changed, 57 insertions(+), 57 deletions(-)
> >
> > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > index 864b194dbaa0..1822493dd084 100644
> > --- a/fs/smb/client/dir.c
> > +++ b/fs/smb/client/dir.c
> > @@ -627,7 +627,7 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode,
> > goto mknod_out;
> > }
> >
> > - trace_smb3_mknod_enter(xid, tcon->ses->Suid, tcon->tid, full_path);
> > + trace_smb3_mknod_enter(xid, tcon->tid, tcon->ses->Suid, full_path);
> >
> > rc = tcon->ses->server->ops->make_node(xid, inode, direntry, tcon,
> > full_path, mode,
> > @@ -635,9 +635,9 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode,
> >
> > mknod_out:
> > if (rc)
> > - trace_smb3_mknod_err(xid, tcon->ses->Suid, tcon->tid, rc);
> > + trace_smb3_mknod_err(xid, tcon->tid, tcon->ses->Suid, rc);
> > else
> > - trace_smb3_mknod_done(xid, tcon->ses->Suid, tcon->tid);
> > + trace_smb3_mknod_done(xid, tcon->tid, tcon->ses->Suid);
> >
> > free_dentry_path(page);
> > free_xid(xid);
> > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> > index a55f0044d30b..274672755c19 100644
> > --- a/fs/smb/client/smb2inode.c
> > +++ b/fs/smb/client/smb2inode.c
> > @@ -298,8 +298,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_query_info_compound_enter(xid, ses->Suid,
> > - tcon->tid, full_path);
> > + trace_smb3_query_info_compound_enter(xid, tcon->tid,
> > + ses->Suid, full_path);
> > break;
> > case SMB2_OP_POSIX_QUERY_INFO:
> > rqst[num_rqst].rq_iov = &vars->qi_iov;
> > @@ -334,18 +334,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_posix_query_info_compound_enter(xid, ses->Suid,
> > - tcon->tid, full_path);
> > + trace_smb3_posix_query_info_compound_enter(xid, tcon->tid,
> > + ses->Suid, full_path);
> > break;
> > case SMB2_OP_DELETE:
> > - trace_smb3_delete_enter(xid, ses->Suid, tcon->tid, full_path);
> > + trace_smb3_delete_enter(xid, tcon->tid, ses->Suid, full_path);
> > break;
> > case SMB2_OP_MKDIR:
> > /*
> > * Directories are created through parameters in the
> > * SMB2_open() call.
> > */
> > - trace_smb3_mkdir_enter(xid, ses->Suid, tcon->tid, full_path);
> > + trace_smb3_mkdir_enter(xid, tcon->tid, ses->Suid, full_path);
> > break;
> > case SMB2_OP_RMDIR:
> > rqst[num_rqst].rq_iov = &vars->si_iov[0];
> > @@ -363,7 +363,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > smb2_set_next_command(tcon, &rqst[num_rqst]);
> > smb2_set_related(&rqst[num_rqst++]);
> > - trace_smb3_rmdir_enter(xid, ses->Suid, tcon->tid, full_path);
> > + trace_smb3_rmdir_enter(xid, tcon->tid, ses->Suid, full_path);
> > break;
> > case SMB2_OP_SET_EOF:
> > rqst[num_rqst].rq_iov = &vars->si_iov[0];
> > @@ -398,7 +398,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_set_eof_enter(xid, ses->Suid, tcon->tid, full_path);
> > + trace_smb3_set_eof_enter(xid, tcon->tid, ses->Suid, full_path);
> > break;
> > case SMB2_OP_SET_INFO:
> > rqst[num_rqst].rq_iov = &vars->si_iov[0];
> > @@ -429,8 +429,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_set_info_compound_enter(xid, ses->Suid,
> > - tcon->tid, full_path);
> > + trace_smb3_set_info_compound_enter(xid, tcon->tid,
> > + ses->Suid, full_path);
> > break;
> > case SMB2_OP_RENAME:
> > rqst[num_rqst].rq_iov = &vars->si_iov[0];
> > @@ -469,7 +469,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_rename_enter(xid, ses->Suid, tcon->tid, full_path);
> > + trace_smb3_rename_enter(xid, tcon->tid, ses->Suid, full_path);
> > break;
> > case SMB2_OP_HARDLINK:
> > rqst[num_rqst].rq_iov = &vars->si_iov[0];
> > @@ -496,7 +496,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > smb2_set_next_command(tcon, &rqst[num_rqst]);
> > smb2_set_related(&rqst[num_rqst++]);
> > - trace_smb3_hardlink_enter(xid, ses->Suid, tcon->tid, full_path);
> > + trace_smb3_hardlink_enter(xid, tcon->tid, ses->Suid, full_path);
> > break;
> > case SMB2_OP_SET_REPARSE:
> > rqst[num_rqst].rq_iov = vars->io_iov;
> > @@ -523,8 +523,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_set_reparse_compound_enter(xid, ses->Suid,
> > - tcon->tid, full_path);
> > + trace_smb3_set_reparse_compound_enter(xid, tcon->tid,
> > + ses->Suid, full_path);
> > break;
> > case SMB2_OP_GET_REPARSE:
> > rqst[num_rqst].rq_iov = vars->io_iov;
> > @@ -549,8 +549,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > goto finished;
> > }
> > num_rqst++;
> > - trace_smb3_get_reparse_compound_enter(xid, ses->Suid,
> > - tcon->tid, full_path);
> > + trace_smb3_get_reparse_compound_enter(xid, tcon->tid,
> > + ses->Suid, full_path);
> > break;
> > case SMB2_OP_QUERY_WSL_EA:
> > rqst[num_rqst].rq_iov = &vars->ea_iov;
> > @@ -656,11 +656,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > }
> > SMB2_query_info_free(&rqst[num_rqst++]);
> > if (rc)
> > - trace_smb3_query_info_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_query_info_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > else
> > - trace_smb3_query_info_compound_done(xid, ses->Suid,
> > - tcon->tid);
> > + trace_smb3_query_info_compound_done(xid, tcon->tid,
> > + ses->Suid);
> > break;
> > case SMB2_OP_POSIX_QUERY_INFO:
> > idata = in_iov[i].iov_base;
> > @@ -683,15 +683,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> >
> > SMB2_query_info_free(&rqst[num_rqst++]);
> > if (rc)
> > - trace_smb3_posix_query_info_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_posix_query_info_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > else
> > - trace_smb3_posix_query_info_compound_done(xid, ses->Suid,
> > - tcon->tid);
> > + trace_smb3_posix_query_info_compound_done(xid, tcon->tid,
> > + ses->Suid);
> > break;
> > case SMB2_OP_DELETE:
> > if (rc)
> > - trace_smb3_delete_err(xid, ses->Suid, tcon->tid, rc);
> > + trace_smb3_delete_err(xid, tcon->tid, ses->Suid, rc);
> > else {
> > /*
> > * If dentry (hence, inode) is NULL, lease break is going to
> > @@ -699,59 +699,59 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > */
> > if (inode)
> > cifs_mark_open_handles_for_deleted_file(inode, full_path);
> > - trace_smb3_delete_done(xid, ses->Suid, tcon->tid);
> > + trace_smb3_delete_done(xid, tcon->tid, ses->Suid);
> > }
> > break;
> > case SMB2_OP_MKDIR:
> > if (rc)
> > - trace_smb3_mkdir_err(xid, ses->Suid, tcon->tid, rc);
> > + trace_smb3_mkdir_err(xid, tcon->tid, ses->Suid, rc);
> > else
> > - trace_smb3_mkdir_done(xid, ses->Suid, tcon->tid);
> > + trace_smb3_mkdir_done(xid, tcon->tid, ses->Suid);
> > break;
> > case SMB2_OP_HARDLINK:
> > if (rc)
> > - trace_smb3_hardlink_err(xid, ses->Suid, tcon->tid, rc);
> > + trace_smb3_hardlink_err(xid, tcon->tid, ses->Suid, rc);
> > else
> > - trace_smb3_hardlink_done(xid, ses->Suid, tcon->tid);
> > + trace_smb3_hardlink_done(xid, tcon->tid, ses->Suid);
> > SMB2_set_info_free(&rqst[num_rqst++]);
> > break;
> > case SMB2_OP_RENAME:
> > if (rc)
> > - trace_smb3_rename_err(xid, ses->Suid, tcon->tid, rc);
> > + trace_smb3_rename_err(xid, tcon->tid, ses->Suid, rc);
> > else
> > - trace_smb3_rename_done(xid, ses->Suid, tcon->tid);
> > + trace_smb3_rename_done(xid, tcon->tid, ses->Suid);
> > SMB2_set_info_free(&rqst[num_rqst++]);
> > break;
> > case SMB2_OP_RMDIR:
> > if (rc)
> > - trace_smb3_rmdir_err(xid, ses->Suid, tcon->tid, rc);
> > + trace_smb3_rmdir_err(xid, tcon->tid, ses->Suid, rc);
> > else
> > - trace_smb3_rmdir_done(xid, ses->Suid, tcon->tid);
> > + trace_smb3_rmdir_done(xid, tcon->tid, ses->Suid);
> > SMB2_set_info_free(&rqst[num_rqst++]);
> > break;
> > case SMB2_OP_SET_EOF:
> > if (rc)
> > - trace_smb3_set_eof_err(xid, ses->Suid, tcon->tid, rc);
> > + trace_smb3_set_eof_err(xid, tcon->tid, ses->Suid, rc);
> > else
> > - trace_smb3_set_eof_done(xid, ses->Suid, tcon->tid);
> > + trace_smb3_set_eof_done(xid, tcon->tid, ses->Suid);
> > SMB2_set_info_free(&rqst[num_rqst++]);
> > break;
> > case SMB2_OP_SET_INFO:
> > if (rc)
> > - trace_smb3_set_info_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_set_info_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > else
> > - trace_smb3_set_info_compound_done(xid, ses->Suid,
> > - tcon->tid);
> > + trace_smb3_set_info_compound_done(xid, tcon->tid,
> > + ses->Suid);
> > SMB2_set_info_free(&rqst[num_rqst++]);
> > break;
> > case SMB2_OP_SET_REPARSE:
> > if (rc) {
> > - trace_smb3_set_reparse_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_set_reparse_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > } else {
> > - trace_smb3_set_reparse_compound_done(xid, ses->Suid,
> > - tcon->tid);
> > + trace_smb3_set_reparse_compound_done(xid, tcon->tid,
> > + ses->Suid);
> > }
> > SMB2_ioctl_free(&rqst[num_rqst++]);
> > break;
> > @@ -764,18 +764,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > rbuf = reparse_buf_ptr(iov);
> > if (IS_ERR(rbuf)) {
> > rc = PTR_ERR(rbuf);
> > - trace_smb3_set_reparse_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_get_reparse_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > } else {
> > idata->reparse.tag = le32_to_cpu(rbuf->ReparseTag);
> > - trace_smb3_set_reparse_compound_done(xid, ses->Suid,
> > - tcon->tid);
> > + trace_smb3_get_reparse_compound_done(xid, tcon->tid,
> > + ses->Suid);
> > }
> > memset(iov, 0, sizeof(*iov));
> > resp_buftype[i + 1] = CIFS_NO_BUFFER;
> > } else {
> > - trace_smb3_set_reparse_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_get_reparse_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > }
> > SMB2_ioctl_free(&rqst[num_rqst++]);
> > break;
> > @@ -792,11 +792,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > }
> > }
> > if (!rc) {
> > - trace_smb3_query_wsl_ea_compound_done(xid, ses->Suid,
> > - tcon->tid);
> > + trace_smb3_query_wsl_ea_compound_done(xid, tcon->tid,
> > + ses->Suid);
> > } else {
> > - trace_smb3_query_wsl_ea_compound_err(xid, ses->Suid,
> > - tcon->tid, rc);
> > + trace_smb3_query_wsl_ea_compound_err(xid, tcon->tid,
> > + ses->Suid, rc);
> > }
> > SMB2_query_info_free(&rqst[num_rqst++]);
> > break;
> > --
> > 2.48.1
> >
> >
>
>
> --
> Thanks,
>
> Steve
On zondag 19 januari 2025 at 08:16:29 Steve French wrote: > Here is a minor patch to add the missing tracepoint (see attached). > Let me know if any thoughts, or other obviousb missing tracepoints Hi Steve, Thank you for your review. I noticed the missing trace_smb3_query_wsl_ea_compound_enter too but I was not sure if it was left out for a reason. I'm not aware of other missing tracepoints. I saw that both your patch and my patch are in your for-next tree now. Does this mean everything is OK and there is no need for a v2? > > smb3: add missing tracepoint for querying wsl EAs > > We had tracepoints for the return code for querying WSL EAs > (trace_smb3_query_wsl_ea_compound_err and > trace_smb3_query_wsl_ea_compound_done) but were missing one for > trace_smb3_query_wsl_ea_compound_enter. > > Fixes: ea41367b2a60 ("smb: client: introduce SMB2_OP_QUERY_WSL_EA") > > > On Sat, Jan 18, 2025 at 10:38 PM Steve French <smfrench@gmail.com> wrote: > > > > Good catch. > > > > Looking at your patch, I noticed one trace point was missing from the > > original patch: > > > > trace_smb3_query_wsl_ea_compound_enter > > > > commit ea41367b2a602f602ea6594fc4a310520dcc64f4 > > Author: Paulo Alcantara <pc@manguebit.com> > > Date: Sun Jan 28 01:12:01 2024 -0300 > > > > smb: client: introduce SMB2_OP_QUERY_WSL_EA > > > > On Sat, Jan 18, 2025 at 2:04 PM Ruben Devos <devosruben6@gmail.com> wrote: > > > > > > The tracepoints based on smb3_inf_compound_*_class have tcon id and > > > session id swapped around. This results in incorrect output in > > > `trace-cmd report`. > > > > > > Fix the order of arguments to resolve this issue. The trace-cmd output > > > below shows the before and after of the smb3_delete_enter and > > > smb3_delete_done events as an example. The smb3_cmd_* events show the > > > correct session and tcon id for reference. > > > > > > Also fix tracepoint set -> get in the SMB2_OP_GET_REPARSE case. > > > > > > BEFORE: > > > rm-2211 [001] ..... 1839.550888: smb3_delete_enter: xid=281 sid=0x5 tid=0x3d path=\hello2.txt > > > rm-2211 [001] ..... 1839.550894: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61 > > > rm-2211 [001] ..... 1839.550896: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62 > > > rm-2211 [001] ..... 1839.552091: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61 > > > rm-2211 [001] ..... 1839.552093: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62 > > > rm-2211 [001] ..... 1839.552103: smb3_delete_done: xid=281 sid=0x5 tid=0x3d > > > > > > AFTER: > > > rm-2501 [001] ..... 3237.656110: smb3_delete_enter: xid=88 sid=0x1ac0000000041 tid=0x5 path=\hello2.txt > > > rm-2501 [001] ..... 3237.656122: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84 > > > rm-2501 [001] ..... 3237.656123: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85 > > > rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84 > > > rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85 > > > rm-2501 [001] ..... 3237.657922: smb3_delete_done: xid=88 sid=0x1ac0000000041 tid=0x5 > > > > > > Signed-off-by: Ruben Devos <devosruben6@gmail.com> > > > --- > > > fs/smb/client/dir.c | 6 +-- > > > fs/smb/client/smb2inode.c | 108 +++++++++++++++++++------------------- > > > 2 files changed, 57 insertions(+), 57 deletions(-) > > > > > > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c > > > index 864b194dbaa0..1822493dd084 100644 > > > --- a/fs/smb/client/dir.c > > > +++ b/fs/smb/client/dir.c > > > @@ -627,7 +627,7 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode, > > > goto mknod_out; > > > } > > > > > > - trace_smb3_mknod_enter(xid, tcon->ses->Suid, tcon->tid, full_path); > > > + trace_smb3_mknod_enter(xid, tcon->tid, tcon->ses->Suid, full_path); > > > > > > rc = tcon->ses->server->ops->make_node(xid, inode, direntry, tcon, > > > full_path, mode, > > > @@ -635,9 +635,9 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode, > > > > > > mknod_out: > > > if (rc) > > > - trace_smb3_mknod_err(xid, tcon->ses->Suid, tcon->tid, rc); > > > + trace_smb3_mknod_err(xid, tcon->tid, tcon->ses->Suid, rc); > > > else > > > - trace_smb3_mknod_done(xid, tcon->ses->Suid, tcon->tid); > > > + trace_smb3_mknod_done(xid, tcon->tid, tcon->ses->Suid); > > > > > > free_dentry_path(page); > > > free_xid(xid); > > > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c > > > index a55f0044d30b..274672755c19 100644 > > > --- a/fs/smb/client/smb2inode.c > > > +++ b/fs/smb/client/smb2inode.c > > > @@ -298,8 +298,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_query_info_compound_enter(xid, ses->Suid, > > > - tcon->tid, full_path); > > > + trace_smb3_query_info_compound_enter(xid, tcon->tid, > > > + ses->Suid, full_path); > > > break; > > > case SMB2_OP_POSIX_QUERY_INFO: > > > rqst[num_rqst].rq_iov = &vars->qi_iov; > > > @@ -334,18 +334,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_posix_query_info_compound_enter(xid, ses->Suid, > > > - tcon->tid, full_path); > > > + trace_smb3_posix_query_info_compound_enter(xid, tcon->tid, > > > + ses->Suid, full_path); > > > break; > > > case SMB2_OP_DELETE: > > > - trace_smb3_delete_enter(xid, ses->Suid, tcon->tid, full_path); > > > + trace_smb3_delete_enter(xid, tcon->tid, ses->Suid, full_path); > > > break; > > > case SMB2_OP_MKDIR: > > > /* > > > * Directories are created through parameters in the > > > * SMB2_open() call. > > > */ > > > - trace_smb3_mkdir_enter(xid, ses->Suid, tcon->tid, full_path); > > > + trace_smb3_mkdir_enter(xid, tcon->tid, ses->Suid, full_path); > > > break; > > > case SMB2_OP_RMDIR: > > > rqst[num_rqst].rq_iov = &vars->si_iov[0]; > > > @@ -363,7 +363,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > smb2_set_next_command(tcon, &rqst[num_rqst]); > > > smb2_set_related(&rqst[num_rqst++]); > > > - trace_smb3_rmdir_enter(xid, ses->Suid, tcon->tid, full_path); > > > + trace_smb3_rmdir_enter(xid, tcon->tid, ses->Suid, full_path); > > > break; > > > case SMB2_OP_SET_EOF: > > > rqst[num_rqst].rq_iov = &vars->si_iov[0]; > > > @@ -398,7 +398,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_set_eof_enter(xid, ses->Suid, tcon->tid, full_path); > > > + trace_smb3_set_eof_enter(xid, tcon->tid, ses->Suid, full_path); > > > break; > > > case SMB2_OP_SET_INFO: > > > rqst[num_rqst].rq_iov = &vars->si_iov[0]; > > > @@ -429,8 +429,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_set_info_compound_enter(xid, ses->Suid, > > > - tcon->tid, full_path); > > > + trace_smb3_set_info_compound_enter(xid, tcon->tid, > > > + ses->Suid, full_path); > > > break; > > > case SMB2_OP_RENAME: > > > rqst[num_rqst].rq_iov = &vars->si_iov[0]; > > > @@ -469,7 +469,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_rename_enter(xid, ses->Suid, tcon->tid, full_path); > > > + trace_smb3_rename_enter(xid, tcon->tid, ses->Suid, full_path); > > > break; > > > case SMB2_OP_HARDLINK: > > > rqst[num_rqst].rq_iov = &vars->si_iov[0]; > > > @@ -496,7 +496,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > smb2_set_next_command(tcon, &rqst[num_rqst]); > > > smb2_set_related(&rqst[num_rqst++]); > > > - trace_smb3_hardlink_enter(xid, ses->Suid, tcon->tid, full_path); > > > + trace_smb3_hardlink_enter(xid, tcon->tid, ses->Suid, full_path); > > > break; > > > case SMB2_OP_SET_REPARSE: > > > rqst[num_rqst].rq_iov = vars->io_iov; > > > @@ -523,8 +523,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_set_reparse_compound_enter(xid, ses->Suid, > > > - tcon->tid, full_path); > > > + trace_smb3_set_reparse_compound_enter(xid, tcon->tid, > > > + ses->Suid, full_path); > > > break; > > > case SMB2_OP_GET_REPARSE: > > > rqst[num_rqst].rq_iov = vars->io_iov; > > > @@ -549,8 +549,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > goto finished; > > > } > > > num_rqst++; > > > - trace_smb3_get_reparse_compound_enter(xid, ses->Suid, > > > - tcon->tid, full_path); > > > + trace_smb3_get_reparse_compound_enter(xid, tcon->tid, > > > + ses->Suid, full_path); > > > break; > > > case SMB2_OP_QUERY_WSL_EA: > > > rqst[num_rqst].rq_iov = &vars->ea_iov; > > > @@ -656,11 +656,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > } > > > SMB2_query_info_free(&rqst[num_rqst++]); > > > if (rc) > > > - trace_smb3_query_info_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_query_info_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > else > > > - trace_smb3_query_info_compound_done(xid, ses->Suid, > > > - tcon->tid); > > > + trace_smb3_query_info_compound_done(xid, tcon->tid, > > > + ses->Suid); > > > break; > > > case SMB2_OP_POSIX_QUERY_INFO: > > > idata = in_iov[i].iov_base; > > > @@ -683,15 +683,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > > > > SMB2_query_info_free(&rqst[num_rqst++]); > > > if (rc) > > > - trace_smb3_posix_query_info_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_posix_query_info_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > else > > > - trace_smb3_posix_query_info_compound_done(xid, ses->Suid, > > > - tcon->tid); > > > + trace_smb3_posix_query_info_compound_done(xid, tcon->tid, > > > + ses->Suid); > > > break; > > > case SMB2_OP_DELETE: > > > if (rc) > > > - trace_smb3_delete_err(xid, ses->Suid, tcon->tid, rc); > > > + trace_smb3_delete_err(xid, tcon->tid, ses->Suid, rc); > > > else { > > > /* > > > * If dentry (hence, inode) is NULL, lease break is going to > > > @@ -699,59 +699,59 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > */ > > > if (inode) > > > cifs_mark_open_handles_for_deleted_file(inode, full_path); > > > - trace_smb3_delete_done(xid, ses->Suid, tcon->tid); > > > + trace_smb3_delete_done(xid, tcon->tid, ses->Suid); > > > } > > > break; > > > case SMB2_OP_MKDIR: > > > if (rc) > > > - trace_smb3_mkdir_err(xid, ses->Suid, tcon->tid, rc); > > > + trace_smb3_mkdir_err(xid, tcon->tid, ses->Suid, rc); > > > else > > > - trace_smb3_mkdir_done(xid, ses->Suid, tcon->tid); > > > + trace_smb3_mkdir_done(xid, tcon->tid, ses->Suid); > > > break; > > > case SMB2_OP_HARDLINK: > > > if (rc) > > > - trace_smb3_hardlink_err(xid, ses->Suid, tcon->tid, rc); > > > + trace_smb3_hardlink_err(xid, tcon->tid, ses->Suid, rc); > > > else > > > - trace_smb3_hardlink_done(xid, ses->Suid, tcon->tid); > > > + trace_smb3_hardlink_done(xid, tcon->tid, ses->Suid); > > > SMB2_set_info_free(&rqst[num_rqst++]); > > > break; > > > case SMB2_OP_RENAME: > > > if (rc) > > > - trace_smb3_rename_err(xid, ses->Suid, tcon->tid, rc); > > > + trace_smb3_rename_err(xid, tcon->tid, ses->Suid, rc); > > > else > > > - trace_smb3_rename_done(xid, ses->Suid, tcon->tid); > > > + trace_smb3_rename_done(xid, tcon->tid, ses->Suid); > > > SMB2_set_info_free(&rqst[num_rqst++]); > > > break; > > > case SMB2_OP_RMDIR: > > > if (rc) > > > - trace_smb3_rmdir_err(xid, ses->Suid, tcon->tid, rc); > > > + trace_smb3_rmdir_err(xid, tcon->tid, ses->Suid, rc); > > > else > > > - trace_smb3_rmdir_done(xid, ses->Suid, tcon->tid); > > > + trace_smb3_rmdir_done(xid, tcon->tid, ses->Suid); > > > SMB2_set_info_free(&rqst[num_rqst++]); > > > break; > > > case SMB2_OP_SET_EOF: > > > if (rc) > > > - trace_smb3_set_eof_err(xid, ses->Suid, tcon->tid, rc); > > > + trace_smb3_set_eof_err(xid, tcon->tid, ses->Suid, rc); > > > else > > > - trace_smb3_set_eof_done(xid, ses->Suid, tcon->tid); > > > + trace_smb3_set_eof_done(xid, tcon->tid, ses->Suid); > > > SMB2_set_info_free(&rqst[num_rqst++]); > > > break; > > > case SMB2_OP_SET_INFO: > > > if (rc) > > > - trace_smb3_set_info_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_set_info_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > else > > > - trace_smb3_set_info_compound_done(xid, ses->Suid, > > > - tcon->tid); > > > + trace_smb3_set_info_compound_done(xid, tcon->tid, > > > + ses->Suid); > > > SMB2_set_info_free(&rqst[num_rqst++]); > > > break; > > > case SMB2_OP_SET_REPARSE: > > > if (rc) { > > > - trace_smb3_set_reparse_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_set_reparse_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > } else { > > > - trace_smb3_set_reparse_compound_done(xid, ses->Suid, > > > - tcon->tid); > > > + trace_smb3_set_reparse_compound_done(xid, tcon->tid, > > > + ses->Suid); > > > } > > > SMB2_ioctl_free(&rqst[num_rqst++]); > > > break; > > > @@ -764,18 +764,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > rbuf = reparse_buf_ptr(iov); > > > if (IS_ERR(rbuf)) { > > > rc = PTR_ERR(rbuf); > > > - trace_smb3_set_reparse_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_get_reparse_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > } else { > > > idata->reparse.tag = le32_to_cpu(rbuf->ReparseTag); > > > - trace_smb3_set_reparse_compound_done(xid, ses->Suid, > > > - tcon->tid); > > > + trace_smb3_get_reparse_compound_done(xid, tcon->tid, > > > + ses->Suid); > > > } > > > memset(iov, 0, sizeof(*iov)); > > > resp_buftype[i + 1] = CIFS_NO_BUFFER; > > > } else { > > > - trace_smb3_set_reparse_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_get_reparse_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > } > > > SMB2_ioctl_free(&rqst[num_rqst++]); > > > break; > > > @@ -792,11 +792,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > > } > > > } > > > if (!rc) { > > > - trace_smb3_query_wsl_ea_compound_done(xid, ses->Suid, > > > - tcon->tid); > > > + trace_smb3_query_wsl_ea_compound_done(xid, tcon->tid, > > > + ses->Suid); > > > } else { > > > - trace_smb3_query_wsl_ea_compound_err(xid, ses->Suid, > > > - tcon->tid, rc); > > > + trace_smb3_query_wsl_ea_compound_err(xid, tcon->tid, > > > + ses->Suid, rc); > > > } > > > SMB2_query_info_free(&rqst[num_rqst++]); > > > break; > > > -- > > > 2.48.1 > > > > > > > > > > > > -- > > Thanks, > > > > Steve > > > >
On Sun, Jan 19, 2025 at 5:11 AM Ruben Devos <devosruben6@gmail.com> wrote: > > On zondag 19 januari 2025 at 08:16:29 Steve French wrote: > > Here is a minor patch to add the missing tracepoint (see attached). > > Let me know if any thoughts, or other obviousb missing tracepoints > Hi Steve, > > Thank you for your review. > I noticed the missing trace_smb3_query_wsl_ea_compound_enter too but I > was not sure if it was left out for a reason. > I'm not aware of other missing tracepoints. > > I saw that both your patch and my patch are in your for-next tree now. > Does this mean everything is OK and there is no need for a v2? Yes, it is in for-next. I didn't see any obvious problems, but will welcome any additional testing and review.since a lot of patches to go through for the upcoming merge window
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c index 864b194dbaa0..1822493dd084 100644 --- a/fs/smb/client/dir.c +++ b/fs/smb/client/dir.c @@ -627,7 +627,7 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode, goto mknod_out; } - trace_smb3_mknod_enter(xid, tcon->ses->Suid, tcon->tid, full_path); + trace_smb3_mknod_enter(xid, tcon->tid, tcon->ses->Suid, full_path); rc = tcon->ses->server->ops->make_node(xid, inode, direntry, tcon, full_path, mode, @@ -635,9 +635,9 @@ int cifs_mknod(struct mnt_idmap *idmap, struct inode *inode, mknod_out: if (rc) - trace_smb3_mknod_err(xid, tcon->ses->Suid, tcon->tid, rc); + trace_smb3_mknod_err(xid, tcon->tid, tcon->ses->Suid, rc); else - trace_smb3_mknod_done(xid, tcon->ses->Suid, tcon->tid); + trace_smb3_mknod_done(xid, tcon->tid, tcon->ses->Suid); free_dentry_path(page); free_xid(xid); diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index a55f0044d30b..274672755c19 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -298,8 +298,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_query_info_compound_enter(xid, ses->Suid, - tcon->tid, full_path); + trace_smb3_query_info_compound_enter(xid, tcon->tid, + ses->Suid, full_path); break; case SMB2_OP_POSIX_QUERY_INFO: rqst[num_rqst].rq_iov = &vars->qi_iov; @@ -334,18 +334,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_posix_query_info_compound_enter(xid, ses->Suid, - tcon->tid, full_path); + trace_smb3_posix_query_info_compound_enter(xid, tcon->tid, + ses->Suid, full_path); break; case SMB2_OP_DELETE: - trace_smb3_delete_enter(xid, ses->Suid, tcon->tid, full_path); + trace_smb3_delete_enter(xid, tcon->tid, ses->Suid, full_path); break; case SMB2_OP_MKDIR: /* * Directories are created through parameters in the * SMB2_open() call. */ - trace_smb3_mkdir_enter(xid, ses->Suid, tcon->tid, full_path); + trace_smb3_mkdir_enter(xid, tcon->tid, ses->Suid, full_path); break; case SMB2_OP_RMDIR: rqst[num_rqst].rq_iov = &vars->si_iov[0]; @@ -363,7 +363,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; smb2_set_next_command(tcon, &rqst[num_rqst]); smb2_set_related(&rqst[num_rqst++]); - trace_smb3_rmdir_enter(xid, ses->Suid, tcon->tid, full_path); + trace_smb3_rmdir_enter(xid, tcon->tid, ses->Suid, full_path); break; case SMB2_OP_SET_EOF: rqst[num_rqst].rq_iov = &vars->si_iov[0]; @@ -398,7 +398,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_set_eof_enter(xid, ses->Suid, tcon->tid, full_path); + trace_smb3_set_eof_enter(xid, tcon->tid, ses->Suid, full_path); break; case SMB2_OP_SET_INFO: rqst[num_rqst].rq_iov = &vars->si_iov[0]; @@ -429,8 +429,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_set_info_compound_enter(xid, ses->Suid, - tcon->tid, full_path); + trace_smb3_set_info_compound_enter(xid, tcon->tid, + ses->Suid, full_path); break; case SMB2_OP_RENAME: rqst[num_rqst].rq_iov = &vars->si_iov[0]; @@ -469,7 +469,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_rename_enter(xid, ses->Suid, tcon->tid, full_path); + trace_smb3_rename_enter(xid, tcon->tid, ses->Suid, full_path); break; case SMB2_OP_HARDLINK: rqst[num_rqst].rq_iov = &vars->si_iov[0]; @@ -496,7 +496,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; smb2_set_next_command(tcon, &rqst[num_rqst]); smb2_set_related(&rqst[num_rqst++]); - trace_smb3_hardlink_enter(xid, ses->Suid, tcon->tid, full_path); + trace_smb3_hardlink_enter(xid, tcon->tid, ses->Suid, full_path); break; case SMB2_OP_SET_REPARSE: rqst[num_rqst].rq_iov = vars->io_iov; @@ -523,8 +523,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_set_reparse_compound_enter(xid, ses->Suid, - tcon->tid, full_path); + trace_smb3_set_reparse_compound_enter(xid, tcon->tid, + ses->Suid, full_path); break; case SMB2_OP_GET_REPARSE: rqst[num_rqst].rq_iov = vars->io_iov; @@ -549,8 +549,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } num_rqst++; - trace_smb3_get_reparse_compound_enter(xid, ses->Suid, - tcon->tid, full_path); + trace_smb3_get_reparse_compound_enter(xid, tcon->tid, + ses->Suid, full_path); break; case SMB2_OP_QUERY_WSL_EA: rqst[num_rqst].rq_iov = &vars->ea_iov; @@ -656,11 +656,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, } SMB2_query_info_free(&rqst[num_rqst++]); if (rc) - trace_smb3_query_info_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_query_info_compound_err(xid, tcon->tid, + ses->Suid, rc); else - trace_smb3_query_info_compound_done(xid, ses->Suid, - tcon->tid); + trace_smb3_query_info_compound_done(xid, tcon->tid, + ses->Suid); break; case SMB2_OP_POSIX_QUERY_INFO: idata = in_iov[i].iov_base; @@ -683,15 +683,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, SMB2_query_info_free(&rqst[num_rqst++]); if (rc) - trace_smb3_posix_query_info_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_posix_query_info_compound_err(xid, tcon->tid, + ses->Suid, rc); else - trace_smb3_posix_query_info_compound_done(xid, ses->Suid, - tcon->tid); + trace_smb3_posix_query_info_compound_done(xid, tcon->tid, + ses->Suid); break; case SMB2_OP_DELETE: if (rc) - trace_smb3_delete_err(xid, ses->Suid, tcon->tid, rc); + trace_smb3_delete_err(xid, tcon->tid, ses->Suid, rc); else { /* * If dentry (hence, inode) is NULL, lease break is going to @@ -699,59 +699,59 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, */ if (inode) cifs_mark_open_handles_for_deleted_file(inode, full_path); - trace_smb3_delete_done(xid, ses->Suid, tcon->tid); + trace_smb3_delete_done(xid, tcon->tid, ses->Suid); } break; case SMB2_OP_MKDIR: if (rc) - trace_smb3_mkdir_err(xid, ses->Suid, tcon->tid, rc); + trace_smb3_mkdir_err(xid, tcon->tid, ses->Suid, rc); else - trace_smb3_mkdir_done(xid, ses->Suid, tcon->tid); + trace_smb3_mkdir_done(xid, tcon->tid, ses->Suid); break; case SMB2_OP_HARDLINK: if (rc) - trace_smb3_hardlink_err(xid, ses->Suid, tcon->tid, rc); + trace_smb3_hardlink_err(xid, tcon->tid, ses->Suid, rc); else - trace_smb3_hardlink_done(xid, ses->Suid, tcon->tid); + trace_smb3_hardlink_done(xid, tcon->tid, ses->Suid); SMB2_set_info_free(&rqst[num_rqst++]); break; case SMB2_OP_RENAME: if (rc) - trace_smb3_rename_err(xid, ses->Suid, tcon->tid, rc); + trace_smb3_rename_err(xid, tcon->tid, ses->Suid, rc); else - trace_smb3_rename_done(xid, ses->Suid, tcon->tid); + trace_smb3_rename_done(xid, tcon->tid, ses->Suid); SMB2_set_info_free(&rqst[num_rqst++]); break; case SMB2_OP_RMDIR: if (rc) - trace_smb3_rmdir_err(xid, ses->Suid, tcon->tid, rc); + trace_smb3_rmdir_err(xid, tcon->tid, ses->Suid, rc); else - trace_smb3_rmdir_done(xid, ses->Suid, tcon->tid); + trace_smb3_rmdir_done(xid, tcon->tid, ses->Suid); SMB2_set_info_free(&rqst[num_rqst++]); break; case SMB2_OP_SET_EOF: if (rc) - trace_smb3_set_eof_err(xid, ses->Suid, tcon->tid, rc); + trace_smb3_set_eof_err(xid, tcon->tid, ses->Suid, rc); else - trace_smb3_set_eof_done(xid, ses->Suid, tcon->tid); + trace_smb3_set_eof_done(xid, tcon->tid, ses->Suid); SMB2_set_info_free(&rqst[num_rqst++]); break; case SMB2_OP_SET_INFO: if (rc) - trace_smb3_set_info_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_set_info_compound_err(xid, tcon->tid, + ses->Suid, rc); else - trace_smb3_set_info_compound_done(xid, ses->Suid, - tcon->tid); + trace_smb3_set_info_compound_done(xid, tcon->tid, + ses->Suid); SMB2_set_info_free(&rqst[num_rqst++]); break; case SMB2_OP_SET_REPARSE: if (rc) { - trace_smb3_set_reparse_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_set_reparse_compound_err(xid, tcon->tid, + ses->Suid, rc); } else { - trace_smb3_set_reparse_compound_done(xid, ses->Suid, - tcon->tid); + trace_smb3_set_reparse_compound_done(xid, tcon->tid, + ses->Suid); } SMB2_ioctl_free(&rqst[num_rqst++]); break; @@ -764,18 +764,18 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, rbuf = reparse_buf_ptr(iov); if (IS_ERR(rbuf)) { rc = PTR_ERR(rbuf); - trace_smb3_set_reparse_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_get_reparse_compound_err(xid, tcon->tid, + ses->Suid, rc); } else { idata->reparse.tag = le32_to_cpu(rbuf->ReparseTag); - trace_smb3_set_reparse_compound_done(xid, ses->Suid, - tcon->tid); + trace_smb3_get_reparse_compound_done(xid, tcon->tid, + ses->Suid); } memset(iov, 0, sizeof(*iov)); resp_buftype[i + 1] = CIFS_NO_BUFFER; } else { - trace_smb3_set_reparse_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_get_reparse_compound_err(xid, tcon->tid, + ses->Suid, rc); } SMB2_ioctl_free(&rqst[num_rqst++]); break; @@ -792,11 +792,11 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, } } if (!rc) { - trace_smb3_query_wsl_ea_compound_done(xid, ses->Suid, - tcon->tid); + trace_smb3_query_wsl_ea_compound_done(xid, tcon->tid, + ses->Suid); } else { - trace_smb3_query_wsl_ea_compound_err(xid, ses->Suid, - tcon->tid, rc); + trace_smb3_query_wsl_ea_compound_err(xid, tcon->tid, + ses->Suid, rc); } SMB2_query_info_free(&rqst[num_rqst++]); break;
The tracepoints based on smb3_inf_compound_*_class have tcon id and session id swapped around. This results in incorrect output in `trace-cmd report`. Fix the order of arguments to resolve this issue. The trace-cmd output below shows the before and after of the smb3_delete_enter and smb3_delete_done events as an example. The smb3_cmd_* events show the correct session and tcon id for reference. Also fix tracepoint set -> get in the SMB2_OP_GET_REPARSE case. BEFORE: rm-2211 [001] ..... 1839.550888: smb3_delete_enter: xid=281 sid=0x5 tid=0x3d path=\hello2.txt rm-2211 [001] ..... 1839.550894: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61 rm-2211 [001] ..... 1839.550896: smb3_cmd_enter: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62 rm-2211 [001] ..... 1839.552091: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=5 mid=61 rm-2211 [001] ..... 1839.552093: smb3_cmd_done: sid=0x1ac000000003d tid=0x5 cmd=6 mid=62 rm-2211 [001] ..... 1839.552103: smb3_delete_done: xid=281 sid=0x5 tid=0x3d AFTER: rm-2501 [001] ..... 3237.656110: smb3_delete_enter: xid=88 sid=0x1ac0000000041 tid=0x5 path=\hello2.txt rm-2501 [001] ..... 3237.656122: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84 rm-2501 [001] ..... 3237.656123: smb3_cmd_enter: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85 rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=5 mid=84 rm-2501 [001] ..... 3237.657909: smb3_cmd_done: sid=0x1ac0000000041 tid=0x5 cmd=6 mid=85 rm-2501 [001] ..... 3237.657922: smb3_delete_done: xid=88 sid=0x1ac0000000041 tid=0x5 Signed-off-by: Ruben Devos <devosruben6@gmail.com> --- fs/smb/client/dir.c | 6 +-- fs/smb/client/smb2inode.c | 108 +++++++++++++++++++------------------- 2 files changed, 57 insertions(+), 57 deletions(-)