Re: duplicate BTF_IDs leading to symbol redefinition errors?

Jiri Olsa
2023-09-24T13:27:25Z

On Sun, Sep 17, 2023 at 04:09:31PM +0200, Jiri Olsa wrote:

SNIP

> > > > This will give us time and opportunity to implement a better approach
> > > > to .BTF_ids overall. Encoding the desired type name in the symbol name
> > > > always felt off. Maybe it's better to encode type + name as data,
> > > > which is discarded at the latest stage during vmlinux linking? Either
> > >
> > > hum, so maybe having a special section (.BTF_ids_desc below)
> > > that would have record for each ID placed in .BTF_ids section:
> > >
> > > asm( \
> > > ".pushsection .BTF_ids,\"a\"; \n" \
> > > "1: \n" \
> > > ".zero 4 \n" \
> > > ".popsection; \n" \
> > > ".pushsection .BTF_ids_desc,\"a\"; \n" \
> > > ".long 1b \n" \
> > >
> > > and somehow get prefix and name pointers in here:
> > >
> > > ".long prefix
> > > ".long name
> > >
> > > ".popsection; \n");
> > >
> > > so resolve_btfids would iterate .BTF_ids_desc records and fix
> > > up .BTF_ids data..
> > >
> >
> > Something like that. I don't think it's even a regression in terms of
> > vmlinux space usage, because right now we spend as much space on
> > storing symbol names. So just adding string pointers would be already
> > a win due to repeating "struct", "func", etc strings.
> >
> >
> > > we might need to do one extra link phase to get rid of the
> > > .BTF_ids_desc secion
> >
> > Hopefully we can find a way to avoid this, we already do like 3 link
> > phases at least (for kallsyms), so doing all that on the first one and
> > then stripping it out using link script at subsequent one would be
> > best.
>
> perhaps we could move that section under .init.data and
> get rid of it on startup

hi,
I made first version on having extra sections that contain the
auxiliary data for .BTF_ids section,

new sections are:

.BTF_ids_data that holds type and name strings
.BTF_ids_desc that holds array of

struct {
u64 addr_btf_ids; // address to .BTF_ids section
u64 addr_type; // address of type string
u64 addr_name; // address of name string
}

it seems to work ok for vmlinux, but there' problem with kernel modules,
because all the .BTF_ids_desc datas need to be relocated first.. and we
don't get relocated data when reading elf object.. I'll check if we can
relocate that ourselfs, but might be tricky to support this on other archs

It's all in here together with resolve_btfids change:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
btfid_fix

jirka

---
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 9c59409104f6..2f7518b15301 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -658,6 +658,14 @@
. = ALIGN(4); \
.BTF_ids : AT(ADDR(.BTF_ids) - LOAD_OFFSET) { \
*(.BTF_ids) \
+ } \
+ . = ALIGN(4); \
+ .BTF_ids_data : AT(ADDR(.BTF_ids_data) - LOAD_OFFSET) { \
+ *(.BTF_ids_data) \
+ } \
+ . = ALIGN(4); \
+ .BTF_ids_desc : AT(ADDR(.BTF_ids_desc) - LOAD_OFFSET) { \
+ *(.BTF_ids_desc) \
}
#else
#define BTF
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index a9cb10b0e2e9..63a4ebc7f331 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -34,6 +34,7 @@ struct btf_id_set8 {

#define BTF_IDS_SECTION ".BTF_ids"

+#ifdef MODULE
#define ____BTF_ID(symbol, word) \
asm( \
".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
@@ -65,6 +66,38 @@ word \
#define BTF_ID_FLAGS(prefix, name, ...) \
__BTF_ID_FLAGS(prefix, name, ##__VA_ARGS__, 0)

+#else
+#define __BTF_ID(type, name, word) \
+asm( \
+".pushsection .BTF_ids,\"a\"; \n" \
+"1: \n" \
+".zero 4 \n" \
+word \
+".popsection; \n" \
+".pushsection .BTF_ids_data,\"a\"; \n" \
+"2: \n" \
+".string \"" #type "\" \n" \
+"3: \n" \
+".string \"" #name "\" \n" \
+".popsection; \n" \
+".pushsection .BTF_ids_desc,\"a\"; \n" \
+".quad 1b \n" \
+".quad 2b \n" \
+".quad 3b \n" \
+".popsection; \n");
+
+#define BTF_ID(type, name) \
+ __BTF_ID(type, name, "")
+
+#define ____BTF_ID_FLAGS(type, name, flags, ...) \
+ __BTF_ID(type, name, ".long " #flags "\n")
+#define __BTF_ID_FLAGS(type, name, flags, ...) \
+ ____BTF_ID_FLAGS(type, name, flags)
+#define BTF_ID_FLAGS(type, name, ...) \
+ __BTF_ID_FLAGS(type, name, ##__VA_ARGS__, 0)
+
+#endif /* MODULE */
+
/*
* The BTF_ID_LIST macro defines pure (unsorted) list
* of BTF IDs, with following layout: