Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752506AbbBXOI0 (ORCPT ); Tue, 24 Feb 2015 09:08:26 -0500 Received: from cdptpa-outbound-snat.email.rr.com ([107.14.166.230]:35231 "EHLO cdptpa-oedge-vip.email.rr.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751151AbbBXOIZ (ORCPT ); Tue, 24 Feb 2015 09:08:25 -0500 Date: Tue, 24 Feb 2015 09:09:15 -0500 From: Steven Rostedt To: James Hogan Cc: Jeff Layton , "J. Bruce Fields" , , Trond Myklebust , Ingo Molnar , Subject: Re: [PATCH] sunrpc: Fix trace events to store data in the struct Message-ID: <20150224090915.40d8c7ff@grimm.local.home> In-Reply-To: <1424778476-28242-1-git-send-email-james.hogan@imgtec.com> References: <1424778476-28242-1-git-send-email-james.hogan@imgtec.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-RR-Connecting-IP: 107.14.168.130:25 X-Cloudmark-Score: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2069 Lines: 64 On Tue, 24 Feb 2015 11:47:56 +0000 James Hogan wrote: > TP_printk("xprt=0x%p addr=%pIScp pid=%d flags=%s", __entry->xprt, > - (struct sockaddr *)&__entry->xprt->xpt_remote, There's actually nothing wrong with the above even if xprt is NULL. It's not dereferencing the structure, it is just getting the address of what would be dereference. > - __entry->rqst ? __entry->rqst->rq_task->pid : 0, > - show_svc_xprt_flags(__entry->xprt->xpt_flags)) > + (struct sockaddr *)&__entry->ss, The above is meaningless. You just printed the address of the ring buffer and this will be different (and useless) every time. > + __entry->pid, > + show_svc_xprt_flags(__entry->flags)) > ); > > TRACE_EVENT(svc_xprt_dequeue, > @@ -562,17 +566,21 @@ TRACE_EVENT(svc_handle_xprt, > > TP_STRUCT__entry( > __field(struct svc_xprt *, xprt) > + __field_struct(struct sockaddr_storage, ss) > + __field(unsigned long, flags); > __field(int, len) > ), > > TP_fast_assign( > __entry->xprt = xprt; > + xprt ? memcpy(&__entry->ss, &xprt->xpt_remote, sizeof(__entry->ss)) : memset(&__entry->ss, 0, sizeof(__entry->ss)); > + __entry->flags = xprt ? xprt->xpt_flags : 0; > __entry->len = len; > ), > > 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, Ditto. Don't use field_struct() unless you really know what you are doing. This is copying the entire struct into the ring buffer and only using the address of that struct. Which not only is useless, but wastes a lot of space in the ring buffer. -- Steve > + show_svc_xprt_flags(__entry->flags)) > ); > #endif /* _TRACE_SUNRPC_H */ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/