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

[Liang, Kan]

> 
> 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@xxxxxxxxx>
> > > >
> > > > 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
> 

I c. Thanks Namhyung and Jirka's suggestion.
So for next version, only the common part of the dsos will be checked
and warned once.
For the details of dsos, the user can apply --verbose to get a full list of dsos.
I will also change the man menu for diff accordingly as below.
"--verbose:: Be verbose, for instance, show the raw counts and all build-ids
in addition to the diff"

Thanks,
Kan

> jirka
> 
> >
> > Thanks,
> > Kan
> >
> > > Thanks,
> > > Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/