From: Olga Kornievskaia <aglo@umich.edu>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-rdma <linux-rdma@vger.kernel.org>,
linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 16/19] NFS: Fix show_nfs_errors macros again
Date: Tue, 11 Jun 2019 15:33:12 -0400 [thread overview]
Message-ID: <CAN-5tyHx-r6c6RgHk2ocv2CxTgw_8Ebie_fUUSxzaVKotmX1zw@mail.gmail.com> (raw)
In-Reply-To: <20190611150923.2877.6862.stgit@manet.1015granger.net>
On Tue, Jun 11, 2019 at 11:09 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> I noticed that NFS status values stopped working again.
>
> trace_print_symbols_seq() takes an unsigned long. Passing a negative
> errno or negative NFSERR value just confuses it, and since we're
> using C macros here and not static inline functions, all bets are
> off due to implicit type casting.
>
> Straight-line the calling conventions so that error codes are stored
> in the trace record as positive values in an unsigned long field.
>
> It's often the case that an error value that is positive is a byte
> count but when it's negative, it's an error (e.g. nfs4_write). Fix
> those cases so that the value that is eventually stored in the
> error field is a positive NFS status or errno, or zero.
>
Hi Chuck,
To clarify, so on error case, we no longer going be seeing a negative
value so error=-5 (EIO) would be error=5 (EIO)? I have always relied
on searching for "error=-" thru the trace_pipe log for errors. Do we
really need to change that?
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> fs/nfs/nfs4trace.h | 120 ++++++++++++++++++++++++++--------------------------
> 1 file changed, 60 insertions(+), 60 deletions(-)
>
> diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
> index 6beb1f2..9a01731 100644
> --- a/fs/nfs/nfs4trace.h
> +++ b/fs/nfs/nfs4trace.h
> @@ -156,7 +156,7 @@
> TRACE_DEFINE_ENUM(NFS4ERR_XDEV);
>
> #define show_nfsv4_errors(error) \
> - __print_symbolic(-(error), \
> + __print_symbolic(error, \
> { NFS4_OK, "OK" }, \
> /* Mapped by nfs4_stat_to_errno() */ \
> { EPERM, "EPERM" }, \
> @@ -348,7 +348,7 @@
>
> TP_STRUCT__entry(
> __string(dstaddr, clp->cl_hostname)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -357,7 +357,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) dstaddr=%s",
> + "error=%lu (%s) dstaddr=%s",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> __get_str(dstaddr)
> @@ -420,7 +420,7 @@
> __field(unsigned int, highest_slotid)
> __field(unsigned int, target_highest_slotid)
> __field(unsigned int, status_flags)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -435,7 +435,7 @@
> __entry->error = res->sr_status;
> ),
> TP_printk(
> - "error=%d (%s) session=0x%08x slot_nr=%u seq_nr=%u "
> + "error=%lu (%s) session=0x%08x slot_nr=%u seq_nr=%u "
> "highest_slotid=%u target_highest_slotid=%u "
> "status_flags=%u (%s)",
> __entry->error,
> @@ -467,7 +467,7 @@
> __field(unsigned int, seq_nr)
> __field(unsigned int, highest_slotid)
> __field(unsigned int, cachethis)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -476,11 +476,11 @@
> __entry->seq_nr = args->csa_sequenceid;
> __entry->highest_slotid = args->csa_highestslotid;
> __entry->cachethis = args->csa_cachethis;
> - __entry->error = -be32_to_cpu(status);
> + __entry->error = be32_to_cpu(status);
> ),
>
> TP_printk(
> - "error=%d (%s) session=0x%08x slot_nr=%u seq_nr=%u "
> + "error=%lu (%s) session=0x%08x slot_nr=%u seq_nr=%u "
> "highest_slotid=%u",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -504,7 +504,7 @@
> __field(unsigned int, seq_nr)
> __field(unsigned int, highest_slotid)
> __field(unsigned int, cachethis)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -513,11 +513,11 @@
> __entry->seq_nr = args->csa_sequenceid;
> __entry->highest_slotid = args->csa_highestslotid;
> __entry->cachethis = args->csa_cachethis;
> - __entry->error = -be32_to_cpu(status);
> + __entry->error = be32_to_cpu(status);
> ),
>
> TP_printk(
> - "error=%d (%s) session=0x%08x slot_nr=%u seq_nr=%u "
> + "error=%lu (%s) session=0x%08x slot_nr=%u seq_nr=%u "
> "highest_slotid=%u",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -572,18 +572,18 @@
>
> TP_STRUCT__entry(
> __field(u32, op)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> __entry->op = op;
> - __entry->error = -error;
> + __entry->error = error;
> ),
>
> TP_printk(
> - "operation %d: nfs status %d (%s)",
> - __entry->op,
> - __entry->error, show_nfsv4_errors(__entry->error)
> + "error=%lu (%s) operation %d:",
> + __entry->error, show_nfsv4_errors(__entry->error),
> + __entry->op
> )
> );
>
> @@ -597,7 +597,7 @@
> TP_ARGS(ctx, flags, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(unsigned int, flags)
> __field(unsigned int, fmode)
> __field(dev_t, dev)
> @@ -615,7 +615,7 @@
> const struct nfs4_state *state = ctx->state;
> const struct inode *inode = NULL;
>
> - __entry->error = error;
> + __entry->error = -error;
> __entry->flags = flags;
> __entry->fmode = (__force unsigned int)ctx->mode;
> __entry->dev = ctx->dentry->d_sb->s_dev;
> @@ -647,7 +647,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) flags=%d (%s) fmode=%s "
> + "error=%lu (%s) flags=%d (%s) fmode=%s "
> "fileid=%02x:%02x:%llu fhandle=0x%08x "
> "name=%02x:%02x:%llu/%s stateid=%d:0x%08x "
> "openstateid=%d:0x%08x",
> @@ -733,7 +733,7 @@
> __field(u32, fhandle)
> __field(u64, fileid)
> __field(unsigned int, fmode)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, stateid_seq)
> __field(u32, stateid_hash)
> ),
> @@ -753,7 +753,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fmode=%s fileid=%02x:%02x:%llu "
> + "error=%lu (%s) fmode=%s fileid=%02x:%02x:%llu "
> "fhandle=0x%08x openstateid=%d:0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -795,7 +795,7 @@
> TP_ARGS(request, state, cmd, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, cmd)
> __field(char, type)
> __field(loff_t, start)
> @@ -825,7 +825,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) cmd=%s:%s range=%lld:%lld "
> + "error=%lu (%s) cmd=%s:%s range=%lld:%lld "
> "fileid=%02x:%02x:%llu fhandle=0x%08x "
> "stateid=%d:0x%08x",
> __entry->error,
> @@ -865,7 +865,7 @@
> TP_ARGS(request, state, lockstateid, cmd, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, cmd)
> __field(char, type)
> __field(loff_t, start)
> @@ -901,7 +901,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) cmd=%s:%s range=%lld:%lld "
> + "error=%lu (%s) cmd=%s:%s range=%lld:%lld "
> "fileid=%02x:%02x:%llu fhandle=0x%08x "
> "stateid=%d:0x%08x lockstateid=%d:0x%08x",
> __entry->error,
> @@ -970,7 +970,7 @@
> TP_STRUCT__entry(
> __field(dev_t, dev)
> __field(u32, fhandle)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, stateid_seq)
> __field(u32, stateid_hash)
> ),
> @@ -986,7 +986,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) dev=%02x:%02x fhandle=0x%08x "
> + "error=%lu (%s) dev=%02x:%02x fhandle=0x%08x "
> "stateid=%d:0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1007,7 +1007,7 @@
> TP_ARGS(state, lsp, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(dev_t, dev)
> __field(u32, fhandle)
> __field(u64, fileid)
> @@ -1029,7 +1029,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "stateid=%d:0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1064,7 +1064,7 @@
>
> TP_STRUCT__entry(
> __field(dev_t, dev)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(u64, dir)
> __string(name, name->name)
> ),
> @@ -1072,12 +1072,12 @@
> TP_fast_assign(
> __entry->dev = dir->i_sb->s_dev;
> __entry->dir = NFS_FILEID(dir);
> - __entry->error = error;
> + __entry->error = -error;
> __assign_str(name, name->name);
> ),
>
> TP_printk(
> - "error=%d (%s) name=%02x:%02x:%llu/%s",
> + "error=%lu (%s) name=%02x:%02x:%llu/%s",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> MAJOR(__entry->dev), MINOR(__entry->dev),
> @@ -1114,7 +1114,7 @@
> TP_STRUCT__entry(
> __field(dev_t, dev)
> __field(u64, ino)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -1124,7 +1124,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) inode=%02x:%02x:%llu",
> + "error=%lu (%s) inode=%02x:%02x:%llu",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> MAJOR(__entry->dev), MINOR(__entry->dev),
> @@ -1145,7 +1145,7 @@
>
> TP_STRUCT__entry(
> __field(dev_t, dev)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(u64, olddir)
> __string(oldname, oldname->name)
> __field(u64, newdir)
> @@ -1162,7 +1162,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) oldname=%02x:%02x:%llu/%s "
> + "error=%lu (%s) oldname=%02x:%02x:%llu/%s "
> "newname=%02x:%02x:%llu/%s",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1187,18 +1187,18 @@
> __field(dev_t, dev)
> __field(u32, fhandle)
> __field(u64, fileid)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> __entry->dev = inode->i_sb->s_dev;
> __entry->fileid = NFS_FILEID(inode);
> __entry->fhandle = nfs_fhandle_hash(NFS_FH(inode));
> - __entry->error = error;
> + __entry->error = error < 0 ? -error : 0;
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x",
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> MAJOR(__entry->dev), MINOR(__entry->dev),
> @@ -1238,7 +1238,7 @@
> __field(dev_t, dev)
> __field(u32, fhandle)
> __field(u64, fileid)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, stateid_seq)
> __field(u32, stateid_hash)
> ),
> @@ -1255,7 +1255,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "stateid=%d:0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1295,7 +1295,7 @@
> __field(u32, fhandle)
> __field(u64, fileid)
> __field(unsigned int, valid)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -1307,7 +1307,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "valid=%s",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1342,7 +1342,7 @@
> TP_ARGS(clp, fhandle, inode, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(dev_t, dev)
> __field(u32, fhandle)
> __field(u64, fileid)
> @@ -1363,7 +1363,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "dstaddr=%s",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1397,7 +1397,7 @@
> TP_ARGS(clp, fhandle, inode, stateid, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(dev_t, dev)
> __field(u32, fhandle)
> __field(u64, fileid)
> @@ -1424,7 +1424,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "stateid=%d:0x%08x dstaddr=%s",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1460,7 +1460,7 @@
> TP_ARGS(name, len, id, error),
>
> TP_STRUCT__entry(
> - __field(int, error)
> + __field(unsigned long, error)
> __field(u32, id)
> __dynamic_array(char, name, len > 0 ? len + 1 : 1)
> ),
> @@ -1475,8 +1475,8 @@
> ),
>
> TP_printk(
> - "error=%d id=%u name=%s",
> - __entry->error,
> + "error=%lu (%s) id=%u name=%s",
> + __entry->error, show_nfsv4_errors(__entry->error),
> __entry->id,
> __get_str(name)
> )
> @@ -1509,7 +1509,7 @@
> __field(u64, fileid)
> __field(loff_t, offset)
> __field(size_t, count)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, stateid_seq)
> __field(u32, stateid_hash)
> ),
> @@ -1523,7 +1523,7 @@
> __entry->fhandle = nfs_fhandle_hash(NFS_FH(inode));
> __entry->offset = hdr->args.offset;
> __entry->count = hdr->args.count;
> - __entry->error = error;
> + __entry->error = error < 0 ? -error : 0;
> __entry->stateid_seq =
> be32_to_cpu(state->stateid.seqid);
> __entry->stateid_hash =
> @@ -1531,7 +1531,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "offset=%lld count=%zu stateid=%d:0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1569,7 +1569,7 @@
> __field(u64, fileid)
> __field(loff_t, offset)
> __field(size_t, count)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, stateid_seq)
> __field(u32, stateid_hash)
> ),
> @@ -1583,7 +1583,7 @@
> __entry->fhandle = nfs_fhandle_hash(NFS_FH(inode));
> __entry->offset = hdr->args.offset;
> __entry->count = hdr->args.count;
> - __entry->error = error;
> + __entry->error = error < 0 ? -error : 0;
> __entry->stateid_seq =
> be32_to_cpu(state->stateid.seqid);
> __entry->stateid_hash =
> @@ -1591,7 +1591,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "offset=%lld count=%zu stateid=%d:0x%08x",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1630,7 +1630,7 @@
> __field(u64, fileid)
> __field(loff_t, offset)
> __field(size_t, count)
> - __field(int, error)
> + __field(unsigned long, error)
> ),
>
> TP_fast_assign(
> @@ -1644,7 +1644,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "offset=%lld count=%zu",
> __entry->error,
> show_nfsv4_errors(__entry->error),
> @@ -1694,7 +1694,7 @@
> __field(u32, iomode)
> __field(u64, offset)
> __field(u64, count)
> - __field(int, error)
> + __field(unsigned long, error)
> __field(int, stateid_seq)
> __field(u32, stateid_hash)
> __field(int, layoutstateid_seq)
> @@ -1727,7 +1727,7 @@
> ),
>
> TP_printk(
> - "error=%d (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> + "error=%lu (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> "iomode=%s offset=%llu count=%llu stateid=%d:0x%08x "
> "layoutstateid=%d:0x%08x",
> __entry->error,
>
next prev parent reply other threads:[~2019-06-11 19:33 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-11 15:07 [PATCH v2 00/19] for-5.3 patches for review Chuck Lever
2019-06-11 15:08 ` [PATCH v2 01/19] xprtrdma: Fix a BUG when tracing is enabled with NFSv4.1 on RDMA Chuck Lever
2019-06-11 15:08 ` [PATCH v2 02/19] xprtrdma: Fix use-after-free in rpcrdma_post_recvs Chuck Lever
2019-06-11 15:08 ` [PATCH v2 03/19] xprtrdma: Replace use of xdr_stream_pos in rpcrdma_marshal_req Chuck Lever
2019-06-11 15:08 ` [PATCH v2 04/19] xprtrdma: Fix occasional transport deadlock Chuck Lever
2019-06-11 15:08 ` [PATCH v2 05/19] xprtrdma: Remove the RPCRDMA_REQ_F_PENDING flag Chuck Lever
2019-06-11 15:08 ` [PATCH v2 06/19] xprtrdma: Remove fr_state Chuck Lever
2019-06-11 15:08 ` [PATCH v2 07/19] xprtrdma: Add mechanism to place MRs back on the free list Chuck Lever
2019-06-11 15:08 ` [PATCH v2 08/19] xprtrdma: Reduce context switching due to Local Invalidation Chuck Lever
2019-06-11 15:08 ` [PATCH v2 09/19] xprtrdma: Wake RPCs directly in rpcrdma_wc_send path Chuck Lever
2019-06-11 15:08 ` [PATCH v2 10/19] xprtrdma: Simplify rpcrdma_rep_create Chuck Lever
2019-06-11 15:08 ` [PATCH v2 11/19] xprtrdma: Streamline rpcrdma_post_recvs Chuck Lever
2019-06-11 15:09 ` [PATCH v2 12/19] xprtrdma: Refactor chunk encoding Chuck Lever
2019-06-11 15:09 ` [PATCH v2 13/19] xprtrdma: Remove rpcrdma_req::rl_buffer Chuck Lever
2019-06-11 15:09 ` [PATCH v2 14/19] xprtrdma: Modernize ops->connect Chuck Lever
2019-06-11 15:09 ` [PATCH v2 15/19] NFS4: Add a trace event to record invalid CB sequence IDs Chuck Lever
2019-06-11 15:09 ` [PATCH v2 16/19] NFS: Fix show_nfs_errors macros again Chuck Lever
2019-06-11 19:33 ` Olga Kornievskaia [this message]
2019-06-11 19:37 ` Chuck Lever
2019-06-11 20:00 ` Olga Kornievskaia
2019-06-11 20:06 ` Chuck Lever
2019-06-11 15:09 ` [PATCH v2 17/19] NFS: Display symbolic status code names in trace log Chuck Lever
2019-06-11 15:09 ` [PATCH v2 18/19] NFS: Update symbolic flags displayed by trace events Chuck Lever
2019-06-11 15:09 ` [PATCH v2 19/19] NFS: Record task, client ID, and XID in xdr_status trace points Chuck Lever
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAN-5tyHx-r6c6RgHk2ocv2CxTgw_8Ebie_fUUSxzaVKotmX1zw@mail.gmail.com \
--to=aglo@umich.edu \
--cc=chuck.lever@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).