On Thu, Apr 23, 2026 at 05:14:05PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 23, 2026 at 08:12:07AM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 23, 2026 at 10:38:49AM +0200, Peter Zijlstra wrote:
> > > On Wed, Apr 22, 2026 at 09:04:00PM -0700, Josh Poimboeuf wrote:
> > > 
> > > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > > > index 00c2389f345f..8a6e1338af97 100644
> > > > --- a/tools/objtool/elf.c
> > > > +++ b/tools/objtool/elf.c
> > > > @@ -586,8 +586,11 @@ static int elf_add_symbol(struct elf *elf, struct 
> > > > symbol *sym)
> > > >         if (strstarts(sym->name, ".klp.sym"))
> > > >                 sym->klp = 1;
> > > >  
> > > > +       sym->pfunc = sym->cfunc = sym;
> > > > +
> > > >         if (!sym->klp && !is_sec_sym(sym) && strstr(sym->name, 
> > > > ".cold")) {
> > > > -               sym->cold = 1;
> > > > +               /* Tell read_symbols() this is a cold subfunction */
> > > > +               sym->pfunc = NULL;
> > > >  
> > > >                 /*
> > > >                  * Clang doesn't mark cold subfunctions as STT_FUNC, 
> > > > which
> > > > @@ -596,8 +599,6 @@ static int elf_add_symbol(struct elf *elf, struct 
> > > > symbol *sym)
> > > >                 sym->type = STT_FUNC;
> > > >         }
> > > >  
> > > > -       sym->pfunc = sym->cfunc = sym;
> > > > -
> > > >         return 0;
> > > >  }
> > > 
> > > So now the cold subfunction has a NULL parent-function and a
> > > child-function that points to the parent?
> > > 
> > > I'm confused.
> > 
> > It's a bit clunky.  As the comment implies, 'sym->pfunc = NULL' is a
> > signal to it caller read_symbols() that this is a .cold function.  Then,
> > after all the symbols have been added, read_symbols() goes and finds the
> > parent.
> > 
> > I think I did it this way because klp-diff.c calls elf_add_symbol() (via
> > elf_create_symbol()) and later needs to call is_cold_func() on it.  In
> > that case, even though the parent isn't set, it still works because
> > is_cold_func() returns true for sym->pfunc != sym;
> 
> I'm thinking this needs more comments if it stays like this. Is most
> confusing.

So we can just keep the 'cold' bit and keep the confusingness at its
current level :-)

From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] objtool: Add is_cold_func() helper

Add an is_cold_func() helper.  No functional changes intended.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
 tools/objtool/check.c               | 6 +++---
 tools/objtool/include/objtool/elf.h | 5 +++++
 tools/objtool/klp-diff.c            | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4c18d6e7f6c3..4ed27c53c718 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2614,7 +2614,7 @@ static void mark_holes(struct objtool_file *file)
                if (insn->jump_dest) {
                        struct symbol *dest_func = insn_func(insn->jump_dest);
 
-                       if (dest_func && dest_func->cold)
+                       if (dest_func && is_cold_func(dest_func))
                                dest_func->ignore = true;
                }
        }
@@ -4426,8 +4426,8 @@ static int create_prefix_symbol(struct objtool_file 
*file, struct symbol *func)
        char name[SYM_NAME_LEN];
        struct cfi_state *cfi;
 
-       if (!is_func_sym(func) || is_prefix_func(func) ||
-           func->cold || func->static_call_tramp)
+       if (!is_func_sym(func) || is_prefix_func(func) || is_cold_func(func) ||
+           func->static_call_tramp)
                return 0;
 
        if ((strlen(func->name) + sizeof("__pfx_") > SYM_NAME_LEN)) {
diff --git a/tools/objtool/include/objtool/elf.h 
b/tools/objtool/include/objtool/elf.h
index 3abe4cbc584c..ad0cc57a9d5f 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -289,6 +289,11 @@ static inline bool is_prefix_func(struct symbol *sym)
        return sym->prefix;
 }
 
+static inline bool is_cold_func(struct symbol *sym)
+{
+       return sym->cold;
+}
+
 static inline bool is_reloc_sec(struct section *sec)
 {
        return sec->sh.sh_type == SHT_RELA || sec->sh.sh_type == SHT_REL;
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index 1951a8b2df44..266f0d2ba4fe 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1718,7 +1718,8 @@ static int create_klp_sections(struct elfs *e)
                unsigned long sympos;
                void *func_data;
 
-               if (!is_func_sym(sym) || sym->cold || !sym->clone || 
!sym->clone->changed)
+               if (!is_func_sym(sym) || is_cold_func(sym) ||
+                   !sym->clone || !sym->clone->changed)
                        continue;
 
                /* allocate klp_func_ext */
-- 
2.53.0


Reply via email to