Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230608084407.140323-1-asavkov@redhat.com>
In-Reply-To: <20230608084407.140323-1-asavkov@redhat.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Thu, 8 Jun 2023 14:09:06 -0700
Message-ID: <CAM9d7cgvtuqMeQx8ehLCLw_Ur9Ju-VzDrakzFJxnxvFjdsWk_g@mail.gmail.com>
Subject: Re: [PATCH v2 0/2] perf tools: annotation browser from c2c tui
To:     Artem Savkov <asavkov@redhat.com>
Cc:     linux-perf-users@vger.kernel.org,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hello,

On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <asavkov@redhat.com> wrote:
>
> These patches add ability to start annotation browser from c2c report
> tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].

I was thinking about how it works and realized that it didn't collect
samples by symbol.  Then I'm not sure if the result is meaningful.
I think it'd show a random symbol that touched the cache line
first.  The same cache line can be accessed from other locations
but it cannot know where they are.

Also different instructions in a function (symbol) would access a
different cache line.  The annotate output just shows any memory
access.  So it might be good to check the instruction at the point
but others should not be considered related.

Hmm.. I suspect even the same instruction will hit the different
cache lines at different times.  Then probably the annotation
won't work well in terms of correlating cache lines.

Thanks,
Namhyung

>
> [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
>
> v1->v2: Addressed comments from Namhyung Kim
> - No longer saving evsel for each hist entry, using evlist__first
>   instead.
> - Factored out preparations to call annotation browser to do_annotate()
>   function
> - Other small fixes and adjustments.
>
> Artem Savkov (2):
>   perf util: move symbol__new_unresolved() to util/symbol.c
>   perf tools: allow running annotation browser from c2c-report
>
>  tools/perf/builtin-c2c.c       | 73 +++++++++++++++++++++++++++++++---
>  tools/perf/ui/browsers/hists.c | 22 ----------
>  tools/perf/util/symbol.c       | 22 ++++++++++
>  tools/perf/util/symbol.h       |  1 +
>  4 files changed, 91 insertions(+), 27 deletions(-)
>
> --
> 2.40.1
>