Re: [PATCH] Mark list_add and __list_add as __always_inline

On Fri, Jul 31, 2026 at 1:29=E2=80=AFPM Nick Desaulniers
<[email protected]> wrote:
>
> 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_ad=
d
> >   (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 che=
cks")
> 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=3DwedLheJmjZjOTJsWHijT0jZ=
[email protected]/

Also, I think we might want this applied with:

Cc: [email protected]

since I believe CrOS saw issues as far back as 6.1.

> > ---
> >  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 =
list_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 *ne=
w,
> >   *
> >   * 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 *h=
ead)
> > +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]>
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



--=20
Thanks,
~Nick Desaulniers