Re: [PATCH 0/4] coresight: Add ETR-PERF polling.

From: Denis Nikitin <denik@google.com>
To: Leo Yan <leo.yan@linaro.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>,
	Mike Leach <mike.leach@linaro.org>,
	 "coresight@lists.linaro.org" <coresight@lists.linaro.org>,
	 "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Daniel Kiss <Daniel.Kiss@arm.com>
Subject: Re: [PATCH 0/4] coresight: Add ETR-PERF polling.
Date: Thu, 27 May 2021 00:50:28 -0700	[thread overview]
Message-ID: <CAOYpmdGgiZ=QqBkFAQhaEqq2Bd2XbG1SUFFYqqmKfYJcsQatkg@mail.gmail.com> (raw)
In-Reply-To: <20210523084552.GA23254@leoy-ThinkPad-X240s>

Hi Leo,

On Sun, May 23, 2021 at 1:45 AM Leo Yan <leo.yan@linaro.org> wrote:
>
> On Fri, May 14, 2021 at 02:02:25AM -0700, Denis Nikitin wrote:
>
> [...]
>
> > We could probably utilize the ETM strobing feature and reduce frequency
> > of data collection but I see a problem when I'm using both.
> > Within a minute of profiling the ETM generates a reasonable profile size
> > (with strobing autofdo,preset=9 with period 0x1000 it is up to 20MB).
> > But then the size grows unproportionally.
> > With a 4 minute run I got a 6.3GB profile.
> > I don't see such a problem with the ETR polling patch.
>
> I found there have a potential bug in the perf tool for calculation the
> buffer size for snapshot mode.
>
> In the function cs_etm_find_snapshot of perf code [1], after the "head"
> has wrapped around, it always return the "old" and "head" with the
> difference "mm->len", that means the trace data will be copied with
> length "mm->len".  If the buffer size is 4MB, it always copies trace
> data with 4MB for every time.
>
> This is incorrect for the snapshot with very small interval, even after
> wrapped around, it still have chance to only generate very small amount
> trace data (e.g. for 10ms), so I think we should fix this code like:
>
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index d942f118d32c..8a60e65c6651 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -849,6 +849,13 @@ static int cs_etm_find_snapshot(struct auxtrace_record *itr,
>         if (!wrapped)
>                 return 0;
>
> +       /*
> +        * If the difference between the head and old is less than mm->len,
> +        * it means the new trace data is small so doesn't need ajust them.
> +        */
> +       if (*head - *old < mm->len)
> +               return 0;
> +
>         /*
>          * *head has wrapped around - adjust *head and *old to pickup the
>          * entire content of the AUX buffer.
>
> I will do more testing and send formal patch for this.

Thanks for looking into this problem.
Initially I said that the issue with the excessive profile size was
not reproducible
when I applied https://lists.linaro.org/pipermail/coresight/2021-April/006254.html.
But after a couple of runs the issue came back.
Reproducibility depends on the system workload and it happens more often if
we run any workload.

I tried the fix you shared and so far I don't see the issue anymore.

Thanks,
Denis

>
> Thanks,
> Leo
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/arm/util/cs-etm.c#n853

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel