Re: [PATCH] Mark list_add and __list_add as __always_inline

[Nick Desaulniers]

On Fri, Jul 31, 2026 at 1:30 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> On Fri, Jul 31, 2026 at 1:29 PM Nick Desaulniers
> <ndesaulniers@xxxxxxxxxx> wrote:
> >
> > On Fri, Jul 31, 2026 at 1:15 PM Jordan R Abrahams-Whitehead
> > <ajordanr@xxxxxxxxxx> 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 <ndesaulniers@xxxxxxxxxx>
> > Tested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>

I can also find internal reports about this from two other folks;
worth attributing on the fix IMO.

Reported-by: Giuliano Procida <gprocida@xxxxxxxxxx>
Reported-by: Yabin Cui <yabinc@xxxxxxxxxx>

> >
> > 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 checks")
> > 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 <ajordanr@xxxxxxxxxx>
> > > Suggested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > > Suggested-by: Eric Dumazet <edumazet@xxxxxxxxxx>
> > > Link: https://lore.kernel.org/all/CANn89iJVQe=wedLheJmjZjOTJsWHijT0jZs=iRxKssJZbjAxHw@xxxxxxxxxxxxxx/
>
> Also, I think we might want this applied with:
>
> Cc: stable@xxxxxxxxxxxxxxx
>
> 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 *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 *head)
> > > +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 <ajordanr@xxxxxxxxxx>
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers