[PATCH v2 0/2] ftrace: Drop weak function locations of modules when loading them

[PATCH v2 0/2] ftrace: Drop weak function locations of modules when loading them

[Lawrence Lin via B4 Relay]

Loading a module calls test_for_valid_rec() for each of its ftrace
records, and each call scans the whole module symbol table. For amdgpu
that is 16821 records against about 67000 symbols, and it delays boot by
seconds on a small machine.

v1 kept the per-record check and made it a binary search. Steve replied
that the check could go if it no longer finds anything [1]. It still does
for modules [2], so this version takes the approach proposed there: as
commit ef378c3b8233 ("scripts/sorttable: Zero out weak functions in
mcount_loc table") does for vmlinux at build time, the weak function
locations of a module are zeroed before its records are created, and
ftrace_module_enable() no longer checks each record. Patch 1 adds a
module helper that walks the symbols find_kallsyms_symbol() resolves to,
which also works while the module is still unformed. Patch 2 uses it in
ftrace_process_locs().

This replaces the v2 announced in [3], which only folded
ftrace_cmp_addr() into ftrace_cmp_ips(); that function is no longer
needed. A sorted symbol index for all module lookups, as David asked
about [4], is not needed for this either.

On a Ryzen 3 3200U (x86_64, v7.2.5, three boots each; a new set of boots,
so v1 differs slightly from the numbers in [3]):

                                  unpatched      v1      v2
  amdgpu initialized at              6.19 s  2.03 s  2.00 s
  kernel boot (systemd-analyze)      6.65 s  2.48 s  2.46 s
  modprobe radeon, median of 5       137 ms   70 ms   64 ms
  modprobe nouveau, median of 5      457 ms  125 ms  118 ms
  __ftrace_invalid_address___            18      18       0

Of the 6484 modules of that x86_64 distribution build, only kvm.ko has
weak function locations (18 of 249503 locations in total); the same holds
for ppc64le_defconfig (kvm.ko, 4 with clang and 3 with gcc). Other than
those entries, available_filter_functions is unchanged.

The rule differs slightly from test_for_valid_rec(): a location is kept
when any symbol of the module lies at most FTRACE_MCOUNT_MAX_OFFSET before
it, without checking that the symbol is in the same module memory region.
The two can only differ when a symbol of another memory region lies that
close before a location, that is, when two regions are at most
FTRACE_MCOUNT_MAX_OFFSET bytes apart.

Tested on x86_64 with IBT, v7.3-rc5 with and without the series, under
virtme-ng: the ftrace selftests (157 passed, 0 failed, the same
unresolved, unsupported and xfail cases on both), all eight livepatch
selftests, and loading kvm_amd (1502 kvm records before, 1484 after, none
of them invalid). Built at W=1 without warnings for ppc64le (clang with
patchable function entry and out-of-line stubs, gcc with
MPROFILE_KERNEL), ppc32 (pmac32), arm64, which does not define
FTRACE_MCOUNT_MAX_OFFSET, and x86_64 without modules. powerpc is build
tested only.

[1] https://lore.kernel.org/all/20261004050904.06a5ecab@fedora/
[2] https://lore.kernel.org/all/20261004170600.1541723-1-deduce@gmail.com/
[3] https://lore.kernel.org/all/20261004030438.434327-1-deduce@gmail.com/
[4] https://lore.kernel.org/all/20261004100030.189b1c3d@pumpkin/

---
Changes in v2:
- Zero weak function locations of modules before records are created,
  instead of looking every record up at load time (Steve).
- Add module_kallsyms_on_each_addr() to the module code (new patch 1)
  rather than reading the module symbol table from ftrace.
- Add the module maintainers.
- Link to v1: https://patch.msgid.link/20261003-ftrace-mod-bsearch-v1-1-92e2fd2d80ff@gmail.com

To: Luis Chamberlain <mcgrof@kernel.org>
To: Petr Pavlu <petr.pavlu@suse.com>
To: Daniel Gomez <da.gomez@kernel.org>
To: Sami Tolvanen <samitolvanen@google.com>
To: Aaron Tomlin <atomlin@atomlin.com>
To: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: linux-modules@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org

---
Lawrence Lin (2):
      module: Add module_kallsyms_on_each_addr()
      ftrace: Drop weak function locations of modules when loading them

 include/linux/module.h   | 10 ++++++
 kernel/module/kallsyms.c | 44 ++++++++++++++++++++------
 kernel/trace/ftrace.c    | 82 +++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 115 insertions(+), 21 deletions(-)
---
base-commit: e767a4ea70a3992c37ed604157d32f0dfbf9b1e3
change-id: 20261003-ftrace-mod-bsearch-534a86527ee5

Best regards,
--  
Lawrence Lin <deduce@gmail.com>

[PATCH v2 1/2] module: Add module_kallsyms_on_each_addr()

[Lawrence Lin via B4 Relay]

From: Lawrence Lin <deduce@gmail.com>

ftrace needs the addresses of the symbols of a module while the module is
still being loaded: ftrace_module_init() runs before complete_formation(),
while the module is MODULE_STATE_UNFORMED, and
module_kallsyms_on_each_symbol() skips unformed modules. It also needs
exactly the symbols that find_kallsyms_symbol() may resolve an address
to, so that it agrees with kallsyms_lookup().

Factor the symbol filter of find_kallsyms_symbol() into is_lookup_symbol()
and add module_kallsyms_on_each_addr(), which calls a function with the
address of each such symbol of a given module. Like find_kallsyms_symbol(),
it reads mod->kallsyms under RCU, which add_kallsyms() has set up by then.

No functional change to find_kallsyms_symbol().

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Lawrence Lin <deduce@gmail.com>
---
 include/linux/module.h   | 10 ++++++++++
 kernel/module/kallsyms.c | 44 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 96cc98568eea..8b1c06d1118c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -971,6 +971,10 @@ unsigned long module_kallsyms_lookup_name(const char *name);
 
 unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name);
 
+void module_kallsyms_on_each_addr(struct module *mod,
+				  void (*fn)(void *, unsigned long),
+				  void *data);
+
 #else	/* CONFIG_MODULES && CONFIG_KALLSYMS */
 
 static inline int module_kallsyms_on_each_symbol(const char *modname,
@@ -1014,6 +1018,12 @@ static inline unsigned long find_kallsyms_symbol_value(struct module *mod,
 	return 0;
 }
 
+static inline void module_kallsyms_on_each_addr(struct module *mod,
+						void (*fn)(void *, unsigned long),
+						void *data)
+{
+}
+
 #endif  /* CONFIG_MODULES && CONFIG_KALLSYMS */
 
 /* Define __free(module_put) macro for struct module *. */
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index f23126d804b2..bccee4b8294c 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -246,6 +246,18 @@ static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned
 	return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
 }
 
+/*
+ * Whether find_kallsyms_symbol() may resolve an address to symbol @symnum.
+ * Unnamed symbols are ignored: they're uninformative and inserted at a whim.
+ */
+static bool is_lookup_symbol(struct mod_kallsyms *kallsyms, unsigned int symnum)
+{
+	const char *name = kallsyms_symbol_name(kallsyms, symnum);
+
+	return kallsyms->symtab[symnum].st_shndx != SHN_UNDEF &&
+	       *name != '\0' && !is_mapping_symbol(name);
+}
+
 /*
  * Given a module and address, find the corresponding symbol and return its name
  * while providing its size and offset if needed.
@@ -286,15 +298,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
 		const Elf_Sym *sym = &kallsyms->symtab[i];
 		unsigned long thisval = kallsyms_symbol_value(sym);
 
-		if (sym->st_shndx == SHN_UNDEF)
-			continue;
-
-		/*
-		 * We ignore unnamed symbols: they're uninformative
-		 * and inserted at a whim.
-		 */
-		if (*kallsyms_symbol_name(kallsyms, i) == '\0' ||
-		    is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
+		if (!is_lookup_symbol(kallsyms, i))
 			continue;
 
 		if (thisval <= addr && thisval > bestval) {
@@ -458,6 +462,28 @@ unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
 	return __find_kallsyms_symbol_value(mod, name);
 }
 
+/*
+ * Call @fn with the address of each symbol of @mod that find_kallsyms_symbol()
+ * may resolve an address to. Unlike module_kallsyms_on_each_symbol(), this
+ * also works while @mod is still being loaded.
+ */
+void module_kallsyms_on_each_addr(struct module *mod,
+				  void (*fn)(void *, unsigned long),
+				  void *data)
+{
+	struct mod_kallsyms *kallsyms;
+	unsigned int i;
+
+	guard(rcu)();
+	kallsyms = rcu_dereference(mod->kallsyms);
+
+	/* ELF starts real symbols at 1. */
+	for (i = 1; i < kallsyms->num_symtab; i++) {
+		if (is_lookup_symbol(kallsyms, i))
+			fn(data, kallsyms_symbol_value(&kallsyms->symtab[i]));
+	}
+}
+
 int module_kallsyms_on_each_symbol(const char *modname,
 				   int (*fn)(void *, const char *, unsigned long),
 				   void *data)

-- 
2.55.0

[PATCH v2 2/2] ftrace: Drop weak function locations of modules when loading them

[Lawrence Lin via B4 Relay]

From: Lawrence Lin <deduce@gmail.com>

Since commit b39181f7c690 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid
adding weak function"), ftrace_module_enable() calls test_for_valid_rec()
for every ftrace record of a module being loaded. For a module address,
kallsyms_lookup() scans the whole symbol table of the module, so loading a
module costs O(records * symbols), all of it under ftrace_lock. amdgpu.ko
has 16821 records and about 67000 symbols, and commit 4099b98203d6
("ftrace: Fix softlockup in ftrace_module_enable") already had to add a
cond_resched() to this loop because of it.

Commit ef378c3b8233 ("scripts/sorttable: Zero out weak functions in
mcount_loc table") fixed vmlinux at build time, noting that the real
solution is to not add a weak function into the ftrace table in the first
place. Modules are not covered by it, and still have such locations: a
weak function in virt/kvm that arch/x86 overrides inside the same kvm.ko
keeps its mcount location but has no symbol. In an x86_64 distribution
build of v7.2.5, kvm.ko has 18 of them; none of the other 6483 modules has
any.

Do the same for modules when they are loaded. In ftrace_process_locs(),
after the locations are sorted, find for each symbol of the module, by
binary search, the locations at most FTRACE_MCOUNT_MAX_OFFSET after it,
and zero the locations no symbol marked. ftrace_process_locs() already
skips zeroed locations, so no record is created for them, and
ftrace_module_enable() no longer has to test every record. This costs one
bit per location, about 2 KB for amdgpu, and O(symbols * log(records))
time.

On a Ryzen 3 3200U (x86_64, v7.2.5, amdgpu loaded from the initramfs,
three boots each), amdgpu finishes initializing 6.19 s into boot without
this patch and 2.00 s with it, and the kernel part of boot reported by
systemd-analyze drops from 6.65 s to 2.46 s. Loading radeon and nouveau,
which have no hardware on that machine, goes from 137 ms to 64 ms and from
457 ms to 118 ms. available_filter_functions loses the 18
__ftrace_invalid_address___ entries of kvm; its module entries are
otherwise unchanged.

Fixes: b39181f7c690 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Lawrence Lin <deduce@gmail.com>
---
 kernel/trace/ftrace.c | 82 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 70 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 673a54fdf392..2e1a237ce901 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4445,11 +4445,6 @@ static int print_rec(struct seq_file *m, unsigned long ip)
 	return ret == NULL ? -1 : 0;
 }
 #else
-static inline int test_for_valid_rec(struct dyn_ftrace *rec)
-{
-	return 1;
-}
-
 static inline int print_rec(struct seq_file *m, unsigned long ip)
 {
 	seq_printf(m, "%ps", (void *)ip);
@@ -7611,6 +7606,73 @@ static void test_is_sorted(unsigned long *start, unsigned long count)
 }
 #endif
 
+#ifdef FTRACE_MCOUNT_MAX_OFFSET
+struct ftrace_mod_locs {
+	unsigned long *start;
+	unsigned long count;
+	unsigned long *valid;
+};
+
+/* Mark the locations that lie at most FTRACE_MCOUNT_MAX_OFFSET after @addr. */
+static void ftrace_mark_valid_locs(void *data, unsigned long addr)
+{
+	struct ftrace_mod_locs *locs = data;
+	unsigned long lo = 0, hi = locs->count, mid, ip;
+
+	/*
+	 * ftrace_call_adjust() moves a location forward by at most
+	 * FTRACE_MCOUNT_MAX_OFFSET, so start looking that far before @addr.
+	 */
+	while (lo < hi) {
+		mid = lo + (hi - lo) / 2;
+		if (locs->start[mid] + FTRACE_MCOUNT_MAX_OFFSET < addr)
+			lo = mid + 1;
+		else
+			hi = mid;
+	}
+
+	for (; lo < locs->count; lo++) {
+		if (locs->start[lo] > addr + FTRACE_MCOUNT_MAX_OFFSET)
+			break;
+		ip = ftrace_call_adjust(locs->start[lo]);
+		if (ip >= addr && ip - addr <= FTRACE_MCOUNT_MAX_OFFSET)
+			__set_bit(lo, locs->valid);
+	}
+}
+
+/*
+ * A weak function overridden within its module keeps its mcount location but
+ * has no symbol. Zero such locations before they become records, as sorttable
+ * does for vmlinux: keep only those with a symbol at most
+ * FTRACE_MCOUNT_MAX_OFFSET before them.
+ */
+static int ftrace_zero_weak_locs(struct module *mod, unsigned long *start,
+				 unsigned long count)
+{
+	struct ftrace_mod_locs locs = { .start = start, .count = count };
+	unsigned long i;
+
+	locs.valid = bitmap_zalloc(count, GFP_KERNEL);
+	if (!locs.valid)
+		return -ENOMEM;
+
+	module_kallsyms_on_each_addr(mod, ftrace_mark_valid_locs, &locs);
+
+	for_each_clear_bit(i, locs.valid, count)
+		start[i] = 0;
+
+	bitmap_free(locs.valid);
+	return 0;
+}
+#else
+static inline int ftrace_zero_weak_locs(struct module *mod,
+					unsigned long *start,
+					unsigned long count)
+{
+	return 0;
+}
+#endif
+
 static int ftrace_process_locs(struct module *mod,
 			       unsigned long *start,
 			       unsigned long *end)
@@ -7644,6 +7706,9 @@ static int ftrace_process_locs(struct module *mod,
 		test_is_sorted(start, count);
 	}
 
+	if (mod && ftrace_zero_weak_locs(mod, start, count))
+		return -ENOMEM;
+
 	start_pg = ftrace_allocate_pages(count, &pages);
 	if (!start_pg)
 		return -ENOMEM;
@@ -8049,13 +8114,6 @@ void ftrace_module_enable(struct module *mod)
 
 		cond_resched();
 
-		/* Weak functions should still be ignored */
-		if (!test_for_valid_rec(rec)) {
-			/* Clear all other flags. Should not be enabled anyway */
-			rec->flags = FTRACE_FL_DISABLED;
-			continue;
-		}
-
 		cnt = 0;
 
 		/*

-- 
2.55.0