Re: [PATCH v6 00/16] perf tools: Use generic syscall scripts for all archs

[Athira Rajeev <atrajeev@linux.vnet.ibm.com>]

> On 10 Jan 2025, at 2:44 AM, Charlie Jenkins <charlie@rivosinc.com> wrote:
> 
> On Thu, Jan 09, 2025 at 05:49:39PM -0300, Arnaldo Carvalho de Melo wrote:
>> On Thu, Jan 09, 2025 at 11:48:36AM -0800, Charlie Jenkins wrote:
>>> On Thu, Jan 09, 2025 at 12:59:43PM -0300, Arnaldo Carvalho de Melo wrote:
>>>> ⬢ [acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
>>>> d06826160a982494 (HEAD -> perf-tools-next) perf tools: Remove dependency on libaudit
>>>> make: Entering directory '/home/acme/git/perf-tools-next/tools/perf'
>>>> - tarpkg: ./tests/perf-targz-src-pkg .
>>>>                 make_static: cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 NO_LIBTRACEEVENT=1 NO_LIBELF=1 -j28  DESTDIR=/tmp/tmp.JJT3tvN7bV
>>>>              make_with_gtk2: cd . && make GTK2=1 -j28  DESTDIR=/tmp/tmp.BF53V2qpl3
>>>> - /home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP: cd . && make FEATURE_DUMP_COPY=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP  feature-dump
>>>> cd . && make FEATURE_DUMP_COPY=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP feature-dump
>>>>         make_no_libbionic_O: cd . && make NO_LIBBIONIC=1 FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j28 O=/tmp/tmp.KZuQ0q2Vs6 DESTDIR=/tmp/tmp.0sxMyH91gS
>>>>           make_util_map_o_O: cd . && make util/map.o FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j28 O=/tmp/tmp.Y0Mx3KLREI DESTDIR=/tmp/tmp.wg9HCVVLHE
>>>>              make_install_O: cd . && make install FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j28 O=/tmp/tmp.P0LEBAkW1X DESTDIR=/tmp/tmp.agTavZndFN
>>>>  failed to find: etc/bash_completion.d/perf
>>> 
>>> Is this something introduced by this patch?
>> 
>> I don't think so.
>> 
>> BTW this series is already pushed out to perf-tools-next:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-tools-next
>> 
>> Thanks!
>> 
>> - Arnaldo
> 
> Thank you!
> 
> - Charlie

Hi Charlie, Arnaldo

While testing the series, I hit compilation issue in powerpc

Snippet of logs:


  CC      util/syscalltbl.o
In file included from /home/athira/perf-tools-next/tools/perf/arch/powerpc/include/syscall_table.h:5,
                 from util/syscalltbl.c:16:
arch/powerpc/include/generated/asm/syscalls_64.h:16:16: error: initialized field overwritten [-Werror=override-init]
   16 |         [13] = "time",
      |                ^~~~~~
arch/powerpc/include/generated/asm/syscalls_64.h:16:16: note: (near initialization for ‘syscalltbl[13]’)
arch/powerpc/include/generated/asm/syscalls_64.h:22:16: error: initialized field overwritten [-Werror=override-init]
   22 |         [18] = "oldstat",
      |                ^~~~~~~~~
arch/powerpc/include/generated/asm/syscalls_64.h:22:16: note: (near initialization for ‘syscalltbl[18]’)
arch/powerpc/include/generated/asm/syscalls_64.h:27:16: error: initialized field overwritten [-Werror=override-init]
   27 |         [22] = "umount",
      |                ^~~~~~~~


And similar errors is there for few more entries. The reason is that, the generated syscalls file has two entries for each of these failing cases.

From arch/powerpc/include/generated/asm/syscalls_64.h created by scrips/syscalltbl.sh 


  1 static const char *const syscalltbl[] = {
  2         [0] = "restart_syscall",
  3         [1] = "exit",
  4         [2] = "fork",
  5         [3] = "read",
  6         [4] = "write",
  7         [5] = "open",
  8         [6] = "close",
  9         [7] = "waitpid",
 10         [8] = "creat",
 11         [9] = "link",
 12         [10] = "unlink",
 13         [11] = "execve",
 14         [12] = "chdir",
 15         [13] = "time”,                           
 16         [13] = "time”,                    
 17         [14] = "mknod",
 18         [15] = "chmod",
 19         [16] = "lchown",
 20         [17] = "break",
 21         [18] = "oldstat",
 22         [18] = "oldstat”,

Line number 15 an 16 shows two entries for time. Similarly last two lines for oldstat. This is picked form https://github.com/torvalds/linux/blob/master/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl

13      32      time                            sys_time32
13      64      time                            sys_time

18      32      oldstat                         sys_stat                        sys_ni_syscall
18      64      oldstat                         sys_ni_syscall

For same nr, two entries are there. In the arch specific version of the script that makes the syscall table, this was handled : https://github.com/torvalds/linux/blob/master/tools/perf/arch/powerpc/entry/syscalls/mksyscalltbl#L28

So we will need change in generic script also. Proposing below change : 

diff --git a/tools/perf/scripts/syscalltbl.sh b/tools/perf/scripts/syscalltbl.sh
index 1ce0d5aa8b50..d66cec10cc2d 100755
--- a/tools/perf/scripts/syscalltbl.sh
+++ b/tools/perf/scripts/syscalltbl.sh
@@ -75,8 +75,10 @@ max_nr=0
 # the params are: nr abi name entry compat
 # use _ for intentionally unused variables according to SC2034
 while read nr _ name _ _; do
-    emit "$nr" "$name" >> $outfile
-    max_nr=$nr
+ if [ "$max_nr" -lt "$nr" ]; then
+ emit "$nr" "$name" >> $outfile
+ max_nr=$nr
+ fi
 done < $sorted_table
   rm -f $sorted_table

Arnaldo,
I see we have this patch series in perf-tools-next. If we need above change as a separate patch, please let me know.

Thanks
Athira

> 
>