Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752700AbcC3JmF (ORCPT ); Wed, 30 Mar 2016 05:42:05 -0400 Received: from mail-yw0-f180.google.com ([209.85.161.180]:33124 "EHLO mail-yw0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752059AbcC3JmC (ORCPT ); Wed, 30 Mar 2016 05:42:02 -0400 MIME-Version: 1.0 In-Reply-To: References: <45a62181170848cbd00c8e863f7c84772f2a8a5c.1459147046.git.baolin.wang@linaro.org> Date: Wed, 30 Mar 2016 17:42:00 +0800 Message-ID: Subject: Re: [PATCH v3] mmc: Provide tracepoints for request processing From: Baolin Wang To: Ulf Hansson Cc: Steven Rostedt , mingo@redhat.com, Adrian Hunter , Yangbo Lu , Andrew Morton , "JBottomley@Odin.com" , "Luca Porzio (lporzio)" , Jon Hunter , Grant Grundler , Jens Axboe , Fabian Frederick , Yunpeng Gao , Dan Williams , Rabin Vincent , "Dong, Chuanxiao" , Shawn Lin , =?UTF-8?Q?Heiko_St=C3=BCbner?= , Doug Anderson , David Jander , Mark Brown , Linus Walleij , Takahiro Akashi , "linux-kernel@vger.kernel.org" , linux-mmc Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2723 Lines: 84 On 30 March 2016 at 17:25, Ulf Hansson wrote: > On 28 March 2016 at 08:39, Baolin Wang wrote: >> This patch provides some tracepoints for the lifecycle of a mmc request from >> starting to completion to help with performance analysis of MMC subsystem. >> >> Changes since v2: >> - Remove some redundant tracepoints which are repeated in block layer. >> >> Signed-off-by: Baolin Wang >> --- >> drivers/mmc/core/core.c | 7 +++ >> include/trace/events/mmc.h | 129 ++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 136 insertions(+) >> create mode 100644 include/trace/events/mmc.h >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index f95d41f..7222e3f 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -36,6 +36,9 @@ >> #include >> #include >> >> +#define CREATE_TRACE_POINTS >> +#include >> + >> #include "core.h" >> #include "bus.h" >> #include "host.h" >> @@ -152,6 +155,8 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq) >> >> led_trigger_event(host->led, LED_OFF); >> >> + trace_mmc_request_done(host, mrq); >> + > > At this point we will lack information about "retries" and also about > the re-tune state. I think both would be valuable information to share > about each request. > > So, I would suggest you to move the trace a bit further up, before the > if-sentence and include "retries" and the "re-tune state" via trace > print. Make sense. I'll add the "retries" and the "re-tune state" information in next version. > >> if (mrq->sbc) { >> pr_debug("%s: req done : %d: %08x %08x %08x %08x\n", >> mmc_hostname(host), mrq->sbc->opcode, >> @@ -229,6 +234,8 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) >> if (mmc_card_removed(host->card)) >> return -ENOMEDIUM; >> >> + trace_mmc_request_start(host, mrq); > > This isn't the only place a request will be started from, thus we may > be missing valuable information about which command/request is being > sent. > > I would move this to __mmc_start_request() instead and similar to my > upper comment, please add "retries" and "re-tune state" in the trace > print. Make sense. Thanks for your comments. > >> + >> if (mrq->sbc) { >> pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n", >> mmc_hostname(host), mrq->sbc->opcode, >> > > [...] > > Kind regards > Uffe -- Baolin.wang Best Regards