Re: Re: [perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Feb 12, 2015 at 03:36:58PM +0900, Masami Hiramatsu escreveu:
> (2015/02/11 23:57), Arnaldo Carvalho de Melo wrote:
> > Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:
> >>> Add new buildid cache if the update target file is not cached.
> >>> This can happen when an old binary is replaced by new one
> >>> after caching the old one. In this case, user sees his operation
> >>> just failed. But it does not look straight, since user just
> >>> pass the binary "path", not "build-id".
> >>>
> >>>   ----
> >>>   # ./perf buildid-cache --add ./perf
> >>>   (update ./perf to new binary)
> >>>   # ./perf buildid-cache --update ./perf
> >>>   ./perf wasn't in the cache
> >>
> >> Humm, without re-reading the original motivation for the '--update'
> >> operation I would think it was about finding all build-ids in the cache
> >> that are for a binary with that path, remove them and insert this new
> >> one, no?
> >>
> >> Checking...
> > 
> > commit eeb49845425375481f14c0e5721f88242642e88e
> > Author: Namhyung Kim <namhyung.kim@lge.com>
> > Date:   Thu Feb 7 18:02:11 2013 +0900
> > 
> >     perf buildid-cache: Add --update option
> >     
> >     When adding vmlinux file to build-id cache, it'd be fail since kallsyms
> >     dso with a same build-id was already added by perf record.
> >     
> >     So one needs to remove the kallsyms first to add vmlinux into the cache.
> >     Add --update option for doing it at once.
> > 
> > ----------------------------------------------------------------------------------
> > 
> > So this was really a 'remove the file that is pointed by this build-id' and
> > replace with this new file, i.e. there is both a vmlinux _and_ a kallsyms file
> > for the same build-id. When wanting to use one or the other and existing a
> > link in the cache, one uses this --update thing.

> > That is ok for cases where there are multiple symtabs in the cache for a given
> > build-id, but as you mention above, it is confusing for updating by _pathname_.

> > What a 'update' by pathname would mean? I guess it would be to remove 'old',
> > i.e. 'not up-to-date' stuff, i.e. older build-ids that that pathname had in the
> > past, and leave just this new stuff.
 
> > The way you did it make '--update path' be equivalent to '--add path'.
 
> Right, if there is no same buildid cache, it does that.
> Actually, this is a kind of middle point solution. As far as I can see,
> buildid-cache is (currently) used for annotating the old, or remote perf.data.
> In that case, we may have several different versions of buildid-cache(binaries)
> for one pathname. If --update destructively updates all caches related to
> given pathname, it also means to loose annotatability(?) on old or remote
> perf.data. I think that may not the user wants.

> So, if user really wants to clean up their cache, he/she can use --purge
> (--remove-all).

Ok, I thought about this, i.e. if you want to remove older stuff, purge
it.

Ah, at some point it would be nice to have --purge receive a parameter,
telling that things that are older/newer/within some range should be
removed, but sure, that can be left for later 8-)

> And I think it would be better that --update just adds given current binary
> to cache if there is no cache.

Agreed, and it would be really nice to have parts of this conversation,
edited, available on tools/perf/Documentation/perf-buildid--cache.txt
:-)

- Arnaldo