Re: [PATCH] Mark list_add and __list_add as __always_inline
Nick Desaulniers <[email protected]> Fri, 31 Jul 2026 13:29:03 -0700
| Newsgroups | dev.linux.lists.llvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAKwvOd=6xCKkLLLmSXz1S95jOjRSDGyQbUwp5=yNJeau5q9eJQ@mail.gmail.com> |
On Fri, Jul 31, 2026 at 1:15=E2=80=AFPM Jordan R Abrahams-Whitehead <[email protected]> wrote: > > This commit resolves an issue where modpost section > verification fails due to section mismatches between list_add > and its callers. > > At present, list_add (and its internal __list_add) are called from > both .text and .init code sections. Since inlining can vary per call > site, list_add can be 4 different states: > > list_add in text with arguments to non-.init.data values > list_add in init with arguments to static .init.data values > list_add in init with arguments to non-.init.data values > list_add in text with arguments to static .init.data values > > It is last instance that ends up causing the section mismatch caused by > constant propagation of the address of static libs inside the `dir_add` I think you meant `static __initdata LIST_HEADs`? (not `static libs`). Maybe AKPM could fix that up for you when applying? > as seen below (with the dir_list being defined statically in > initramfs.c, resting in .init.data). > > WARNING: modpost: vmlinux.o: section mismatch in reference: __list_add > (section: .text.unlikely.) -> dir_list (section: .init.data) > > Because of these section matching requirements, semantically, > __list_add and list_add MUST be inlined. This will then ensure > callers inside .init will receive a list_add that exists and refers > to only .init data, and list_add code in .text sections will only refer > to non-init data. > > This issue manifests predominently in AutoFDO with clang, which is very > hesitant to inline cold functions such as list_add even when marked > `inline`. Marking them as `__always_inline` therefore matches the > existing semantic constraints imposed by modpost's section mismatch > checks. > Reviewed-by: Nick Desaulniers <[email protected]> Tested-by: Nick Desaulniers <[email protected]> We've been seeing this particularly on Android and ChromeOS while trying to enable AutoFDO, which seems more likely to outline cold code. include/linux/init.h defines __init as __cold. I saw no change in size for x86 defconfig; enabling CONFIG_LIST_HARDENED resulted in a change of a few bytes (i.e. less that 0.01%). I assume we don't care as much about the size if CONFIG_DEBUG_LIST is set. Checked with bloaty and scripts/bloat-o-meter (thanks to Eric for pointing out that tool in the linked thread). I never heard back from Kees or Marco about their thoughts on this in relation to commit b16c42c8fde8 ("list_debug: Introduce inline wrappers for debug check= s") but I wouldn't mind an Ack (or Nack) from at least one of them. > Closes: https://github.com/ClangBuiltLinux/linux/issues/2173 > Signed-off-by: Jordan R Abrahams-Whitehead <[email protected]> > Suggested-by: Nathan Chancellor <[email protected]> > Suggested-by: Eric Dumazet <[email protected]> > Link: https://lore.kernel.org/all/CANn89iJVQe=3DwedLheJmjZjOTJsWHijT0jZs= [email protected]/ > --- > include/linux/list.h | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/include/linux/list.h b/include/linux/list.h > index 09d979976b3b..59f8aa0905d3 100644 > --- a/include/linux/list.h > +++ b/include/linux/list.h > @@ -150,10 +150,13 @@ static inline bool __list_del_entry_valid(struct li= st_head *entry) > * > * This is only for internal list manipulation where we know > * the prev/next entries already! > + * > + * Must be inlined to ensure it can be safely called > + * with initdata arguments. > */ > -static inline void __list_add(struct list_head *new, > - struct list_head *prev, > - struct list_head *next) > +static __always_inline void __list_add(struct list_head *new, > + struct list_head *prev, > + struct list_head *next) > { > if (!__list_add_valid(new, prev, next)) > return; > @@ -171,8 +174,12 @@ static inline void __list_add(struct list_head *new, > * > * Insert a new entry after the specified head. > * This is good for implementing stacks. > + * > + * Must be inlined to ensure it can be safely called > + * with initdata arguments. > */ > -static inline void list_add(struct list_head *new, struct list_head *hea= d) > +static __always_inline void list_add(struct list_head *new, > + struct list_head *head) > { > __list_add(new, head, head->next); > } > > --- > base-commit: fc46aed51f6280801f43a2cf4b5060cc33b572f9 > change-id: 20260730-always-inline-list-add-363639606e26 > > Best regards, > -- > Jordan R Abrahams-Whitehead <[email protected]> > --=20 Thanks, ~Nick Desaulniers