| Message ID | 20250304050305.901167-7-irogers@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | perf: Support multiple system call tables in the build | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | fail | Failed to apply series |
On Mon, Mar 03, 2025 at 09:03:00PM -0800, Ian Rogers wrote: > For ELF file dsos read the e_machine from the ELF header. For kernel > types assume the e_machine matches the perf tool. In other cases > return EM_NONE. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/dso.c | 54 +++++++++++++++++++++++++++++++++++++++++++ > tools/perf/util/dso.h | 1 + > 2 files changed, 55 insertions(+) > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index 5c6e85fdae0d..7f2f1af4f73b 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -1170,6 +1170,60 @@ ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, > return data_read_write_offset(dso, machine, offset, data, size, true); > } > > +uint16_t dso__e_machine(struct dso *dso, struct machine *machine) > +{ > + uint16_t e_machine = EM_NONE; > + int fd; > + > + switch (dso__binary_type(dso)) { > + case DSO_BINARY_TYPE__KALLSYMS: > + case DSO_BINARY_TYPE__GUEST_KALLSYMS: > + case DSO_BINARY_TYPE__VMLINUX: > + case DSO_BINARY_TYPE__GUEST_VMLINUX: > + case DSO_BINARY_TYPE__GUEST_KMODULE: > + case DSO_BINARY_TYPE__GUEST_KMODULE_COMP: > + case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE: > + case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP: > + case DSO_BINARY_TYPE__KCORE: > + case DSO_BINARY_TYPE__GUEST_KCORE: > + case DSO_BINARY_TYPE__BPF_PROG_INFO: > + case DSO_BINARY_TYPE__BPF_IMAGE: > + case DSO_BINARY_TYPE__OOL: > + case DSO_BINARY_TYPE__JAVA_JIT: I think some of them can be possible in recorded data. But let's go simple with EM_HOST as we haven't supported cross-arch trace. > + return EM_HOST; > + case DSO_BINARY_TYPE__DEBUGLINK: > + case DSO_BINARY_TYPE__BUILD_ID_CACHE: > + case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO: > + case DSO_BINARY_TYPE__SYSTEM_PATH_DSO: > + case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO: > + case DSO_BINARY_TYPE__FEDORA_DEBUGINFO: > + case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO: > + case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO: > + case DSO_BINARY_TYPE__BUILDID_DEBUGINFO: > + break; > + case DSO_BINARY_TYPE__NOT_FOUND: > + default: > + return EM_NONE; > + } > + > + pthread_mutex_lock(&dso__data_open_lock); Hmm.. I'm afraid it'd slow down perf trace a bit more. It sees occasional LOST events. But it may be ok as it's cached in thread later. > + > + /* > + * dso__data(dso)->fd might be closed if other thread opened another > + * file (dso) due to open file limit (RLIMIT_NOFILE). > + */ > + try_to_open_dso(dso, machine); > + fd = dso__data(dso)->fd; > + if (fd >= 0) { > + _Static_assert(offsetof(Elf32_Ehdr, e_machine) == 18, "Unexpected offset"); > + _Static_assert(offsetof(Elf64_Ehdr, e_machine) == 18, "Unexpected offset"); > + if (pread(fd, &e_machine, sizeof(e_machine), 18) != sizeof(e_machine)) I think it needs to check the endianess and swap the data. Thanks, Namhyung > + e_machine = EM_NONE; > + } > + pthread_mutex_unlock(&dso__data_open_lock); > + return e_machine; > +} > + > /** > * dso__data_read_addr - Read data from dso address > * @dso: dso object > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > index f3ca2a5e7670..ba9b83db061a 100644 > --- a/tools/perf/util/dso.h > +++ b/tools/perf/util/dso.h > @@ -818,6 +818,7 @@ int dso__data_file_size(struct dso *dso, struct machine *machine); > off_t dso__data_size(struct dso *dso, struct machine *machine); > ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, > u64 offset, u8 *data, ssize_t size); > +uint16_t dso__e_machine(struct dso *dso, struct machine *machine); > ssize_t dso__data_read_addr(struct dso *dso, struct map *map, > struct machine *machine, u64 addr, > u8 *data, ssize_t size); > -- > 2.48.1.711.g2feabab25a-goog >
On Thu, Mar 6, 2025 at 5:22 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Mon, Mar 03, 2025 at 09:03:00PM -0800, Ian Rogers wrote: > > For ELF file dsos read the e_machine from the ELF header. For kernel > > types assume the e_machine matches the perf tool. In other cases > > return EM_NONE. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/util/dso.c | 54 +++++++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/dso.h | 1 + > > 2 files changed, 55 insertions(+) > > > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > > index 5c6e85fdae0d..7f2f1af4f73b 100644 > > --- a/tools/perf/util/dso.c > > +++ b/tools/perf/util/dso.c > > @@ -1170,6 +1170,60 @@ ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, > > return data_read_write_offset(dso, machine, offset, data, size, true); > > } > > > > +uint16_t dso__e_machine(struct dso *dso, struct machine *machine) > > +{ > > + uint16_t e_machine = EM_NONE; > > + int fd; > > + > > + switch (dso__binary_type(dso)) { > > + case DSO_BINARY_TYPE__KALLSYMS: > > + case DSO_BINARY_TYPE__GUEST_KALLSYMS: > > + case DSO_BINARY_TYPE__VMLINUX: > > + case DSO_BINARY_TYPE__GUEST_VMLINUX: > > + case DSO_BINARY_TYPE__GUEST_KMODULE: > > + case DSO_BINARY_TYPE__GUEST_KMODULE_COMP: > > + case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE: > > + case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP: > > + case DSO_BINARY_TYPE__KCORE: > > + case DSO_BINARY_TYPE__GUEST_KCORE: > > + case DSO_BINARY_TYPE__BPF_PROG_INFO: > > + case DSO_BINARY_TYPE__BPF_IMAGE: > > + case DSO_BINARY_TYPE__OOL: > > + case DSO_BINARY_TYPE__JAVA_JIT: > > I think some of them can be possible in recorded data. But let's go > simple with EM_HOST as we haven't supported cross-arch trace. I think you are agreeing with the code, which I think is right :-) I don't know a way to go from say /proc/kallsyms text file to an e_machine number and inventing a way is getting beyond the scope of this series. > > + return EM_HOST; > > + case DSO_BINARY_TYPE__DEBUGLINK: > > + case DSO_BINARY_TYPE__BUILD_ID_CACHE: > > + case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO: > > + case DSO_BINARY_TYPE__SYSTEM_PATH_DSO: > > + case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO: > > + case DSO_BINARY_TYPE__FEDORA_DEBUGINFO: > > + case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO: > > + case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO: > > + case DSO_BINARY_TYPE__BUILDID_DEBUGINFO: > > + break; > > + case DSO_BINARY_TYPE__NOT_FOUND: > > + default: > > + return EM_NONE; > > + } > > + > > + pthread_mutex_lock(&dso__data_open_lock); > > Hmm.. I'm afraid it'd slow down perf trace a bit more. It sees > occasional LOST events. But it may be ok as it's cached in thread later. Agreed, I think the caching will handle this. The lock is pre-existing and is used to guard use of the dso_data->fd. The lock could be made reader-writer, it could be placed within the dso_data it could also be sharded. I've not done any of that here as I believe the processing is all handled on a single thread anyway. > > + > > + /* > > + * dso__data(dso)->fd might be closed if other thread opened another > > + * file (dso) due to open file limit (RLIMIT_NOFILE). > > + */ > > + try_to_open_dso(dso, machine); > > + fd = dso__data(dso)->fd; > > + if (fd >= 0) { > > + _Static_assert(offsetof(Elf32_Ehdr, e_machine) == 18, "Unexpected offset"); > > + _Static_assert(offsetof(Elf64_Ehdr, e_machine) == 18, "Unexpected offset"); > > + if (pread(fd, &e_machine, sizeof(e_machine), 18) != sizeof(e_machine)) > > I think it needs to check the endianess and swap the data. Agreed. Will send a fix in v5. Thanks, Ian > Thanks, > Namhyung > > > > + e_machine = EM_NONE; > > + } > > + pthread_mutex_unlock(&dso__data_open_lock); > > + return e_machine; > > +} > > + > > /** > > * dso__data_read_addr - Read data from dso address > > * @dso: dso object > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > > index f3ca2a5e7670..ba9b83db061a 100644 > > --- a/tools/perf/util/dso.h > > +++ b/tools/perf/util/dso.h > > @@ -818,6 +818,7 @@ int dso__data_file_size(struct dso *dso, struct machine *machine); > > off_t dso__data_size(struct dso *dso, struct machine *machine); > > ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, > > u64 offset, u8 *data, ssize_t size); > > +uint16_t dso__e_machine(struct dso *dso, struct machine *machine); > > ssize_t dso__data_read_addr(struct dso *dso, struct map *map, > > struct machine *machine, u64 addr, > > u8 *data, ssize_t size); > > -- > > 2.48.1.711.g2feabab25a-goog > >
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 5c6e85fdae0d..7f2f1af4f73b 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -1170,6 +1170,60 @@ ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, return data_read_write_offset(dso, machine, offset, data, size, true); } +uint16_t dso__e_machine(struct dso *dso, struct machine *machine) +{ + uint16_t e_machine = EM_NONE; + int fd; + + switch (dso__binary_type(dso)) { + case DSO_BINARY_TYPE__KALLSYMS: + case DSO_BINARY_TYPE__GUEST_KALLSYMS: + case DSO_BINARY_TYPE__VMLINUX: + case DSO_BINARY_TYPE__GUEST_VMLINUX: + case DSO_BINARY_TYPE__GUEST_KMODULE: + case DSO_BINARY_TYPE__GUEST_KMODULE_COMP: + case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE: + case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP: + case DSO_BINARY_TYPE__KCORE: + case DSO_BINARY_TYPE__GUEST_KCORE: + case DSO_BINARY_TYPE__BPF_PROG_INFO: + case DSO_BINARY_TYPE__BPF_IMAGE: + case DSO_BINARY_TYPE__OOL: + case DSO_BINARY_TYPE__JAVA_JIT: + return EM_HOST; + case DSO_BINARY_TYPE__DEBUGLINK: + case DSO_BINARY_TYPE__BUILD_ID_CACHE: + case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO: + case DSO_BINARY_TYPE__SYSTEM_PATH_DSO: + case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO: + case DSO_BINARY_TYPE__FEDORA_DEBUGINFO: + case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO: + case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO: + case DSO_BINARY_TYPE__BUILDID_DEBUGINFO: + break; + case DSO_BINARY_TYPE__NOT_FOUND: + default: + return EM_NONE; + } + + pthread_mutex_lock(&dso__data_open_lock); + + /* + * dso__data(dso)->fd might be closed if other thread opened another + * file (dso) due to open file limit (RLIMIT_NOFILE). + */ + try_to_open_dso(dso, machine); + fd = dso__data(dso)->fd; + if (fd >= 0) { + _Static_assert(offsetof(Elf32_Ehdr, e_machine) == 18, "Unexpected offset"); + _Static_assert(offsetof(Elf64_Ehdr, e_machine) == 18, "Unexpected offset"); + if (pread(fd, &e_machine, sizeof(e_machine), 18) != sizeof(e_machine)) + e_machine = EM_NONE; + } + pthread_mutex_unlock(&dso__data_open_lock); + return e_machine; +} + /** * dso__data_read_addr - Read data from dso address * @dso: dso object diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index f3ca2a5e7670..ba9b83db061a 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -818,6 +818,7 @@ int dso__data_file_size(struct dso *dso, struct machine *machine); off_t dso__data_size(struct dso *dso, struct machine *machine); ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, u64 offset, u8 *data, ssize_t size); +uint16_t dso__e_machine(struct dso *dso, struct machine *machine); ssize_t dso__data_read_addr(struct dso *dso, struct map *map, struct machine *machine, u64 addr, u8 *data, ssize_t size);
For ELF file dsos read the e_machine from the ELF header. For kernel types assume the e_machine matches the perf tool. In other cases return EM_NONE. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/dso.c | 54 +++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/dso.h | 1 + 2 files changed, 55 insertions(+)