On 28 March 2016 at 08:39, Baolin Wang <baolin.wang@xxxxxxxxxx> 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 <baolin.wang@xxxxxxxxxx>
> ---
> 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 <linux/mmc/sd.h>
> #include <linux/mmc/slot-gpio.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/mmc.h>
> +
> #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.
> if (mrq->sbc) {
> pr_debug("%s: req done <CMD%u>: %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.
> +
> if (mrq->sbc) {
> pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n",
> mmc_hostname(host), mrq->sbc->opcode,
>
[...]
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
