modpost: vmlinux: section mismatch in reference: __list_add (section: .text.unlikely.) -> dir_list (section: .init.data)

[nickdesaulniers]

Both Android and ChromeOS are hitting a modpost warning when enabling AutoFDO. They appear subtly different, but seem to be referring to the same (or similar) callers/callees:

Android:

WARNING: modpost: vmlinux: section mismatch in reference: list_add+0x0 (section: .text.hot.list_add) -> dir_list (section: .init.data)

ChromeOS

WARNING: modpost: vmlinux.o: section mismatch in reference: __list_add (section: .text.unlikely.) -> dir_list (section: .init.data)

Note: one is list_add (no __ prefix), one is __list_add. Both refer to dir_list. I recall there's some what color is your function stuff going on with __init, but forget how we've fixed these previously. It looks like dir_list is defined in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/init/initramfs.c#n155.

I suspect that the call to list_add from dir_list is being specialized for &dir_list, since dir_list is static __initdata, but the specialization isn't being placed in __init.

Thanks to @ajordanr-google for the report. Jordan can provide AutoFDO profiles (if necessary, but I think we can come up with a fix without them) and help test prospective fixes.

[nathanchance]

Do these go away if list_add() and __list_add() are marked as __always_inline instead of just inline?

[ajordanr-google]

I've confirmed this can be fixed with:

diff --git a/include/linux/list.h b/include/linux/list.h
index 164b4d0e9d2a..272ce46ff289 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -143,9 +143,9 @@ 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!
  */
-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;
@@ -164,7 +164,7 @@ static inline void __list_add(struct list_head *new,
  * Insert a new entry after the specified head.
  * This is good for implementing stacks.
  */
-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);
 }

But this is overly aggressive. It seems however that the kernel expects this to always be inlined? the modpost check can never work if it's not inlined.

Do we just want to do that?

[nickdesaulniers]

the modpost check can never work if it's not inlined.

Note: list_add is called throughout the kernel, from __init and non-__init functions alike. This specific issue occurs when constant propagation specializes the call to list_add in init/initramfs.c (because there are no other call sites of this static inline functions, and one of the parameters is a constant, so we can constant propagate it into list_add then delete the corresponding parameter and call frame setup.

The downside to marking list_add and __list_add always_inline is I would expect non-trivial code size increases of .text in vmlinux; but perhaps it's negligible. Would be good to measure via bloaty.

[nathanchance]

Eric Dumazet happened to notice list_add() not being inlined and did some measurements, which seemed to indicate it would be more beneficial to force inlining but I am not sure how many different configurations he tested with:

https://lore.kernel.org/CANn89iJVQe=wedLheJmjZjOTJsWHijT0jZs=iRxKssJZbjAxHw@mail.gmail.com/

[nickdesaulniers]

I suspect maybe you'd want to do something differently for CONFIG_LIST_HARDENED; the always_inline on __list_add_valid looks concerning. I suspect maybe list_add, and __list_add should be always_inline, while the always_inline on __list_add_valid (when CONFIG_LIST_HARDENED is set) should be made no_inline). (probably same for __list_del_entry_valid wrt the same config).

[nickdesaulniers]

@ajordanr-google is ChromeOS setting CONFIG_LIST_HARDENED or CONFIG_DEBUG_LIST in its .config for these kernel builds, by chance?