Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934979AbcLMULF (ORCPT ); Tue, 13 Dec 2016 15:11:05 -0500 Received: from smtprelay0078.hostedemail.com ([216.40.44.78]:59148 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932818AbcLMUK2 (ORCPT ); Tue, 13 Dec 2016 15:10:28 -0500 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::::::::::,RULES_HIT:2:41:355:379:541:599:800:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1434:1437:1515:1516:1518:1535:1593:1594:1605:1730:1747:1777:1792:2393:2553:2559:2562:2693:2897:3138:3139:3140:3141:3142:3622:3865:3866:3867:3870:3871:3872:3874:4049:4118:4321:4605:5007:6119:6261:6609:7875:10004:10848:10967:11026:11232:11473:11657:11658:11914:12043:12291:12296:12438:12555:12683:12740:12760:12986:13019:13439:14096:14097:14659:21080:21451,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:4,LUA_SUMMARY:none X-HE-Tag: owl34_77be90f60a760 X-Filterd-Recvd-Size: 7179 Date: Tue, 13 Dec 2016 15:10:21 -0500 From: Steven Rostedt To: Subhash Jadavani Cc: vinholikatti@gmail.com, jejb@linux.vnet.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, Ingo Molnar , Sahitya Tummala , Venkat Gopalakrishnan , Lee Susman , linux-kernel@vger.kernel.org (open list) Subject: Re: [PATCH v2 02/12] scsi: ufs: add tracing support Message-ID: <20161213151021.217fef28@gandalf.local.home> In-Reply-To: <1481658529-31807-1-git-send-email-subhashj@codeaurora.org> References: <1481658529-31807-1-git-send-email-subhashj@codeaurora.org> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5875 Lines: 198 On Tue, 13 Dec 2016 11:48:45 -0800 Subhash Jadavani wrote: > This change adds the ftrace support for following: > 1. UFS initialization time > 2. Clock gating states > 3. Clock scaling states > 4. Power management APIs latency > 5. BKOPs enable/disable > > Usage: > echo 1 > /sys/kernel/debug/tracing/events/ufs/enable > cat /sys/kernel/debug/tracing/trace_pipe > > Reviewed-by: Sahitya Tummala > Signed-off-by: Subhash Jadavani > --- > drivers/scsi/ufs/ufshcd.c | 133 ++++++++++++++++++++++++++++++++++++---- > include/trace/events/ufs.h | 149 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 270 insertions(+), 12 deletions(-) > create mode 100644 include/trace/events/ufs.h > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index fe516b2..73c5937 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -45,6 +45,9 @@ > #include "ufs_quirks.h" > #include "unipro.h" > > +#define CREATE_TRACE_POINTS > +#include > + > #define UFSHCD_REQ_SENSE_SIZE 18 > > #define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\ > @@ -721,6 +724,40 @@ static void ufshcd_resume_clkscaling(struct ufs_hba *hba) > devfreq_resume_device(hba->devfreq); > } > > +static const char *ufschd_uic_link_state_to_string( > + enum uic_link_state state) > +{ > + switch (state) { > + case UIC_LINK_OFF_STATE: return "UIC_LINK_OFF_STATE"; > + case UIC_LINK_ACTIVE_STATE: return "UIC_LINK_ACTIVE_STATE"; > + case UIC_LINK_HIBERN8_STATE: return "UIC_LINK_HIBERN8_STATE"; > + default: return "UNKNOWN_STATE"; > + } > +} > + > +static const char *ufschd_ufs_dev_pwr_mode_to_string( > + enum ufs_dev_pwr_mode state) > +{ > + switch (state) { > + case UFS_ACTIVE_PWR_MODE: return "UFS_ACTIVE_PWR_MODE"; > + case UFS_SLEEP_PWR_MODE: return "UFS_SLEEP_PWR_MODE"; > + case UFS_POWERDOWN_PWR_MODE: return "UFS_POWERDOWN_PWR_MODE"; > + default: return "UNKNOWN_STATE"; > + } > +} > + > +static const char *ufschd_clk_gating_state_to_string( > + enum clk_gating_state state) > +{ > + switch (state) { > + case CLKS_OFF: return "CLKS_OFF"; > + case CLKS_ON: return "CLKS_ON"; > + case REQ_CLKS_OFF: return "REQ_CLKS_OFF"; > + case REQ_CLKS_ON: return "REQ_CLKS_ON"; > + default: return "UNKNOWN_STATE"; > + } > +} > + A much better way than the above is to use the TRACE_DEFINE_ENUM() macros in the include/trace/events/ufs.h header. In fact, that's what it was made for. Not only will it be much faster to record, it wont waste as much space in the ring buffer. .e.g. #define UFS_LINK_STATES \ EM( UIC_LINK_OFF_STATE ) \ EM( UIC_LINK_ACTIVE_STATE ) \ EMe(UIC_LINK_HIBERN8_STATE ) #define UFS_PWR_MODES \ EM( UFS_ACTIVE_PWR_MODE ) \ EM( UFS_SLEEP_PWR_MODE ) \ EM( UFS_POWERDOWN_PWR_MODE) \ EMe(UFS_POWERDOWN_PWR_MODE) #define UFSCHD_CLK_GATING_STATES \ EM( CLKS_OFF) \ EM( CLKS_ON) \ EM( REQ_CLKS_OFF) \ EMe(REQ_CLKS_ON) #undef EM #undef EMe #define EM(a) TRACE_DEFINE_ENUM(a); #define EMe(a) TRACE_DEFINE_ENUM(a); UFS_LINK_STATES UFS_PWR_MODES UFSCHD_CLK_GATING_STATES #undef EM #undef EMe #define EM(a) { a, #a }, #define EMe(a) { a, #a } DECLARE_EVENT_CLASS(ufshcd_template, TP_PROTO(const char *dev_name int err, s64 usecs, int state, int link_state); [..] TP_printk( "%s: took %lld, dev_state: %s, link_state: %s, err %d", get_str(dev_name), __entry->usecs, __print_symbolic(__entry->state, UFS_PWR_MODES), __print_symbolic(__entry->link_state, UFS_LINK_STATES), __entry->err ) Not to mention, I think some of the strings may not be matching what was passed in. -- Steve > +DECLARE_EVENT_CLASS(ufshcd_template, > + TP_PROTO(const char *dev_name, int err, s64 usecs, > + const char *dev_state, const char *link_state), > + > + TP_ARGS(dev_name, err, usecs, dev_state, link_state), > + > + TP_STRUCT__entry( > + __field(s64, usecs) > + __field(int, err) > + __string(dev_name, dev_name) > + __string(dev_state, dev_state) > + __string(link_state, link_state) > + ), > + > + TP_fast_assign( > + __entry->usecs = usecs; > + __entry->err = err; > + __assign_str(dev_name, dev_name); > + __assign_str(dev_state, dev_state); > + __assign_str(link_state, link_state); > + ), > + > + TP_printk( > + "%s: took %lld usecs, dev_state: %s, link_state: %s, err %d", > + __get_str(dev_name), > + __entry->usecs, > + __get_str(dev_state), > + __get_str(link_state), > + __entry->err > + ) > +); > + > +DEFINE_EVENT(ufshcd_template, ufshcd_system_suspend, > + TP_PROTO(const char *dev_name, int err, s64 usecs, > + const char *dev_state, const char *link_state), > + TP_ARGS(dev_name, err, usecs, dev_state, link_state)); > + > +DEFINE_EVENT(ufshcd_template, ufshcd_system_resume, > + TP_PROTO(const char *dev_name, int err, s64 usecs, > + const char *dev_state, const char *link_state), > + TP_ARGS(dev_name, err, usecs, dev_state, link_state)); > + > +DEFINE_EVENT(ufshcd_template, ufshcd_runtime_suspend, > + TP_PROTO(const char *dev_name, int err, s64 usecs, > + const char *dev_state, const char *link_state), > + TP_ARGS(dev_name, err, usecs, dev_state, link_state)); > + > +DEFINE_EVENT(ufshcd_template, ufshcd_runtime_resume, > + TP_PROTO(const char *dev_name, int err, s64 usecs, > + const char *dev_state, const char *link_state), > + TP_ARGS(dev_name, err, usecs, dev_state, link_state)); > + > +DEFINE_EVENT(ufshcd_template, ufshcd_init, > + TP_PROTO(const char *dev_name, int err, s64 usecs, > + const char *dev_state, const char *link_state), > + TP_ARGS(dev_name, err, usecs, dev_state, link_state)); > +#endif /* if !defined(_TRACE_UFS_H) || defined(TRACE_HEADER_MULTI_READ) */ > + > +/* This part must be outside protection */ > +#include