On Thu, Sep 14, 2023 at 11:14:19AM -0700, Andrii Nakryiko wrote: > On Thu, Sep 14, 2023 at 2:52 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote: > > > > On Thu, Sep 14, 2023 at 05:30:51PM +0900, Masahiro Yamada wrote: > > > > SNIP > > > > > > > so the change is about adding unique id that's basically path of > > > > > the object stored in base32 so it could be used as symbol, so we > > > > > don't really need to read the actual file > > > > > > > > > > the problem is when BTF_ID definition like: > > > > > > > > > > BTF_ID(struct, cgroup) > > > > > > > > > > translates in 2 separate objects into same symbol name because of > > > > > the matching __COUNTER__ macro values (like 380 below) > > > > > > > > > > __BTF_ID__struct__cgroup__380 > > > > > > > > > > this change just adds unique id of the path name at the end of the > > > > > symbol with: > > > > > > > > > > echo -n 'kernel/bpf/helpers' | base32 -w0 --> NNSXE3TFNQXWE4DGF5UGK3DQMVZHG > > > > > > > > > > so the symbol looks like: > > > > > > > > > > __BTF_ID__struct__cgroup__380NNSXE3TFNQXWE4DGF5UGK3DQMVZHG > > > > > > > > > > and is unique over the sources > > > > > > > > > > but I still hope we could come up with some better solution ;-) > > > > > > > > so far the only better solution I could come up with is to use > > > > cksum (also from coreutils) instead of base32, which makes the > > > > BTF_ID_BASE value compact > > > > > > > > I'll run test to find out how much it hurts the build time > > > > > > > > jirka > > > > > > > > > > > > Seems a bad idea to me. > > > > > > It would fork a new shell and chsum for all files, > > > while only a few of them need it. > > > > right, I have a change to limit this on kernel and net directories, > > but it's still bad > > > > > > > > Better to consult BTF forks. > > > > perhaps there's better way within kbuild to get unique id/value > > for each object file? > > let's just use __LINE__ + __COUNTER__ for now and teach resolve_btfids > to fail and complain loudly about duplicate symbols? ok, will send that.. but it fails during link before resolve_btfids takes place > > > 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.. we might need to do one extra link phase to get rid of the .BTF_ids_desc secion > way, this baseid hack seems worse and unnecessary. yes, it's bad jirka
