Re: [PATCH v2 6/7] perf syscalltbl: Use lookup table containing multiple architectures

Ian Rogers
2025-02-11T05:15:31Z

On Mon, Feb 10, 2025 at 3:39 PM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> On Mon, Feb 10, 2025 at 08:51:07AM -0800, Ian Rogers wrote:
> > Switch to use the lookup table containing all architectures rather
> > than tables matching the perf binary.
> >
> > This fixes perf trace when executed on a 32-bit i386 binary on an
> > x86-64 machine. Note in the following the system call names of the
> > 32-bit i386 binary as seen by an x86-64 perf.
> >
> > Before:
> > ```
> > ? ( ): a.out/447296 ... [continued]: munmap()) = 0
> > 0.024 ( 0.001 ms): a.out/447296 recvfrom(ubuf: 0x2, size: 4160585708, flags: DONTROUTE|CTRUNC|TRUNC|DONTWAIT|EOR|WAITALL|FIN|SYN|CONFIRM|RST|ERRQUEUE|NOSIGNAL|WAITFORONE|BATCH|SOCK_DEVMEM|ZEROCOPY|FASTOPEN|CMSG_CLOEXEC|0x91f80000, addr: 0xe30, addr_len: 0xffce438c) = 1475198976
> > 0.042 ( 0.003 ms): a.out/447296 lgetxattr(name: "", value: 0x3, size: 34) = 4160344064
> > 0.054 ( 0.003 ms): a.out/447296 dup2(oldfd: -134422744, newfd: 4) = -1 ENOENT (No such file or directory)
> > 0.060 ( 0.009 ms): a.out/447296 preadv(fd: 4294967196, vec: (struct iovec){.iov_base = (void *)0x2e646c2f6374652f,.iov_len = (__kernel_size_t)7307199665335594867,}, vlen: 557056, pos_h: 4160585708) = 3
> > 0.074 ( 0.004 ms): a.out/447296 lgetxattr(name: "", value: 0x1, size: 2) = 4160237568
> > 0.080 ( 0.001 ms): a.out/447296 lstat(filename: "", statbuf: 0x193f6) = 0
> > 0.089 ( 0.007 ms): a.out/447296 preadv(fd: 4294967196, vec: (struct iovec){.iov_base = (void *)0x3833692f62696c2f,.iov_len = (__kernel_size_t)3276497845987585334,}, vlen: 557056, pos_h: 4160585708) = 3
> > 0.097 ( 0.002 ms): a.out/447296 close(fd: 3</proc/447296/status>) = 512
> > 0.103 ( 0.002 ms): a.out/447296 lgetxattr(name: "", value: 0x1, size: 2050) = 4157935616
> > 0.107 ( 0.007 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x5, size: 2066) = 4158078976
> > 0.116 ( 0.003 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x1, size: 2066) = 4159639552
> > 0.121 ( 0.003 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x3, size: 2066) = 4160184320
> > 0.129 ( 0.002 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x3, size: 50) = 4160196608
> > 0.138 ( 0.001 ms): a.out/447296 lstat(filename: "") = 0
> > 0.145 ( 0.002 ms): a.out/447296 mq_timedreceive(mqdes: 4291706800, u_msg_ptr: 0xf7f9ea48, msg_len: 134616640, u_msg_prio: 0xf7fd7fec, u_abs_timeout: (struct __kernel_timespec){.tv_sec = (__kernel_time64_t)-578174027777317696,.tv_nsec = (long long int)4160349376,}) = 0
> > 0.148 ( 0.001 ms): a.out/447296 mkdirat(dfd: -134617816, pathname: " ��� ���▒���▒���", mode: IFREG|ISUID|IRUSR|IWGRP|0xf7fd0000) = 447296
> > 0.150 ( 0.001 ms): a.out/447296 process_vm_writev(pid: -134617812, lvec: (struct iovec){.iov_base = (void *)0xf7f9e9c8f7f9e4c0,.iov_len = (__kernel_size_t)4160349376,}, liovcnt: 4160588048, rvec: (struct iovec){}, riovcnt: 4160585708, flags: 4291707352) = 0
> > 0.197 ( 0.004 ms): a.out/447296 capget(header: 4160184320, dataptr: 8192) = 0
> > 0.202 ( 0.002 ms): a.out/447296 capget(header: 1448669184, dataptr: 4096) = 0
> > 0.208 ( 0.002 ms): a.out/447296 capget(header: 4160577536, dataptr: 8192) = 0
> > 0.220 ( 0.001 ms): a.out/447296 getxattr(pathname: "", name: "c������", value: 0xf7f77e34, size: 1) = 0
> > 0.228 ( 0.005 ms): a.out/447296 fchmod(fd: -134729728, mode: IRUGO|IWUGO|IFREG|IFIFO|ISVTX|IXUSR|0x10000) = 0
> > 0.240 ( 0.009 ms): a.out/447296 preadv(fd: 4294967196, vec: 0x5658e008, pos_h: 4160192052) = 3
> > 0.250 ( 0.008 ms): a.out/447296 close(fd: 3</proc/447296/status>) = 1436
> > 0.260 ( 0.018 ms): a.out/447296 stat(filename: "", statbuf: 0xffce32ac) = 1436
> > 0.288 (1000.213 ms): a.out/447296 readlinkat(buf: 0xffce31d4, bufsiz: 4291703244) = 0
> > ```
> >
> > After:
> > ```
> > ? ( ): a.out/442930 ... [continued]: execve()) = 0
> > 0.023 ( 0.002 ms): a.out/442930 brk() = 0x57760000
> > 0.052 ( 0.003 ms): a.out/442930 access(filename: 0xf7f5af28, mode: R) = -1 ENOENT (No such file or directory)
> > 0.059 ( 0.009 ms): a.out/442930 openat(dfd: CWD, filename: "/etc/ld.so.cache", flags: RDONLY|CLOEXEC|LARGEFILE) = 3
> > 0.078 ( 0.001 ms): a.out/442930 close(fd: 3</proc/442930/status>) = 0
> > 0.087 ( 0.007 ms): a.out/442930 openat(dfd: CWD, filename: "/lib/i386-linux-", flags: RDONLY|CLOEXEC|LARGEFILE) = 3
> > 0.095 ( 0.002 ms): a.out/442930 read(fd: 3</proc/442930/status>, buf: 0xffbdbb70, count: 512) = 512
> > 0.135 ( 0.001 ms): a.out/442930 close(fd: 3</proc/442930/status>) = 0
> > 0.148 ( 0.001 ms): a.out/442930 set_tid_address(tidptr: 0xf7f2b528) = 442930 (a.out)
> > 0.150 ( 0.001 ms): a.out/442930 set_robust_list(head: 0xf7f2b52c, len: 12) =
> > 0.196 ( 0.004 ms): a.out/442930 mprotect(start: 0xf7f03000, len: 8192, prot: READ) = 0
> > 0.202 ( 0.002 ms): a.out/442930 mprotect(start: 0x5658e000, len: 4096, prot: READ) = 0
> > 0.207 ( 0.002 ms): a.out/442930 mprotect(start: 0xf7f63000, len: 8192, prot: READ) = 0
> > 0.230 ( 0.005 ms): a.out/442930 munmap(addr: 0xf7f10000, len: 103414) = 0
> > 0.244 ( 0.010 ms): a.out/442930 openat(dfd: CWD, filename: 0x5658d008) = 3
> > 0.255 ( 0.007 ms): a.out/442930 read(fd: 3</proc/442930/status>, buf: 0xffbdb67c, count: 4096) = 1436
> > 0.264 ( 0.018 ms): a.out/442930 write(fd: 1</dev/pts/4>, buf: , count: 1436) = 1436
> > 0.292 (1000.173 ms): a.out/442930 clock_nanosleep(rqtp: { .tv_sec: 17866546940376776704, .tv_nsec: 4159878336 }, rmtp: 0xffbdb59c) = 0
> > 1000.478 ( ): a.out/442930 exit_group() = ?
> > ```
> >
>
> I think I am conflating some things in my mind here. This change doesn't
> impact perf report does it? perf report reports syscall numbers only,
> but it could be hooked up into this change to correctly report the
> correct syscall name from a perf.data generated on any architecture?
>
> I believe that question is tangential to this patch but let me know!

You are right it is tangential. There are tracepoints for most system
calls that could be used to break down individual counts. You may be
thinking of `perf trace record` that I mentioned. Many of the perf
commands have a record and report option, so that things can be done
off-line. The support isn't what it should be. For example, `perf stat
record` will record event information and counts but it doesn't do
things like store metrics, so you'd need to manually compute a metric
from `perf stat report` if you wanted to use `perf stat` in this
record/report mode. I think often times think C is a very slow and
verbose way of doing the plumbing for these things, so I keep pushing
my python patches along.

Thanks for all your changes and the reviews! This patch series is just
moving things you've enabled around and was little work as a
consequence.

Thanks,
Ian

> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > Reviewed-by: Howard Chu <howardchu95@gmail.com>
> > ---
> > tools/perf/util/syscalltbl.c | 89 ++++++++++++++++++++++++++----------
> > 1 file changed, 64 insertions(+), 25 deletions(-)
> >
> > diff --git a/tools/perf/util/syscalltbl.c b/tools/perf/util/syscalltbl.c
> > index 760ac4d0869f..db0d2b81aed1 100644
> > --- a/tools/perf/util/syscalltbl.c
> > +++ b/tools/perf/util/syscalltbl.c
> > @@ -15,16 +15,39 @@
> > #include <string.h>
> > #include "string2.h"
> >
> > -#if __BITS_PER_LONG == 64
> > - #include <asm/syscalls_64.h>
> > -#else
> > - #include <asm/syscalls_32.h>
> > -#endif
> > +#include "trace/beauty/generated/syscalltbl.c"
> >
> > -const char *syscalltbl__name(int e_machine __maybe_unused, int id)
> > +static const struct syscalltbl *find_table(int e_machine)
> > {
> > - if (id >= 0 && id <= (int)ARRAY_SIZE(syscall_num_to_name))
> > - return syscall_num_to_name[id];
> > + static const struct syscalltbl *last_table;
> > + static int last_table_machine = EM_NONE;
> > +
> > + /* Tables only exist for EM_SPARC. */
> > + if (e_machine == EM_SPARCV9)
> > + e_machine = EM_SPARC;
> > +
> > + if (last_table_machine == e_machine && last_table != NULL)
> > + return last_table;
> > +
> > + for (size_t i = 0; i < ARRAY_SIZE(syscalltbls); i++) {
> > + const struct syscalltbl *entry = &syscalltbls[i];
> > +
> > + if (entry->e_machine != e_machine && entry->e_machine != EM_NONE)
> > + continue;
> > +
> > + last_table = entry;
> > + last_table_machine = e_machine;
> > + return entry;
> > + }
> > + return NULL;
> > +}
> > +
> > +const char *syscalltbl__name(int e_machine, int id)
> > +{
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + if (table && id >= 0 && id < table->num_to_name_len)
> > + return table->num_to_name[id];
> > return NULL;
> > }
> >
> > @@ -41,38 +64,54 @@ static int syscallcmpname(const void *vkey, const void *ventry)
> > return strcmp(key->name, key->tbl[*entry]);
> > }
> >
> > -int syscalltbl__id(int e_machine __maybe_unused, const char *name)
> > +int syscalltbl__id(int e_machine, const char *name)
> > {
> > - struct syscall_cmp_key key = {
> > - .name = name,
> > - .tbl = syscall_num_to_name,
> > - };
> > - const int *id = bsearch(&key, syscall_sorted_names,
> > - ARRAY_SIZE(syscall_sorted_names),
> > - sizeof(syscall_sorted_names[0]),
> > - syscallcmpname);
> > + const struct syscalltbl *table = find_table(e_machine);
> > + struct syscall_cmp_key key;
> > + const int *id;
> > +
> > + if (!table)
> > + return -1;
> > +
> > + key.name = name;
> > + key.tbl = table->num_to_name;
> > + id = bsearch(&key, table->sorted_names, table->sorted_names_len,
> > + sizeof(table->sorted_names[0]), syscallcmpname);
> >
> > return id ? *id : -1;
> > }
> >
> > -int syscalltbl__num_idx(int e_machine __maybe_unused)
> > +int syscalltbl__num_idx(int e_machine)
> > {
> > - return ARRAY_SIZE(syscall_sorted_names);
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + if (!table)
> > + return 0;
> > +
> > + return table->sorted_names_len;
> > }
> >
> > -int syscalltbl__id_at_idx(int e_machine __maybe_unused, int idx)
> > +int syscalltbl__id_at_idx(int e_machine, int idx)
> > {
> > - return syscall_sorted_names[idx];
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + if (!table)
> > + return -1;
> > +
> > + assert(idx >= 0 && idx < table->sorted_names_len);
> > + return table->sorted_names[idx];
> > }
> >
> > -int syscalltbl__strglobmatch_next(int e_machine __maybe_unused, const char *syscall_glob, int *idx)
> > +int syscalltbl__strglobmatch_next(int e_machine, const char *syscall_glob, int *idx)
> > {
> > - for (int i = *idx + 1; i < (int)ARRAY_SIZE(syscall_sorted_names); ++i) {
> > - const char *name = syscall_num_to_name[syscall_sorted_names[i]];
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + for (int i = *idx + 1; table && i < table->sorted_names_len; ++i) {
> > + const char *name = table->num_to_name[table->sorted_names[i]];
> >
> > if (strglobmatch(name, syscall_glob)) {
> > *idx = i;
> > - return syscall_sorted_names[i];
> > + return table->sorted_names[i];
> > }
> > }
> >
> > --
> > 2.48.1.502.g6dc24dfdaf-goog
> >