Re: [PATCH V5 3/3] perf tool: check buildid for symoff

[Jiri Olsa <jolsa@redhat.com>]

On Thu, Nov 27, 2014 at 02:09:51PM +0000, Liang, Kan wrote:
> 
> 
> > Hi Kan,
> > 
> > On Mon, 24 Nov 2014 11:00:29 -0500, Kan Liang wrote:
> > > From: Kan Liang <kan.liang@intel.com>
> > >
> > > symoff can support both same binaries and different binaries. However,
> > > the offset may be changed for different binaries. This patch checks
> > > the buildid of perf.data. If they are from different binaries, print a
> > > warning to notify the user.
> > 
> > Hmm.. I think that perf diff is supposed to compare performance between
> > different (i.e. modified) binaries.  So there's a little point to print the
> > warning IMHO - but I'm not insist it strongly..
> > 
> > Anyway, I think what we really need for the warning is different version of
> > same binary.  For example, if data file 1 has DSO A and B, and data file 2 has
> > DSO B and C, we should not consider they're different (unless build-ids of B
> > in data file 1 and 2 are different) since A and C won't affect symoff
> > comparision.

that sounds good to me

> >
> 
> It looks good.
> But I still slightly prefer to warn/inform the user if there are any different dsos,
> not just from common part. But it's not a strong option.
> I'd like to hear from others.
> 
> Arnaldo? Jirka?

sorry for late reply.. anyway like I said in the other email

---
IMO one (WARN_ONCE style) warning by default if we see
buildids discrepancy and detailed comparison for --verbose
---

I think the breakage of the check (that Namhyung described) could
be mentioned/labeled somehow as serious issue in the warning and
we could also 'inform' about "any different dsos" as you mentioned

jirka

> 
> Thanks,
> Kan
>  
> > Thanks,
> > Namhyung