Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs

From: Mike Leach <mike.leach@linaro.org>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	James Clark <james.clark@arm.com>,
	 Coresight ML <coresight@lists.linaro.org>,
	linux-perf-users@vger.kernel.org,  Leo Yan <leo.yan@linaro.org>,
	"Suzuki K. Poulose" <suzuki.poulose@arm.com>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>,  Namhyung Kim <namhyung@kernel.org>,
	John Garry <john.garry@huawei.com>,
	 Will Deacon <will@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	 linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
Date: Tue, 3 Aug 2021 09:06:17 +0100	[thread overview]
Message-ID: <CAJ9a7VhLqgsEOVV3yksLiTJ2w2FOogNS92-gX5G4sgL8wiEwCA@mail.gmail.com> (raw)
In-Reply-To: <20210802170307.GA3050918@p14s>

HI James,

On Mon, 2 Aug 2021 at 18:03, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
> On Mon, Aug 02, 2021 at 11:51:58AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark escreveu:
> > > Currently decode will silently fail if no binary data is available for
> > > the decode. This is made worse if only partial data is available because
> > > the decode will appear to work, but any trace from that missing DSO will
> > > silently not be generated.
> > >

The decoder actually outputs a OCSD_GEN_TRC_ELEM_ADDR_NACC packet if
binary memory data is missing.
These packets are currently ignored by perf / cs-etm-decoder.c.

I think this per DSO warning is fine, but perhaps at some point add in
handling for OCSD_GEN_TRC_ELEM_ADDR_NACC - which perhaps is only
active when dumping trace packets.

Regards

Mike

> > > Add a UI popup once if there is any data missing, and then warn in the
> > > bottom left for each individual DSO that's missing.
> >
> > Looks ok to me (the last 3 patches in this series, the rest I applied
> > already), can I get some Acked-by/Reviewed-by from the CoreSight people?
> >
>
> I have a substantial backlog of patches to go through and I will be away for the
> next two weeks.  As such it will be some time before I get to review this set.
>
> Regards,
> Mathieu
>
> > Thanks,
> >
> > - Arnaldo
> >
> > > Signed-off-by: James Clark <james.clark@arm.com>
> > > ---
> > >  tools/perf/util/cs-etm.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index 32ad92d3e454..e6851260d059 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
> > >
> > >     len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
> > >
> > > -   if (len <= 0)
> > > +   if (len <= 0) {
> > > +           ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> > > +           if (!al.map->dso->auxtrace_warned) {
> > > +                   pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> > > +                               address,
> > > +                               al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> > > +                   al.map->dso->auxtrace_warned = true;
> > > +           }
> > >             return 0;
> > > +   }
> > >
> > >     return len;
> > >  }
> > > --
> > > 2.28.0
> > >
> >
> > --
> >
> > - Arnaldo

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

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