All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: "chuck.lever@oracle.com" <chuck.lever@oracle.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v4 2/7] SUNRPC: Avoid NULL pointer dereferences in tracepoints
Date: Wed, 20 Oct 2021 18:43:55 +0000	[thread overview]
Message-ID: <7cf900a259aba1ef9e7b4e081945fe63779f2db9.camel@hammerspace.com> (raw)
In-Reply-To: <163442175137.1585.5220370195383350175.stgit@morisot.1015granger.net>

On Sat, 2021-10-16 at 18:02 -0400, Chuck Lever wrote:
> On occasion, a NULL rpc_task pointer is passed into tracepoints.
> We've open-coded a few places where this is expected, but let's
> be defensive and protect every place that wants to dereference
> a task to assign the tk_pid and cl_clid.
> 

No, I won't apply this.

This patch is going to lead to a bunch of static checkers complaining
that we're checking 'task' for NULL after we've already dereferenced it
in the same function, and/or complaining that we're dereferencing a
value that might be null because we just had a check for it.

Even within the macros touched by this patch, we have instances of the
latter occurring, where we read task->tk_client->cl_vers (and other
fields) immediately after we just declared that we had to check both
task and task->tk_client for NULL.

> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfs/nfs4trace.h                 |    8 +---
>  fs/nfs/nfstrace.h                  |    3 -
>  include/trace/events/rpcgss.h      |   18 +++-----
>  include/trace/events/rpcrdma.h     |   62 ++++++++------------------
> ---
>  include/trace/events/sunrpc.h      |   77 +++++++++-----------------
> ----------
>  include/trace/events/sunrpc_base.h |   15 +++++++
>  6 files changed, 61 insertions(+), 122 deletions(-)
> 
> diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
> index d4f061046c09..66fbd3c33c15 100644
> --- a/fs/nfs/nfs4trace.h
> +++ b/fs/nfs/nfs4trace.h
> @@ -686,10 +686,8 @@ TRACE_EVENT(nfs4_xdr_bad_operation,
>  
>                 TP_fast_assign(
>                         const struct rpc_rqst *rqstp = xdr->rqst;
> -                       const struct rpc_task *task = rqstp->rq_task;
>  
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client-
> >cl_clid;
> +                       SUNRPC_TRACE_TASK_ASSIGN(rqstp->rq_task);
>                         __entry->xid = be32_to_cpu(rqstp->rq_xid);
>                         __entry->op = op;
>                         __entry->expected = expected;
> @@ -721,10 +719,8 @@ DECLARE_EVENT_CLASS(nfs4_xdr_event,
>  
>                 TP_fast_assign(
>                         const struct rpc_rqst *rqstp = xdr->rqst;
> -                       const struct rpc_task *task = rqstp->rq_task;
>  
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client-
> >cl_clid;
> +                       SUNRPC_TRACE_TASK_ASSIGN(rqstp->rq_task);
>                         __entry->xid = be32_to_cpu(rqstp->rq_xid);
>                         __entry->op = op;
>                         __entry->error = error;
> diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
> index 82b51120450f..e9be65b52bfe 100644
> --- a/fs/nfs/nfstrace.h
> +++ b/fs/nfs/nfstrace.h
> @@ -1401,8 +1401,7 @@ DECLARE_EVENT_CLASS(nfs_xdr_event,
>                         const struct rpc_rqst *rqstp = xdr->rqst;
>                         const struct rpc_task *task = rqstp->rq_task;
>  
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client-
> >cl_clid;
> +                       SUNRPC_TRACE_TASK_ASSIGN(task);
>                         __entry->xid = be32_to_cpu(rqstp->rq_xid);
>                         __entry->version = task->tk_client->cl_vers;
>                         __entry->error = error;
> diff --git a/include/trace/events/rpcgss.h
> b/include/trace/events/rpcgss.h
> index 3ba63319af3c..87b17ebd09c3 100644
> --- a/include/trace/events/rpcgss.h
> +++ b/include/trace/events/rpcgss.h
> @@ -96,8 +96,7 @@ DECLARE_EVENT_CLASS(rpcgss_gssapi_event,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->maj_stat = maj_stat;
>         ),
>  
> @@ -330,8 +329,7 @@ TRACE_EVENT(rpcgss_unwrap_failed,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>         ),
>  
>         TP_printk(SUNRPC_TRACE_TASK_SPECIFIER,
> @@ -355,8 +353,7 @@ TRACE_EVENT(rpcgss_bad_seqno,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->expected = expected;
>                 __entry->received = received;
>         ),
> @@ -384,8 +381,7 @@ TRACE_EVENT(rpcgss_seqno,
>         TP_fast_assign(
>                 const struct rpc_rqst *rqst = task->tk_rqstp;
>  
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->seqno = rqst->rq_seqno;
>         ),
> @@ -414,8 +410,7 @@ TRACE_EVENT(rpcgss_need_reencode,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(task->tk_rqstp->rq_xid);
>                 __entry->seq_xmit = seq_xmit;
>                 __entry->seqno = task->tk_rqstp->rq_seqno;
> @@ -448,8 +443,7 @@ TRACE_EVENT(rpcgss_update_slack,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(task->tk_rqstp->rq_xid);
>                 __entry->auth = auth;
>                 __entry->rslack = auth->au_rslack;
> diff --git a/include/trace/events/rpcrdma.h
> b/include/trace/events/rpcrdma.h
> index 7f46ef621c95..c2ab9e5d775b 100644
> --- a/include/trace/events/rpcrdma.h
> +++ b/include/trace/events/rpcrdma.h
> @@ -271,8 +271,7 @@ DECLARE_EVENT_CLASS(xprtrdma_rdch_event,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->pos = pos;
>                 __entry->nents = mr->mr_nents;
>                 __entry->handle = mr->mr_handle;
> @@ -320,8 +319,7 @@ DECLARE_EVENT_CLASS(xprtrdma_wrch_event,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->nents = mr->mr_nents;
>                 __entry->handle = mr->mr_handle;
>                 __entry->length = mr->mr_length;
> @@ -380,15 +378,8 @@ DECLARE_EVENT_CLASS(xprtrdma_mr_class,
>         TP_fast_assign(
>                 const struct rpcrdma_req *req = mr->mr_req;
>  
> -               if (req) {
> -                       const struct rpc_task *task = req-
> >rl_slot.rq_task;
> -
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client-
> >cl_clid;
> -               } else {
> -                       __entry->task_id = 0;
> -                       __entry->client_id = -1;
> -               }
> +               if (req)
> +                       SUNRPC_TRACE_TASK_ASSIGN(req-
> >rl_slot.rq_task);
>                 __entry->mr_id  = mr->mr_ibmr->res.id;
>                 __entry->nents  = mr->mr_nents;
>                 __entry->handle = mr->mr_handle;
> @@ -633,10 +624,7 @@ TRACE_EVENT(xprtrdma_nomrs_err,
>         ),
>  
>         TP_fast_assign(
> -               const struct rpc_rqst *rqst = &req->rl_slot;
> -
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(req->rl_slot.rq_task);
>                 __assign_str(addr, rpcrdma_addrstr(r_xprt));
>                 __assign_str(port, rpcrdma_portstr(r_xprt));
>         ),
> @@ -694,8 +682,7 @@ TRACE_EVENT(xprtrdma_marshal,
>         TP_fast_assign(
>                 const struct rpc_rqst *rqst = &req->rl_slot;
>  
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->hdrlen = req->rl_hdrbuf.len;
>                 __entry->headlen = rqst->rq_snd_buf.head[0].iov_len;
> @@ -730,8 +717,7 @@ TRACE_EVENT(xprtrdma_marshal_failed,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->ret = ret;
>         ),
> @@ -757,8 +743,7 @@ TRACE_EVENT(xprtrdma_prepsend_failed,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->ret = ret;
>         ),
> @@ -791,9 +776,7 @@ TRACE_EVENT(xprtrdma_post_send,
>  
>                 __entry->cq_id = sc->sc_cid.ci_queue_id;
>                 __entry->completion_id = sc->sc_cid.ci_completion_id;
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client ?
> -                                    rqst->rq_task->tk_client-
> >cl_clid : -1;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->num_sge = req->rl_wr.num_sge;
>                 __entry->signaled = req->rl_wr.send_flags &
> IB_SEND_SIGNALED;
>         ),
> @@ -827,9 +810,7 @@ TRACE_EVENT(xprtrdma_post_send_err,
>                 const struct rpcrdma_ep *ep = r_xprt->rx_ep;
>  
>                 __entry->cq_id = ep ? ep->re_attr.recv_cq->res.id :
> 0;
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client ?
> -                                    rqst->rq_task->tk_client-
> >cl_clid : -1;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->rc = rc;
>         ),
>  
> @@ -938,10 +919,7 @@ TRACE_EVENT(xprtrdma_post_linv_err,
>         ),
>  
>         TP_fast_assign(
> -               const struct rpc_task *task = req->rl_slot.rq_task;
> -
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(req->rl_slot.rq_task);
>                 __entry->status = status;
>         ),
>  
> @@ -1127,8 +1105,7 @@ TRACE_EVENT(xprtrdma_reply,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(rep->rr_xid);
>                 __entry->credits = credits;
>         ),
> @@ -1162,8 +1139,7 @@ TRACE_EVENT(xprtrdma_err_vers,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->min = be32_to_cpup(min);
>                 __entry->max = be32_to_cpup(max);
> @@ -1189,8 +1165,7 @@ TRACE_EVENT(xprtrdma_err_chunk,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>         ),
>  
> @@ -1215,8 +1190,7 @@ TRACE_EVENT(xprtrdma_err_unrecognized,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->procedure = be32_to_cpup(procedure);
>         ),
>  
> @@ -1244,8 +1218,7 @@ TRACE_EVENT(xprtrdma_fixup,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->fixup = fixup;
>                 __entry->headlen = rqst->rq_rcv_buf.head[0].iov_len;
>                 __entry->pagelen = rqst->rq_rcv_buf.page_len;
> @@ -1298,8 +1271,7 @@ TRACE_EVENT(xprtrdma_mrs_zap,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>         ),
>  
>         TP_printk(SUNRPC_TRACE_TASK_SPECIFIER,
> diff --git a/include/trace/events/sunrpc.h
> b/include/trace/events/sunrpc.h
> index 92def7d6663e..4fd6299bc907 100644
> --- a/include/trace/events/sunrpc.h
> +++ b/include/trace/events/sunrpc.h
> @@ -69,9 +69,7 @@ DECLARE_EVENT_CLASS(rpc_xdr_buf_class,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client ?
> -                                    task->tk_client->cl_clid : -1;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->head_base = xdr->head[0].iov_base;
>                 __entry->head_len = xdr->head[0].iov_len;
>                 __entry->tail_base = xdr->tail[0].iov_base;
> @@ -248,8 +246,7 @@ DECLARE_EVENT_CLASS(rpc_task_status,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->status = task->tk_status;
>         ),
>  
> @@ -285,8 +282,7 @@ TRACE_EVENT(rpc_request,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->version = task->tk_client->cl_vers;
>                 __entry->async = RPC_IS_ASYNC(task);
>                 __assign_str(progname, task->tk_client->cl_program-
> >name);
> @@ -344,9 +340,7 @@ DECLARE_EVENT_CLASS(rpc_task_running,
>                 ),
>  
>         TP_fast_assign(
> -               __entry->client_id = task->tk_client ?
> -                                    task->tk_client->cl_clid : -1;
> -               __entry->task_id = task->tk_pid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->action = action;
>                 __entry->runstate = task->tk_runstate;
>                 __entry->status = task->tk_status;
> @@ -396,9 +390,7 @@ DECLARE_EVENT_CLASS(rpc_task_queued,
>                 ),
>  
>         TP_fast_assign(
> -               __entry->client_id = task->tk_client ?
> -                                    task->tk_client->cl_clid : -1;
> -               __entry->task_id = task->tk_pid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->timeout = rpc_task_timeout(task);
>                 __entry->runstate = task->tk_runstate;
>                 __entry->status = task->tk_status;
> @@ -439,8 +431,7 @@ DECLARE_EVENT_CLASS(rpc_failure,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>         ),
>  
>         TP_printk(SUNRPC_TRACE_TASK_SPECIFIER,
> @@ -476,8 +467,7 @@ DECLARE_EVENT_CLASS(rpc_reply_event,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(task->tk_rqstp->rq_xid);
>                 __assign_str(progname, task->tk_client->cl_program-
> >name);
>                 __entry->version = task->tk_client->cl_vers;
> @@ -539,8 +529,7 @@ TRACE_EVENT(rpc_buf_alloc,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->callsize = task->tk_rqstp->rq_callsize;
>                 __entry->recvsize = task->tk_rqstp->rq_rcvsize;
>                 __entry->status = status;
> @@ -570,8 +559,7 @@ TRACE_EVENT(rpc_call_rpcerror,
>         ),
>  
>         TP_fast_assign(
> -               __entry->client_id = task->tk_client->cl_clid;
> -               __entry->task_id = task->tk_pid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->tk_status = tk_status;
>                 __entry->rpc_status = rpc_status;
>         ),
> @@ -606,8 +594,7 @@ TRACE_EVENT(rpc_stats_latency,
>         ),
>  
>         TP_fast_assign(
> -               __entry->client_id = task->tk_client->cl_clid;
> -               __entry->task_id = task->tk_pid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(task->tk_rqstp->rq_xid);
>                 __entry->version = task->tk_client->cl_vers;
>                 __assign_str(progname, task->tk_client->cl_program-
> >name);
> @@ -655,8 +642,7 @@ TRACE_EVENT(rpc_xdr_overflow,
>                 if (xdr->rqst) {
>                         const struct rpc_task *task = xdr->rqst-
> >rq_task;
>  
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client-
> >cl_clid;
> +                       SUNRPC_TRACE_TASK_ASSIGN(task);
>                         __assign_str(progname,
>                                      task->tk_client->cl_program-
> >name);
>                         __entry->version = task->tk_client->cl_vers;
> @@ -721,8 +707,7 @@ TRACE_EVENT(rpc_xdr_alignment,
>         TP_fast_assign(
>                 const struct rpc_task *task = xdr->rqst->rq_task;
>  
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __assign_str(progname,
>                              task->tk_client->cl_program->name);
>                 __entry->version = task->tk_client->cl_vers;
> @@ -922,8 +907,7 @@ TRACE_EVENT(rpc_socket_nospace,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->total = rqst->rq_slen;
>                 __entry->remaining = rqst->rq_slen - transport-
> >xmit.offset;
>         ),
> @@ -1046,9 +1030,7 @@ TRACE_EVENT(xprt_transmit,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client ?
> -                       rqst->rq_task->tk_client->cl_clid : -1;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->seqno = rqst->rq_seqno;
>                 __entry->status = status;
> @@ -1082,9 +1064,7 @@ TRACE_EVENT(xprt_retransmit,
>         TP_fast_assign(
>                 struct rpc_task *task = rqst->rq_task;
>  
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client ?
> -                       task->tk_client->cl_clid : -1;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>                 __entry->ntrans = rqst->rq_ntrans;
>                 __entry->timeout = task->tk_timeout;
> @@ -1137,14 +1117,7 @@ DECLARE_EVENT_CLASS(xprt_writelock_event,
>         ),
>  
>         TP_fast_assign(
> -               if (task) {
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client ?
> -                                            task->tk_client->cl_clid
> : -1;
> -               } else {
> -                       __entry->task_id = -1;
> -                       __entry->client_id = -1;
> -               }
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->snd_task_id = xprt->snd_task ?
>                                         xprt->snd_task->tk_pid : -1;
>         ),
> @@ -1183,14 +1156,7 @@ DECLARE_EVENT_CLASS(xprt_cong_event,
>         ),
>  
>         TP_fast_assign(
> -               if (task) {
> -                       __entry->task_id = task->tk_pid;
> -                       __entry->client_id = task->tk_client ?
> -                                            task->tk_client->cl_clid
> : -1;
> -               } else {
> -                       __entry->task_id = -1;
> -                       __entry->client_id = -1;
> -               }
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->snd_task_id = xprt->snd_task ?
>                                         xprt->snd_task->tk_pid : -1;
>                 __entry->cong = xprt->cong;
> @@ -1233,8 +1199,7 @@ TRACE_EVENT(xprt_reserve,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = rqst->rq_task->tk_pid;
> -               __entry->client_id = rqst->rq_task->tk_client-
> >cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(rqst->rq_task);
>                 __entry->xid = be32_to_cpu(rqst->rq_xid);
>         ),
>  
> @@ -1318,8 +1283,7 @@ TRACE_EVENT(rpcb_getport,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = clnt->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->program = clnt->cl_prog;
>                 __entry->version = clnt->cl_vers;
>                 __entry->protocol = task->tk_xprt->prot;
> @@ -1352,8 +1316,7 @@ TRACE_EVENT(rpcb_setport,
>         ),
>  
>         TP_fast_assign(
> -               __entry->task_id = task->tk_pid;
> -               __entry->client_id = task->tk_client->cl_clid;
> +               SUNRPC_TRACE_TASK_ASSIGN(task);
>                 __entry->status = status;
>                 __entry->port = port;
>         ),
> diff --git a/include/trace/events/sunrpc_base.h
> b/include/trace/events/sunrpc_base.h
> index 588557d07ea8..2cbed4a9a63a 100644
> --- a/include/trace/events/sunrpc_base.h
> +++ b/include/trace/events/sunrpc_base.h
> @@ -10,6 +10,21 @@
>  
>  #include <linux/tracepoint.h>
>  
> +#define SUNRPC_TRACE_PID_SPECIAL       (-1)
> +
> +#define SUNRPC_TRACE_TASK_ASSIGN(t) \
> +       do { \
> +               if ((t) != NULL) { \
> +                       __entry->task_id = (t)->tk_pid; \
> +                       __entry->client_id = (t)->tk_client ? \
> +                                            (t)->tk_client->cl_clid
> : \
> +                                           
> SUNRPC_TRACE_PID_SPECIAL; \
> +               } else { \
> +                       __entry->task_id = SUNRPC_TRACE_PID_SPECIAL;
> \
> +                       __entry->client_id =
> SUNRPC_TRACE_PID_SPECIAL; \
> +               } \
> +       } while (0);
> +
>  #define SUNRPC_TRACE_PID_SPECIFIER     "%08x"
>  #define SUNRPC_TRACE_CLID_SPECIFIER    "%08x"
>  #define SUNRPC_TRACE_TASK_SPECIFIER \
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



  reply	other threads:[~2021-10-20 18:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-16 22:02 [PATCH v4 0/7] Update RPC task pid as displayed by tracepoints Chuck Lever
2021-10-16 22:02 ` [PATCH v4 1/7] SUNRPC: Tracepoints should display tk_pid and cl_clid as a fixed-size field Chuck Lever
2021-10-16 22:02 ` [PATCH v4 2/7] SUNRPC: Avoid NULL pointer dereferences in tracepoints Chuck Lever
2021-10-20 18:43   ` Trond Myklebust [this message]
2021-10-20 19:39     ` Chuck Lever III
2021-10-20 20:09       ` Trond Myklebust
2021-10-16 22:02 ` [PATCH v4 3/7] SUNRPC: Use BIT() macro in rpc_show_xprt_state() Chuck Lever
2021-10-16 22:02 ` [PATCH v4 4/7] SUNRPC: Don't dereference xprt->snd_task if it's a cookie Chuck Lever
2021-10-16 22:02 ` [PATCH v4 5/7] NFS: Replace dprintk callsites in nfs_readpage(s) Chuck Lever
2021-11-02 19:36   ` David Wysochanski
2021-11-02 19:41     ` Chuck Lever III
2021-10-16 22:02 ` [PATCH v4 6/7] SUNRPC: Trace calls to .rpc_call_done Chuck Lever
2021-10-16 22:03 ` [PATCH v4 7/7] NFS: Remove --> and <-- dprintk call sites 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=7cf900a259aba1ef9e7b4e081945fe63779f2db9.camel@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.