| Message ID | 20241212-doc_syscalls_link-v1-2-69a916958ba9@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | doc: support for links in syscalls | expand |
Hi Andrea, TL;DR Reviewed-by: Petr Vorel <pvorel@suse.cz> I'm glad that my original idea to have links did not get lost :). > From: Andrea Cervesato <andrea.cervesato@suse.com> > Under statistics tab, add possibility to click on a syscalls and > being redirected to the source code which is testing them. > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > doc/conf.py | 120 ++++++++++++++++++++++++++++++++++-------------------------- > 1 file changed, 69 insertions(+), 51 deletions(-) > diff --git a/doc/conf.py b/doc/conf.py > index 1c6a7f74c9401842c01f33dd34a5171d5328248c..258a1b9e285581e40c03caaf643e295fb90cb0c5 100644 > --- a/doc/conf.py > +++ b/doc/conf.py > @@ -61,39 +61,38 @@ def generate_syscalls_stats(_): > # because in some cases (i.e. io_ring) syscalls are tested, but they are > # part of a more complex scenario. In the following list, we define syscalls > # which we know they are 100% tested already. > - white_list = [ > - 'epoll_pwait2', > - 'fadvise64', > - 'fanotify_init', > - 'fanotify_mark', > - 'getdents64', > - 'getmsg', > - 'getpmsg', You removed getmsg, getpmsg, putmsg and seccomp. I originally put them in my patch to black_list (or I wanted) https://patchwork.ozlabs.org/project/ltp/patch/20241115033916.1707627-1-petr.vorel@gmail.com/ You haven't noticed you removed them (ok, not everything must be documented), but still it would be nice to remove them from "Untested syscalls" table, therefore I sent v2. https://patchwork.ozlabs.org/project/ltp/patch/20241212133024.1480177-1-pvorel@suse.cz/ > - 'inotify_add_watch', > - 'inotify_rm_watch', > - 'io_uring_enter', > - 'io_uring_register', > - 'io_uring_setup', > - 'landlock_add_rule', > - 'landlock_create_ruleset', > - 'landlock_restrict_self', > - 'lsetxattr', > - 'newfstatat', > - 'putmsg', > - 'putpmsg', > - 'pkey_alloc', > - 'pkey_free', > - 'pkey_mprotect', > - 'prlimit64', > - 'pread64', > - 'pselect6', > - 'pwrite64', > - 'quotactl_fd', > - 'rt_sigpending', > - 'seccomp', > - 'semtimedop', > - 'sethostname', > - ] > + ltp_syscalls_path = "testcases/kernel/syscalls" > + white_list = { > + 'bpf': f'{ltp_syscalls_path}/bpf', > + 'epoll_pwait2': f'{ltp_syscalls_path}/epoll_pwait', > + 'fadvise64': f'{ltp_syscalls_path}/fadvise', > + 'fanotify_init': f'{ltp_syscalls_path}/fanotify', > + 'fanotify_mark': f'{ltp_syscalls_path}/fanotify', > + 'futex': f'{ltp_syscalls_path}/futex', > + 'getdents64': f'{ltp_syscalls_path}/gettdents', > + 'inotify_add_watch': f'{ltp_syscalls_path}/inotify', > + 'inotify_init': f'{ltp_syscalls_path}/inotify', > + 'inotify_rm_watch': f'{ltp_syscalls_path}/inotify', > + 'io_uring_enter': f'{ltp_syscalls_path}/io_uring', > + 'io_uring_register': f'{ltp_syscalls_path}/io_uring', > + 'io_uring_setup': f'{ltp_syscalls_path}/io_uring', > + 'landlock_add_rule': f'{ltp_syscalls_path}/landlock', > + 'landlock_create_ruleset': f'{ltp_syscalls_path}/landlock', > + 'landlock_restrict_self': f'{ltp_syscalls_path}/landlock', > + 'lsetxattr': f'{ltp_syscalls_path}/lgetxattr', > + 'newfstatat': f'{ltp_syscalls_path}/fstatat', > + 'pkey_alloc': f'{ltp_syscalls_path}/pkeys', > + 'pkey_free': f'{ltp_syscalls_path}/pkeys', > + 'pkey_mprotect': f'{ltp_syscalls_path}/pkeys', > + 'prlimit64': f'{ltp_syscalls_path}/getrlimit', > + 'pread64': f'{ltp_syscalls_path}/pread', > + 'pselect6': f'{ltp_syscalls_path}/pselect', > + 'pwrite64': f'{ltp_syscalls_path}/pwrite', > + 'quotactl_fd': f'{ltp_syscalls_path}/quotactl', > + 'rt_sigpending': f'{ltp_syscalls_path}/sigpending', > + 'semtimedop': f'{ltp_syscalls_path}/ipc/semop', > + 'sethostname': f'{ltp_syscalls_path}/sethostname' > + } > # populate with not implemented, reserved, unmaintained syscalls defined > # inside the syscalls file > @@ -134,6 +133,7 @@ def generate_syscalls_stats(_): > if error: > return > + syscalls_base_dir = "https://github.com/linux-test-project/ltp/tree/master" nit: we already have some variables for various https://github.com/linux-test-project/ltp paths. I would personally define at the top variable holding "https://github.com/linux-test-project/ltp" and deriver others from it (as a separate commit or a part of previous commit). > text = [ > 'Syscalls\n', > '--------\n\n', > @@ -145,15 +145,33 @@ def generate_syscalls_stats(_): > with open("syscalls.tbl", 'r', encoding='utf-8') as data: > for line in data: > match = regexp.search(line) > - if match: > - ker_syscalls.append(match.group('syscall')) > + if not match: > + continue > + > + ker_syscalls.append(match.group('syscall')) > # collect all LTP tested syscalls > - ltp_syscalls = [] > - for _, _, files in os.walk('../testcases/kernel/syscalls'): > + name_patterns = [ > + re.compile(r'(?P<name>[a-zA-Z_]+[^_])\d{2}\.c'), > + re.compile(r'(?P<name>[a-zA-Z_]+[1-9])_\d{2}\.c'), Using regexp is probably better than my way to set paths (fewer things to maintain). Thanks! Kind regards, Petr
Hi Petr, On 12/12/24 14:34, Petr Vorel wrote: > Hi Andrea, > > TL;DR > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > I'm glad that my original idea to have links did not get lost :). > >> From: Andrea Cervesato <andrea.cervesato@suse.com> >> Under statistics tab, add possibility to click on a syscalls and >> being redirected to the source code which is testing them. >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> doc/conf.py | 120 ++++++++++++++++++++++++++++++++++-------------------------- >> 1 file changed, 69 insertions(+), 51 deletions(-) >> diff --git a/doc/conf.py b/doc/conf.py >> index 1c6a7f74c9401842c01f33dd34a5171d5328248c..258a1b9e285581e40c03caaf643e295fb90cb0c5 100644 >> --- a/doc/conf.py >> +++ b/doc/conf.py >> @@ -61,39 +61,38 @@ def generate_syscalls_stats(_): >> # because in some cases (i.e. io_ring) syscalls are tested, but they are >> # part of a more complex scenario. In the following list, we define syscalls >> # which we know they are 100% tested already. >> - white_list = [ >> - 'epoll_pwait2', >> - 'fadvise64', >> - 'fanotify_init', >> - 'fanotify_mark', >> - 'getdents64', >> - 'getmsg', >> - 'getpmsg', > You removed getmsg, getpmsg, putmsg and seccomp. > > I originally put them in my patch to black_list (or I wanted) > https://patchwork.ozlabs.org/project/ltp/patch/20241115033916.1707627-1-petr.vorel@gmail.com/ > > You haven't noticed you removed them (ok, not everything must be documented), > but still it would be nice to remove them from "Untested syscalls" table, > therefore I sent v2. > https://patchwork.ozlabs.org/project/ltp/patch/20241212133024.1480177-1-pvorel@suse.cz/ Sure, I will send your follow up patch, thanks. >> - 'inotify_add_watch', >> - 'inotify_rm_watch', >> - 'io_uring_enter', >> - 'io_uring_register', >> - 'io_uring_setup', >> - 'landlock_add_rule', >> - 'landlock_create_ruleset', >> - 'landlock_restrict_self', >> - 'lsetxattr', >> - 'newfstatat', >> - 'putmsg', >> - 'putpmsg', >> - 'pkey_alloc', >> - 'pkey_free', >> - 'pkey_mprotect', >> - 'prlimit64', >> - 'pread64', >> - 'pselect6', >> - 'pwrite64', >> - 'quotactl_fd', >> - 'rt_sigpending', >> - 'seccomp', >> - 'semtimedop', >> - 'sethostname', >> - ] >> + ltp_syscalls_path = "testcases/kernel/syscalls" >> + white_list = { >> + 'bpf': f'{ltp_syscalls_path}/bpf', >> + 'epoll_pwait2': f'{ltp_syscalls_path}/epoll_pwait', >> + 'fadvise64': f'{ltp_syscalls_path}/fadvise', >> + 'fanotify_init': f'{ltp_syscalls_path}/fanotify', >> + 'fanotify_mark': f'{ltp_syscalls_path}/fanotify', >> + 'futex': f'{ltp_syscalls_path}/futex', >> + 'getdents64': f'{ltp_syscalls_path}/gettdents', >> + 'inotify_add_watch': f'{ltp_syscalls_path}/inotify', >> + 'inotify_init': f'{ltp_syscalls_path}/inotify', >> + 'inotify_rm_watch': f'{ltp_syscalls_path}/inotify', >> + 'io_uring_enter': f'{ltp_syscalls_path}/io_uring', >> + 'io_uring_register': f'{ltp_syscalls_path}/io_uring', >> + 'io_uring_setup': f'{ltp_syscalls_path}/io_uring', >> + 'landlock_add_rule': f'{ltp_syscalls_path}/landlock', >> + 'landlock_create_ruleset': f'{ltp_syscalls_path}/landlock', >> + 'landlock_restrict_self': f'{ltp_syscalls_path}/landlock', >> + 'lsetxattr': f'{ltp_syscalls_path}/lgetxattr', >> + 'newfstatat': f'{ltp_syscalls_path}/fstatat', >> + 'pkey_alloc': f'{ltp_syscalls_path}/pkeys', >> + 'pkey_free': f'{ltp_syscalls_path}/pkeys', >> + 'pkey_mprotect': f'{ltp_syscalls_path}/pkeys', >> + 'prlimit64': f'{ltp_syscalls_path}/getrlimit', >> + 'pread64': f'{ltp_syscalls_path}/pread', >> + 'pselect6': f'{ltp_syscalls_path}/pselect', >> + 'pwrite64': f'{ltp_syscalls_path}/pwrite', >> + 'quotactl_fd': f'{ltp_syscalls_path}/quotactl', >> + 'rt_sigpending': f'{ltp_syscalls_path}/sigpending', >> + 'semtimedop': f'{ltp_syscalls_path}/ipc/semop', >> + 'sethostname': f'{ltp_syscalls_path}/sethostname' >> + } >> # populate with not implemented, reserved, unmaintained syscalls defined >> # inside the syscalls file >> @@ -134,6 +133,7 @@ def generate_syscalls_stats(_): >> if error: >> return >> + syscalls_base_dir = "https://github.com/linux-test-project/ltp/tree/master" > nit: we already have some variables for various https://github.com/linux-test-project/ltp > paths. I would personally define at the top variable holding > "https://github.com/linux-test-project/ltp" and deriver others from it > (as a separate commit or a part of previous commit). Good catch, I will include it in the last patch since it's required by stats generator specifically. > >> text = [ >> 'Syscalls\n', >> '--------\n\n', >> @@ -145,15 +145,33 @@ def generate_syscalls_stats(_): >> with open("syscalls.tbl", 'r', encoding='utf-8') as data: >> for line in data: >> match = regexp.search(line) >> - if match: >> - ker_syscalls.append(match.group('syscall')) >> + if not match: >> + continue >> + >> + ker_syscalls.append(match.group('syscall')) >> # collect all LTP tested syscalls >> - ltp_syscalls = [] >> - for _, _, files in os.walk('../testcases/kernel/syscalls'): >> + name_patterns = [ >> + re.compile(r'(?P<name>[a-zA-Z_]+[^_])\d{2}\.c'), >> + re.compile(r'(?P<name>[a-zA-Z_]+[1-9])_\d{2}\.c'), > Using regexp is probably better than my way to set paths (fewer things to > maintain). > > Thanks! > > Kind regards, > Petr I will push with the given suggestions and follow up patch, thanks. Andrea
diff --git a/doc/conf.py b/doc/conf.py index 1c6a7f74c9401842c01f33dd34a5171d5328248c..258a1b9e285581e40c03caaf643e295fb90cb0c5 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -61,39 +61,38 @@ def generate_syscalls_stats(_): # because in some cases (i.e. io_ring) syscalls are tested, but they are # part of a more complex scenario. In the following list, we define syscalls # which we know they are 100% tested already. - white_list = [ - 'epoll_pwait2', - 'fadvise64', - 'fanotify_init', - 'fanotify_mark', - 'getdents64', - 'getmsg', - 'getpmsg', - 'inotify_add_watch', - 'inotify_rm_watch', - 'io_uring_enter', - 'io_uring_register', - 'io_uring_setup', - 'landlock_add_rule', - 'landlock_create_ruleset', - 'landlock_restrict_self', - 'lsetxattr', - 'newfstatat', - 'putmsg', - 'putpmsg', - 'pkey_alloc', - 'pkey_free', - 'pkey_mprotect', - 'prlimit64', - 'pread64', - 'pselect6', - 'pwrite64', - 'quotactl_fd', - 'rt_sigpending', - 'seccomp', - 'semtimedop', - 'sethostname', - ] + ltp_syscalls_path = "testcases/kernel/syscalls" + white_list = { + 'bpf': f'{ltp_syscalls_path}/bpf', + 'epoll_pwait2': f'{ltp_syscalls_path}/epoll_pwait', + 'fadvise64': f'{ltp_syscalls_path}/fadvise', + 'fanotify_init': f'{ltp_syscalls_path}/fanotify', + 'fanotify_mark': f'{ltp_syscalls_path}/fanotify', + 'futex': f'{ltp_syscalls_path}/futex', + 'getdents64': f'{ltp_syscalls_path}/gettdents', + 'inotify_add_watch': f'{ltp_syscalls_path}/inotify', + 'inotify_init': f'{ltp_syscalls_path}/inotify', + 'inotify_rm_watch': f'{ltp_syscalls_path}/inotify', + 'io_uring_enter': f'{ltp_syscalls_path}/io_uring', + 'io_uring_register': f'{ltp_syscalls_path}/io_uring', + 'io_uring_setup': f'{ltp_syscalls_path}/io_uring', + 'landlock_add_rule': f'{ltp_syscalls_path}/landlock', + 'landlock_create_ruleset': f'{ltp_syscalls_path}/landlock', + 'landlock_restrict_self': f'{ltp_syscalls_path}/landlock', + 'lsetxattr': f'{ltp_syscalls_path}/lgetxattr', + 'newfstatat': f'{ltp_syscalls_path}/fstatat', + 'pkey_alloc': f'{ltp_syscalls_path}/pkeys', + 'pkey_free': f'{ltp_syscalls_path}/pkeys', + 'pkey_mprotect': f'{ltp_syscalls_path}/pkeys', + 'prlimit64': f'{ltp_syscalls_path}/getrlimit', + 'pread64': f'{ltp_syscalls_path}/pread', + 'pselect6': f'{ltp_syscalls_path}/pselect', + 'pwrite64': f'{ltp_syscalls_path}/pwrite', + 'quotactl_fd': f'{ltp_syscalls_path}/quotactl', + 'rt_sigpending': f'{ltp_syscalls_path}/sigpending', + 'semtimedop': f'{ltp_syscalls_path}/ipc/semop', + 'sethostname': f'{ltp_syscalls_path}/sethostname' + } # populate with not implemented, reserved, unmaintained syscalls defined # inside the syscalls file @@ -134,6 +133,7 @@ def generate_syscalls_stats(_): if error: return + syscalls_base_dir = "https://github.com/linux-test-project/ltp/tree/master" text = [ 'Syscalls\n', '--------\n\n', @@ -145,15 +145,33 @@ def generate_syscalls_stats(_): with open("syscalls.tbl", 'r', encoding='utf-8') as data: for line in data: match = regexp.search(line) - if match: - ker_syscalls.append(match.group('syscall')) + if not match: + continue + + ker_syscalls.append(match.group('syscall')) # collect all LTP tested syscalls - ltp_syscalls = [] - for _, _, files in os.walk('../testcases/kernel/syscalls'): + name_patterns = [ + re.compile(r'(?P<name>[a-zA-Z_]+[^_])\d{2}\.c'), + re.compile(r'(?P<name>[a-zA-Z_]+[1-9])_\d{2}\.c'), + ] + ltp_syscalls = {} + for dirpath, _, files in os.walk(f'../{ltp_syscalls_path}'): for myfile in files: - if myfile.endswith('.c'): - ltp_syscalls.append(myfile) + match = None + for pattern in name_patterns: + match = pattern.search(myfile) + if match: + break + + if not match: + continue + + # we need to use relative path from the project root + path = dirpath.replace('../', '') + name = match.group('name') + + ltp_syscalls[name] = f'{syscalls_base_dir}/{path}' # compare kernel syscalls with LTP tested syscalls syscalls = {} @@ -163,19 +181,19 @@ def generate_syscalls_stats(_): if kersc not in syscalls: if kersc in white_list: - syscalls[kersc] = True + syscalls[kersc] = f'{syscalls_base_dir}/{white_list[kersc]}' continue - syscalls[kersc] = False + syscalls[kersc] = None - for ltpsc in ltp_syscalls: - if ltpsc.startswith(kersc): - syscalls[kersc] = True + for ltpsc, ltpsp in ltp_syscalls.items(): + if ltpsc == kersc: + syscalls[kersc] = ltpsp # generate the statistics file - tested_syscalls = [key for key, val in syscalls.items() if val] - text.append( - 'syscalls which are tested under :master:`testcases/kernel/syscalls`:\n\n') + tested_syscalls = [key for key, val in syscalls.items() if val is not None] + text.append('syscalls which are tested under ' + ':master:`testcases/kernel/syscalls`:\n\n') text.append(f'* kernel syscalls: {len(ker_syscalls)}\n') text.append(f'* tested syscalls: {len(tested_syscalls)}\n\n') @@ -198,12 +216,12 @@ def generate_syscalls_stats(_): max_columns = 3 - for sysname, tested in syscalls.items(): - if tested: + for sysname, path in syscalls.items(): + if path is not None: if (index_tested % max_columns) == 0: - table_tested.append(f' * - {sysname}\n') + table_tested.append(f' * - `{sysname} <{path}>`_\n') else: - table_tested.append(f' - {sysname}\n') + table_tested.append(f' - `{sysname} <{path}>`_\n') index_tested += 1 else: