Re: [PATCH v6 4/5] perf script: Add --lazy-load-symbols to load symbol names lazily

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Nov 13, 2026 at 08:00:55PM -0800, Alireza Haghdoost wrote:
> On Sat, Oct 31, 2026 at 6:58 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Fri, Oct 30, 2026 at 08:55:55AM -0700, Ian Rogers wrote:
> > > I'm thinking this option should really be the default, requiring
> > > people to opt out.
> >
> > Agreed on making it default.  I'm not sure what's the case that needs
> > eager loading.  We can change the code to load all symbols just before
> > it (oh, it seems the patch already handles the cases) then no option
> > would be needed. :)
> 
> Sorry for the slow reply. I wanted to have v7 ready before answering.
> I agree that lazy loading should become the default, but I would like
> to keep it opt-in in v7 and flip it in a separate patch later.
> 
> While checking what default-on would change, I found that a binary
> replaced during the run gave wrong names. Lazy loading reads a name
> later, at the offset the symbol table had at load time. If a deploy
> replaces the binary in between, or the path resolves to another file
> because the process of a mount namespace exited, perf read the names
> from the new file. I reproduced it by swapping the binary under gdb;
> perf printed names of the other build. Eager loading reads every name
> at load time, so it is not affected. I fixed that in v7 and kept the
> device, inode,
> size and modification time of the file and checks them whenever it opens
> the file again. If they changed, perf warns once and shows the
> remaining symbols of that DSO as [unknown].

We have struct dso_id to identify the binary.  The build-ID is the
preferred method to check it.

> 
> Also, each batch of names opens the file again, resolves its name and
> enters the mount namespace. On my test profile this is cheap (1.3 s
> lazy vs 2.9 s eager, identical output), but I have not measured it for
> containers or debuglink layouts. The type-aware data cache that we
> discussed on v5 would remove most of that cost.

Right, having a fd in the cache, would solve many problems.

> 
> So my plan is to run lazy loading in our continuous profiling at scale
> first, and then send a patch that makes it the default. By that time,
> hopefully we will have already added the type-aware data cache.
> Does that work for you?

Sounds good.  A real world testing would be great.  I'll find time to
work on the DSO access.

> 
> > > I'm curious if this is global just to avoid the expense of a mutex per
> > > symbol or it needs to be global? [...] but perhaps the dso__lock works?
> >
> > Probably.  dso__load() already takes the dso lock.
> 
> It was global because dso__named_symbols() and dso__sort_by_name()
> already hold the DSO lock when they load names, and the mutex is not
> recursive. Nothing that holds the DSO lock looks up symbols, so v7
> uses it: a lookup takes it only when the symbol it returns still needs
> its name, and the batch loader requires it held.
> 
> > Nit: I'm hesitate to argue about naming, but it seems a bit confusing.
> > Why does it skip to load when it doesn't have a (lazy) name?  Probably
> > symbol__needs_lazy_name() would be a little better...?
> 
> Agreed, renamed in v7.

Thanks,
Namhyung