Re: [PATCH] Mark list_add and __list_add as __always_inline

[Jordan Abrahams-Whitehead]

On Fri, Jul 31, 2026 at 1:36 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> 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?
> > >

Apologies, this is a typo, yes. I think I missed it when re-flowing the text
(and also the slightly janky phrasing here).
Should have been something more along the lines of:

It is this last instance that ends up causing the section mismatch
shown below due to constant propagation of the address of the
static __initdata dir_list head inside of `dir_add`.

The processes for the kernel mailing list are unfamiliar to me,
so uncertain if this typo is worthy of sending a new patch with
an updated message.

> > > > 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>
>

Good to call out! Thank you.


> > >
> > > 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