diff mbox

[V3] net: sunrpc: fix tracepoint Warning: unknown op '->'

Message ID 26296332fcef5466c2d38c5009efbc6c3e808d11.1440608659.git.panand@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pratyush Anand Aug. 27, 2015, 4:31 a.m. UTC 
`perf stat  -e sunrpc:svc_xprt_do_enqueue true` results in

Warning: unknown op '->'
Warning: [sunrpc:svc_xprt_do_enqueue] unknown op '->'

Similar warning for svc_handle_xprt as well.

Actually TP_printk() should never dereference an address saved in the ring
buffer that points somewhere in the kernel. There's no guarantee that that
object still exists (with the exception of static strings).

Therefore change all the arguments for TP_printk(), so that it references
values existing in the ring buffer only.

While doing that, also fix another possible bug when argument xprt could be
NULL and TP_fast_assign() tries to access it's elements.

Signed-off-by: Pratyush Anand <panand@redhat.com>
Reviewed-by: Jeff Layton <jeff.layton@primarydata.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
---
Changes since V2:
- removed storing of "rqst" now
- removed replacement of , to ; at the end of statement __entry->xprt =
  xprt
Changes since V1:
- struct sockaddr memcopied to ring buffer so that its internal element can
be accessed while printing %pIS.

 include/trace/events/sunrpc.h | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

J. Bruce Fields Aug. 27, 2015, 6:16 p.m. UTC | #1 
Thanks, applying, with:

	Cc: stable@vger.kernel.org
	Fixes: 83a712e0afef "sunrpc: add some tracepoints around ..."

(let me know if that's not right.)

--b.

On Thu, Aug 27, 2015 at 10:01:33AM +0530, Pratyush Anand wrote:
> `perf stat  -e sunrpc:svc_xprt_do_enqueue true` results in
> 
> Warning: unknown op '->'
> Warning: [sunrpc:svc_xprt_do_enqueue] unknown op '->'
> 
> Similar warning for svc_handle_xprt as well.
> 
> Actually TP_printk() should never dereference an address saved in the ring
> buffer that points somewhere in the kernel. There's no guarantee that that
> object still exists (with the exception of static strings).
> 
> Therefore change all the arguments for TP_printk(), so that it references
> values existing in the ring buffer only.
> 
> While doing that, also fix another possible bug when argument xprt could be
> NULL and TP_fast_assign() tries to access it's elements.
> 
> Signed-off-by: Pratyush Anand <panand@redhat.com>
> Reviewed-by: Jeff Layton <jeff.layton@primarydata.com>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> Changes since V2:
> - removed storing of "rqst" now
> - removed replacement of , to ; at the end of statement __entry->xprt =
>   xprt
> Changes since V1:
> - struct sockaddr memcopied to ring buffer so that its internal element can
> be accessed while printing %pIS.
> 
>  include/trace/events/sunrpc.h | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
> index fd1a02cb3c82..003dca933803 100644
> --- a/include/trace/events/sunrpc.h
> +++ b/include/trace/events/sunrpc.h
> @@ -529,18 +529,21 @@ TRACE_EVENT(svc_xprt_do_enqueue,
>  
>  	TP_STRUCT__entry(
>  		__field(struct svc_xprt *, xprt)
> -		__field(struct svc_rqst *, rqst)
> +		__field_struct(struct sockaddr_storage, ss)
> +		__field(int, pid)
> +		__field(unsigned long, flags)
>  	),
>  
>  	TP_fast_assign(
>  		__entry->xprt = xprt;
> -		__entry->rqst = rqst;
> +		xprt ? memcpy(&__entry->ss, &xprt->xpt_remote, sizeof(__entry->ss)) : memset(&__entry->ss, 0, sizeof(__entry->ss));
> +		__entry->pid = rqst? rqst->rq_task->pid : 0;
> +		__entry->flags = xprt ? xprt->xpt_flags : 0;
>  	),
>  
>  	TP_printk("xprt=0x%p addr=%pIScp pid=%d flags=%s", __entry->xprt,
> -		(struct sockaddr *)&__entry->xprt->xpt_remote,
> -		__entry->rqst ? __entry->rqst->rq_task->pid : 0,
> -		show_svc_xprt_flags(__entry->xprt->xpt_flags))
> +		(struct sockaddr *)&__entry->ss,
> +		__entry->pid, show_svc_xprt_flags(__entry->flags))
>  );
>  
>  TRACE_EVENT(svc_xprt_dequeue,
> @@ -589,16 +592,20 @@ TRACE_EVENT(svc_handle_xprt,
>  	TP_STRUCT__entry(
>  		__field(struct svc_xprt *, xprt)
>  		__field(int, len)
> +		__field_struct(struct sockaddr_storage, ss)
> +		__field(unsigned long, flags)
>  	),
>  
>  	TP_fast_assign(
>  		__entry->xprt = xprt;
> +		xprt ? memcpy(&__entry->ss, &xprt->xpt_remote, sizeof(__entry->ss)) : memset(&__entry->ss, 0, sizeof(__entry->ss));
>  		__entry->len = len;
> +		__entry->flags = xprt ? xprt->xpt_flags : 0;
>  	),
>  
>  	TP_printk("xprt=0x%p addr=%pIScp len=%d flags=%s", __entry->xprt,
> -		(struct sockaddr *)&__entry->xprt->xpt_remote, __entry->len,
> -		show_svc_xprt_flags(__entry->xprt->xpt_flags))
> +		(struct sockaddr *)&__entry->ss,
> +		__entry->len, show_svc_xprt_flags(__entry->flags))
>  );
>  #endif /* _TRACE_SUNRPC_H */
>  
> -- 
> 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index fd1a02cb3c82..003dca933803 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -529,18 +529,21 @@  TRACE_EVENT(svc_xprt_do_enqueue,
 
 	TP_STRUCT__entry(
 		__field(struct svc_xprt *, xprt)
-		__field(struct svc_rqst *, rqst)
+		__field_struct(struct sockaddr_storage, ss)
+		__field(int, pid)
+		__field(unsigned long, flags)
 	),
 
 	TP_fast_assign(
 		__entry->xprt = xprt;
-		__entry->rqst = rqst;
+		xprt ? memcpy(&__entry->ss, &xprt->xpt_remote, sizeof(__entry->ss)) : memset(&__entry->ss, 0, sizeof(__entry->ss));
+		__entry->pid = rqst? rqst->rq_task->pid : 0;
+		__entry->flags = xprt ? xprt->xpt_flags : 0;
 	),
 
 	TP_printk("xprt=0x%p addr=%pIScp pid=%d flags=%s", __entry->xprt,
-		(struct sockaddr *)&__entry->xprt->xpt_remote,
-		__entry->rqst ? __entry->rqst->rq_task->pid : 0,
-		show_svc_xprt_flags(__entry->xprt->xpt_flags))
+		(struct sockaddr *)&__entry->ss,
+		__entry->pid, show_svc_xprt_flags(__entry->flags))
 );
 
 TRACE_EVENT(svc_xprt_dequeue,
@@ -589,16 +592,20 @@  TRACE_EVENT(svc_handle_xprt,
 	TP_STRUCT__entry(
 		__field(struct svc_xprt *, xprt)
 		__field(int, len)
+		__field_struct(struct sockaddr_storage, ss)
+		__field(unsigned long, flags)
 	),
 
 	TP_fast_assign(
 		__entry->xprt = xprt;
+		xprt ? memcpy(&__entry->ss, &xprt->xpt_remote, sizeof(__entry->ss)) : memset(&__entry->ss, 0, sizeof(__entry->ss));
 		__entry->len = len;
+		__entry->flags = xprt ? xprt->xpt_flags : 0;
 	),
 
 	TP_printk("xprt=0x%p addr=%pIScp len=%d flags=%s", __entry->xprt,
-		(struct sockaddr *)&__entry->xprt->xpt_remote, __entry->len,
-		show_svc_xprt_flags(__entry->xprt->xpt_flags))
+		(struct sockaddr *)&__entry->ss,
+		__entry->len, show_svc_xprt_flags(__entry->flags))
 );
 #endif /* _TRACE_SUNRPC_H */