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@xxxxxxxxx> 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@xxxxxxxxxxxxx>
> > 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@xxxxxxxxx> 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@xxxxxxxxx>
> > > ---
> > > 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
>
>
>
>
