Re: [PATCH v1] perf symbol: Lazily compute idle and use the perf_env

Ian Rogers
2026-03-25T15:58:18Z

On Tue, Mar 24, 2026 at 11:58 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> Sorry for the delay.
>
> On Tue, Mar 24, 2026 at 10:14:01AM -0700, Ian Rogers wrote:
> > On Mon, Mar 2, 2026 at 3:43 PM Ian Rogers <irogers@google.com> wrote:
> [SNIP]
> > > - if (idle_symbols_list)
> > > - return strlist__has_entry(idle_symbols_list, name);
> > > + /*
> > > + * ppc64 uses function descriptors and appends a '.' to the
> > > + * start of every instruction address. Remove it.
> > > + */
> > > + if (name[0] == '.')
>
> Then e_machine == EM_PPC64 can be checked here.

Agreed, but potentially this is load bearing for more than just PPC so
I'd rather leave it as it is.

> > > + name++;
> > > +
> > > +
>
> Two blank lines.

Will fix in v2.

> > > + if (bsearch(name, idle_symbols, ARRAY_SIZE(idle_symbols),
> > > + sizeof(idle_symbols[0]), sym_name_cmp)) {
> > > + sym->idle = SYMBOL_IDLE__IDLE;
> > > + return true;
> > > + }
> > > +
> > > + if (e_machine == EM_386 || e_machine == EM_X86_64) {
> > > + if (strstarts(name, "mwait_idle") ||
> > > + strstarts(name, "intel_idle")) {
> > > + sym->idle = SYMBOL_IDLE__IDLE;
> > > + return true;
> > > + }
> > > + }
> > > +
> > > + if (e_machine == EM_PPC64 &&!strcmp(name, "ppc64_runlatch_off")) {
> > > + sym->idle = SYMBOL_IDLE__IDLE;
> > > + return true;
> > > + }
> > >
> > > - idle_symbols_list = strlist__new(NULL, NULL);
> > > + if (e_machine == EM_S390) {
> > > + int major = 0, minor = 0;
> > > + const char *release = env && env->os_release
> > > + ? env->os_release : perf_version_string;
> > >
> > > - for (i = 0; idle_symbols[i]; i++)
> > > - strlist__add(idle_symbols_list, idle_symbols[i]);
> > > + sscanf(release, "%d.%d", &major, &minor);
> > >
> > > - return strlist__has_entry(idle_symbols_list, name);
> > > + /* Before v6.10, s390 used psw_idle. */
> > > + if ((major < 6 || (major == 6 && minor < 10)) && strstarts(name, "psw_idle")) {
> > > + sym->idle = SYMBOL_IDLE__IDLE;
> > > + return true;
> > > + }
> > > + }
> > > +
> > > + sym->idle = SYMBOL_IDLE__NOT_IDLE;
> > > + return false;
> > > }
> > >
> > > static int map__process_kallsym_symbol(void *arg, const char *name,
> > > @@ -785,7 +815,7 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
> > > * We will pass the symbols to the filter later, in
> > > * map__split_kallsyms, when we have split the maps per module
> > > */
> > > - __symbols__insert(root, sym, !strchr(name, '['));
> > > + __symbols__insert(root, sym);
> > >
> > > return 0;
> > > }
> > > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > > index 3fb5d146d9b1..508dd9f336e9 100644
> > > --- a/tools/perf/util/symbol.h
> > > +++ b/tools/perf/util/symbol.h
> > > @@ -24,6 +24,7 @@ struct dso;
> > > struct map;
> > > struct maps;
> > > struct option;
> > > +struct perf_env;
> > > struct build_id;
> > >
> > > /*
> > > @@ -41,6 +42,12 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
> > > GElf_Shdr *shp, const char *name, size_t *idx);
> > > #endif
> > >
> > > +enum symbol_idle_kind {
> > > + SYMBOL_IDLE__UNKNOWN = 0,
> > > + SYMBOL_IDLE__NOT_IDLE = 1,
> > > + SYMBOL_IDLE__IDLE = 2,
> > > +};
> > > +
> > > /**
> > > * A symtab entry. When allocated this may be preceded by an annotation (see
> > > * symbol__annotation) and/or a browser_index (see symbol__browser_index).
> > > @@ -56,8 +63,8 @@ struct symbol {
> > > u8 type:4;
> > > /** ELF binding type as defined for st_info. E.g. STB_WEAK or STB_GLOBAL. */
> > > u8 binding:4;
> > > - /** Set true for kernel symbols of idle routines. */
> > > - u8 idle:1;
> > > + /** Cache for symbol__is_idle. */
> > > + enum symbol_idle_kind idle:2;
>
> I'm curious if bitfields with different types (u8 and enum) can be
> placed consecutively bitwise. There can be a lot of symbols so it
> could be a concern.

pahole says no size difference:

Before:
```
struct symbol {
struct rb_node rb_node
__attribute__((__aligned__(8))); /* 0 24 */
u64 start; /* 24 8 */
u64 end; /* 32 8 */
u16 namelen; /* 40 2 */
u8 type:4; /* 42: 0 1 */
u8 binding:4; /* 42: 4 1 */
u8 idle:1; /* 43: 0 1 */
u8 ignore:1; /* 43: 1 1 */
u8 inlined:1; /* 43: 2 1 */
u8 annotate2:1; /* 43: 3 1 */
u8 ifunc_alias:1; /* 43: 4 1 */

/* XXX 3 bits hole, try to pack */

u8 arch_sym; /* 44 1 */
char name[]; /* 45 0 */

/* size: 48, cachelines: 1, members: 13 */
/* sum members: 43 */
/* sum bitfield members: 13 bits, bit holes: 1, sum bit holes: 3 bits */
/* padding: 3 */
/* forced alignments: 1 */
/* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));
```

After:
```
struct symbol {
struct rb_node rb_node
__attribute__((__aligned__(8))); /* 0 24 */
u64 start; /* 24 8 */
u64 end; /* 32 8 */
u16 namelen; /* 40 2 */
u8 type:4; /* 42: 0 1 */
u8 binding:4; /* 42: 4 1 */

/* Bitfield combined with previous fields */

enum symbol_idle_kind idle:2; /* 40:24 4 */

/* Bitfield combined with next fields */

u8 ignore:1; /* 43: 2 1 */
u8 inlined:1; /* 43: 3 1 */
u8 annotate2:1; /* 43: 4 1 */
u8 ifunc_alias:1; /* 43: 5 1 */

/* XXX 2 bits hole, try to pack */

u8 arch_sym; /* 44 1 */
char name[]; /* 45 0 */

/* size: 48, cachelines: 1, members: 13 */
/* sum members: 43 */
/* sum bitfield members: 14 bits, bit holes: 1, sum bit holes: 2 bits */
/* padding: 3 */
/* forced alignments: 1 */
/* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));
```

Thanks,
Ian

> Thanks,
> Namhyung
>
>
> > > /** Resolvable but tools ignore it (e.g. idle routines). */
> > > u8 ignore:1;
> > > /** Symbol for an inlined function. */
> > > @@ -184,8 +191,7 @@ int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss);
> > >
> > > char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name);
> > >
> > > -void __symbols__insert(struct rb_root_cached *symbols, struct symbol *sym,
> > > - bool kernel);
> > > +void __symbols__insert(struct rb_root_cached *symbols, struct symbol *sym);
> > > void symbols__insert(struct rb_root_cached *symbols, struct symbol *sym);
> > > void symbols__fixup_duplicate(struct rb_root_cached *symbols);
> > > void symbols__fixup_end(struct rb_root_cached *symbols, bool is_kallsyms);
> > > @@ -269,5 +275,6 @@ enum {
> > > };
> > >
> > > int symbol__validate_sym_arguments(void);
> > > +bool symbol__is_idle(struct symbol *sym, const struct dso *dso, const struct perf_env *env);
> > >
> > > #endif /* __PERF_SYMBOL */
> > > --
> > > 2.53.0.473.g4a7958ca14-goog
> > >