[1/2] kallsyms: get rid of code for absolute kallsyms
Commit Message
To compile code for absolute kallsyms, you had to turn off
KALLSYMS_BASE_RELATIVE. I think based on the way it was declared in
Kconfig (without a string after "bool"), it wasn't even possible to
select it.
Get rid of this config option, as preparation for some kallsyms
optimizations that would otherwise be infeasible.
Signed-off-by: Jann Horn <jannh@google.com>
---
init/Kconfig | 18 -------
kernel/crash_core.c | 4 --
kernel/kallsyms.c | 10 +---
kernel/kallsyms_internal.h | 1 -
scripts/kallsyms.c | 78 ++++++++++++-----------------
scripts/link-vmlinux.sh | 4 --
tools/perf/tests/vmlinux-kallsyms.c | 1 -
7 files changed, 34 insertions(+), 82 deletions(-)
Comments
On Wed, Feb 21, 2024 at 9:27 PM Jann Horn <jannh@google.com> wrote:
> Currently, kallsyms builds a big assembly file (~19M with a normal
> kernel config), and then the assembler has to turn that big assembly
> file back into binary data, which takes around a second per kallsyms
> invocation. (Normally there are two kallsyms invocations per build.)
>
> It is much faster to instead directly output binary data, which can
> be imported in an assembly file using ".incbin". This is also the
> approach taken by arch/x86/boot/compressed/mkpiggy.c.
> So this patch switches kallsyms to that approach.
>
> A complication with this is that the endianness of numbers between
> host and target might not match (for example, when cross-compiling);
> and there seems to be no kconfig symbol that tells us what endianness
> the target has.
> So pass the path to the intermediate vmlinux ELF file to the kallsyms
> tool, and let it parse the ELF header to figure out the target's
> endianness.
>
> I have verified that running kallsyms without these changes and
> kallsyms with these changes on the same input System.map results
> in identical object files.
>
> This change reduces the time for an incremental kernel rebuild
> (touch fs/ioctl.c, then re-run make) from 27.7s to 24.1s (medians
> over 16 runs each) on my machine - saving around 3.6 seconds.
Ah, I found no maintainer for this file in MAINTAINERS, but now that
I'm looking at the git history, it looks like fixes have come in
through Masahiro Yamada's kbuild tree? So I'm not entirely sure
whether the maintainer for this is Masahiro Yamada or akpm.
On Wed, Feb 21, 2024, at 21:26, Jann Horn wrote:
> To compile code for absolute kallsyms, you had to turn off
> KALLSYMS_BASE_RELATIVE. I think based on the way it was declared in
> Kconfig (without a string after "bool"), it wasn't even possible to
> select it.
>
> Get rid of this config option, as preparation for some kallsyms
> optimizations that would otherwise be infeasible.
>
> Signed-off-by: Jann Horn <jannh@google.com>
Nice catch!
The code was needed until cf8e8658100d ("arch: Remove
Itanium (IA-64) architecture"), and we missed that this
allowed the cleanup you now did.
Acked-by: Arnd Bergmann <arnd@arndb.de>
Arnd
On Thu, Feb 22, 2024 at 5:27 AM Jann Horn <jannh@google.com> wrote:
>
> To compile code for absolute kallsyms, you had to turn off
> KALLSYMS_BASE_RELATIVE. I think based on the way it was declared in
> Kconfig (without a string after "bool"), it wasn't even possible to
> select it.
>
> Get rid of this config option, as preparation for some kallsyms
> optimizations that would otherwise be infeasible.
>
> Signed-off-by: Jann Horn <jannh@google.com>
The code is correct.
Based on Arnd's feedback, could you rephrase the commit description?
When 2213e9a66bb87d83 was applied, IA64 and (TILE && 64BIT)
opted out of KALLSYMS_BASE_RELATIVE.
Now, both architectures are gone.
In the commit description, for instance, you can say:
Commit cf8e8658100d ("arch: Remove Itanium (IA-64) architecture")
removed the last use for the combination of KALLSYMS=y and
KALLSYMS_BASE_RELATIVE=n.
--
Best Regards
Masahiro Yamada
@@ -1791,24 +1791,6 @@ config KALLSYMS_ABSOLUTE_PERCPU
depends on KALLSYMS
default X86_64 && SMP
-config KALLSYMS_BASE_RELATIVE
- bool
- depends on KALLSYMS
- default y
- help
- Instead of emitting them as absolute values in the native word size,
- emit the symbol references in the kallsyms table as 32-bit entries,
- each containing a relative value in the range [base, base + U32_MAX]
- or, when KALLSYMS_ABSOLUTE_PERCPU is in effect, each containing either
- an absolute value in the range [0, S32_MAX] or a relative value in the
- range [base, base + S32_MAX], where base is the lowest relative symbol
- address encountered in the image.
-
- On 64-bit builds, this reduces the size of the address table by 50%,
- but more importantly, it results in entries whose values are build
- time constants, and no relocation pass is required at runtime to fix
- up the entries based on the runtime load address of the kernel.
-
# end of the "standard kernel features (expert users)" menu
config ARCH_HAS_MEMBARRIER_CALLBACKS
@@ -817,12 +817,8 @@ static int __init crash_save_vmcoreinfo_init(void)
VMCOREINFO_SYMBOL(kallsyms_num_syms);
VMCOREINFO_SYMBOL(kallsyms_token_table);
VMCOREINFO_SYMBOL(kallsyms_token_index);
-#ifdef CONFIG_KALLSYMS_BASE_RELATIVE
VMCOREINFO_SYMBOL(kallsyms_offsets);
VMCOREINFO_SYMBOL(kallsyms_relative_base);
-#else
- VMCOREINFO_SYMBOL(kallsyms_addresses);
-#endif /* CONFIG_KALLSYMS_BASE_RELATIVE */
#endif /* CONFIG_KALLSYMS */
arch_crash_save_vmcoreinfo();
@@ -148,9 +148,6 @@ static unsigned int get_symbol_offset(unsigned long pos)
unsigned long kallsyms_sym_address(int idx)
{
- if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
- return kallsyms_addresses[idx];
-
/* values are unsigned offsets if --absolute-percpu is not in effect */
if (!IS_ENABLED(CONFIG_KALLSYMS_ABSOLUTE_PERCPU))
return kallsyms_relative_base + (u32)kallsyms_offsets[idx];
@@ -326,12 +323,9 @@ static unsigned long get_symbol_pos(unsigned long addr,
unsigned long i, low, high, mid;
/* This kernel should never had been booted. */
- if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
- BUG_ON(!kallsyms_addresses);
- else
- BUG_ON(!kallsyms_offsets);
+ BUG_ON(!kallsyms_offsets);
- /* Do a binary search on the sorted kallsyms_addresses array. */
+ /* Do a binary search on the sorted kallsyms_offsets array. */
low = 0;
high = kallsyms_num_syms;
@@ -8,7 +8,6 @@
* These will be re-linked against their real values
* during the second link stage.
*/
-extern const unsigned long kallsyms_addresses[] __weak;
extern const int kallsyms_offsets[] __weak;
extern const u8 kallsyms_names[] __weak;
@@ -6,7 +6,7 @@
* of the GNU General Public License, incorporated herein by reference.
*
* Usage: kallsyms [--all-symbols] [--absolute-percpu]
- * [--base-relative] [--lto-clang] in.map > out.S
+ * [--lto-clang] in.map > out.S
*
* Table compression uses all the unused char codes on the symbols and
* maps these to the most used substrings (tokens). For instance, it might
@@ -63,7 +63,6 @@ static struct sym_entry **table;
static unsigned int table_size, table_cnt;
static int all_symbols;
static int absolute_percpu;
-static int base_relative;
static int lto_clang;
static int token_profit[0x10000];
@@ -76,7 +75,7 @@ static unsigned char best_table_len[256];
static void usage(void)
{
fprintf(stderr, "Usage: kallsyms [--all-symbols] [--absolute-percpu] "
- "[--base-relative] [--lto-clang] in.map > out.S\n");
+ "[--lto-clang] in.map > out.S\n");
exit(1);
}
@@ -484,54 +483,43 @@ static void write_src(void)
printf("\t.short\t%d\n", best_idx[i]);
printf("\n");
- if (!base_relative)
- output_label("kallsyms_addresses");
- else
- output_label("kallsyms_offsets");
+ output_label("kallsyms_offsets");
for (i = 0; i < table_cnt; i++) {
- if (base_relative) {
- /*
- * Use the offset relative to the lowest value
- * encountered of all relative symbols, and emit
- * non-relocatable fixed offsets that will be fixed
- * up at runtime.
- */
+ /*
+ * Use the offset relative to the lowest value
+ * encountered of all relative symbols, and emit
+ * non-relocatable fixed offsets that will be fixed
+ * up at runtime.
+ */
- long long offset;
- int overflow;
-
- if (!absolute_percpu) {
- offset = table[i]->addr - relative_base;
- overflow = (offset < 0 || offset > UINT_MAX);
- } else if (symbol_absolute(table[i])) {
- offset = table[i]->addr;
- overflow = (offset < 0 || offset > INT_MAX);
- } else {
- offset = relative_base - table[i]->addr - 1;
- overflow = (offset < INT_MIN || offset >= 0);
- }
- if (overflow) {
- fprintf(stderr, "kallsyms failure: "
- "%s symbol value %#llx out of range in relative mode\n",
- symbol_absolute(table[i]) ? "absolute" : "relative",
- table[i]->addr);
- exit(EXIT_FAILURE);
- }
- printf("\t.long\t%#x /* %s */\n", (int)offset, table[i]->sym);
- } else if (!symbol_absolute(table[i])) {
- output_address(table[i]->addr);
+ long long offset;
+ int overflow;
+
+ if (!absolute_percpu) {
+ offset = table[i]->addr - relative_base;
+ overflow = (offset < 0 || offset > UINT_MAX);
+ } else if (symbol_absolute(table[i])) {
+ offset = table[i]->addr;
+ overflow = (offset < 0 || offset > INT_MAX);
} else {
- printf("\tPTR\t%#llx\n", table[i]->addr);
+ offset = relative_base - table[i]->addr - 1;
+ overflow = (offset < INT_MIN || offset >= 0);
}
+ if (overflow) {
+ fprintf(stderr, "kallsyms failure: "
+ "%s symbol value %#llx out of range in relative mode\n",
+ symbol_absolute(table[i]) ? "absolute" : "relative",
+ table[i]->addr);
+ exit(EXIT_FAILURE);
+ }
+ printf("\t.long\t%#x /* %s */\n", (int)offset, table[i]->sym);
}
printf("\n");
- if (base_relative) {
- output_label("kallsyms_relative_base");
- output_address(relative_base);
- printf("\n");
- }
+ output_label("kallsyms_relative_base");
+ output_address(relative_base);
+ printf("\n");
if (lto_clang)
for (i = 0; i < table_cnt; i++)
@@ -813,7 +801,6 @@ int main(int argc, char **argv)
static const struct option long_options[] = {
{"all-symbols", no_argument, &all_symbols, 1},
{"absolute-percpu", no_argument, &absolute_percpu, 1},
- {"base-relative", no_argument, &base_relative, 1},
{"lto-clang", no_argument, <o_clang, 1},
{},
};
@@ -834,8 +821,7 @@ int main(int argc, char **argv)
if (absolute_percpu)
make_percpus_absolute();
sort_symbols();
- if (base_relative)
- record_relative_base();
+ record_relative_base();
optimize_token_table();
write_src();
@@ -157,10 +157,6 @@ kallsyms()
kallsymopt="${kallsymopt} --absolute-percpu"
fi
- if is_enabled CONFIG_KALLSYMS_BASE_RELATIVE; then
- kallsymopt="${kallsymopt} --base-relative"
- fi
-
if is_enabled CONFIG_LTO_CLANG; then
kallsymopt="${kallsymopt} --lto-clang"
fi
@@ -26,7 +26,6 @@ static bool is_ignored_symbol(const char *name, char type)
* when --all-symbols is specified so exclude them to get a
* stable symbol list.
*/
- "kallsyms_addresses",
"kallsyms_offsets",
"kallsyms_relative_base",
"kallsyms_num_syms",