Merge ~canonical-kernel-team/+git/autotest-client-tests:phlin/ftrace-granularity into ~canonical-kernel-team/+git/autotest-client-tests:master

Proposed by Po-Hsu Lin
Status: Superseded
Proposed branch: ~canonical-kernel-team/+git/autotest-client-tests:phlin/ftrace-granularity
Merge into: ~canonical-kernel-team/+git/autotest-client-tests:master
Diff against target: 95 lines (+22/-40)
2 files modified
ubuntu_kselftests_ftrace/control (+18/-28)
ubuntu_kselftests_ftrace/ubuntu_kselftests_ftrace.py (+4/-12)
Reviewer Review Type Date Requested Status
Sean Feole Approve
Francis Ginther Approve
Review via email: mp+446847@code.launchpad.net

This proposal has been superseded by a proposal from 2023-07-20.

Commit message

Improve the ftracetest granularity by running sub-tests one-by-one.
With this patch, we will be able to hint specific known failures
without letting other regressions to slip through.

Test case listing is achieved by porting the find_testcases() code
in ftracetest script from tools/testing/selftests/ftrace of a kernel
tree. Test verbosity increased by using -vvv flag, this will increase
report file size but it will be easier for debugging.

As we're not running the whole test altogether, the timeout threshold
has been modified to 10 minutes for each case on non-riscv64 systems.
We might need to adjust this later.

I have also removed some leftovers when we copy this test from
ubuntu_kernel_selftests.

Description of the change

Patch tested on a Focal VM. The test number (88) is identical before and after applying this patch.

The verbosity flag comparison between -v, -vv and -vvv can be found here:
https://pastebin.ubuntu.com/p/bP7fhYTxpQ/

To post a comment you must log in.
Revision history for this message
Po-Hsu Lin (cypressyew) wrote :

I will try to run this on an instance that will fail with this test.

Revision history for this message
Po-Hsu Lin (cypressyew) wrote :

I have this patch tested on node janbi with j-realtime, with 10 minute timeout and 30 minute timeout.

The results are identical:
* 10 min
  - timeout ubuntu_kselftests_ftrace.ftrace:test.d--direct--ftrace-direct.tc
  - timeout ubuntu_kselftests_ftrace.ftrace:test.d--event--subsystem-enable.tc
  - fail ubuntu_kselftests_ftrace.ftrace:test.d--ftrace--func_traceonoff_triggers.tc
* 30 min
  - timeout ubuntu_kselftests_ftrace.ftrace:test.d--direct--ftrace-direct.tc
  - timeout ubuntu_kselftests_ftrace.ftrace:test.d--event--subsystem-enable.tc
  - fail ubuntu_kselftests_ftrace.ftrace:test.d--ftrace--func_traceonoff_triggers.tc

They all timeout / fail on the same point, please find the log here:
https://bugs.launchpad.net/ubuntu-kernel-tests/+bug/2027770/comments/1
https://bugs.launchpad.net/ubuntu-kernel-tests/+bug/2027770/comments/2

Therefore I think we can keep the timeout as 10min.

Revision history for this message
Cory Todd (corytodd) wrote :

> As we're not running the whole test altogether, the timeout threshold
has been modified to 10 minutes for each case on non-riscv64 systems.

I'm not seeing where we adjust for non-riscv64. Maybe that's handled in some other definition now?

Revision history for this message
Po-Hsu Lin (cypressyew) wrote :

> > As we're not running the whole test altogether, the timeout threshold
> has been modified to 10 minutes for each case on non-riscv64 systems.
>
> I'm not seeing where we adjust for non-riscv64. Maybe that's handled in some
> other definition now?
For riscv64, it will have arch_scale = 2 by this if statement:
    # Scale timeouts by 2 for riscv64, some tests timeout due to lack of
    # timeout, despite progressing fine
    if arch in ['riscv64']:
        arch_scale = 2

But this reminds me maybe I should give it a try on those RISCV64 first. I will do this tomorrow. Thanks!

Revision history for this message
Cory Todd (corytodd) wrote :

derp, there it is at the top of the control file thanks!

Revision history for this message
Francis Ginther (fginther) wrote :

The "-vvv" may end up being too verbose and end up with huge log files. But let's add it and see if it becomes a problem.

review: Approve
Revision history for this message
Sean Feole (sfeole) :
review: Approve

Unmerged commits

3a2b62a... by Po-Hsu Lin

UBUNTU: SAUCE: ubuntu_kselftests_ftrace: improve test granularity

BugLink: https://bugs.launchpad.net/bugs/2027770

Improve the ftracetest granularity by running sub-tests one-by-one.
With this patch, we will be able to hint specific known failures
without letting other regressions to slip through.

Test case listing is achieved by porting the find_testcases() code
in ftracetest script from tools/testing/selftests/ftrace of a kernel
tree. Test verbosity increased by using -vvv flag, this will increase
report file size but it will be easier for debugging.

As we're not running the whole test altogether, the timeout threshold
has been modified to 10 minutes for each case on non-riscv64 systems.
For riscv64 the timeout will need to be disabled due to the autotest
timeout on it is incorrect (LP: #1940080)

I have also removed some leftovers when we copy this test from
ubuntu_kernel_selftests.

Signed-off-by: Po-Hsu Lin <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/ubuntu_kselftests_ftrace/control b/ubuntu_kselftests_ftrace/control
2index 7d27f65..3d4f04a 100644
3--- a/ubuntu_kselftests_ftrace/control
4+++ b/ubuntu_kselftests_ftrace/control
5@@ -17,7 +17,7 @@ arch_scale = 1
6 # Scale timeouts by 2 for riscv64, some tests timeout due to lack of
7 # timeout, despite progressing fine
8 if arch in ['riscv64']:
9- arch_scale = 2
10+ arch_scale = 2
11
12 result = job.run_test_detail(NAME, test_name='setup', tag='setup', timeout=arch_scale*60*45)
13 if result == 'GOOD':
14@@ -32,36 +32,26 @@ if result == 'GOOD':
15 dir_src = os.path.join(dir_root, category)
16 mk_src = os.path.join(dir_src, 'Makefile')
17 os.chdir(dir_src)
18- cmd = 'grep SUB_DIRS {}'.format(mk_src)
19- timeout_threshold = arch_scale*60*30
20- if utils.system_output(cmd, verbose=False, ignore_status=True):
21- cmd = 'make -f {} -f {} getsubdirs'.format(mk_helper, mk_src)
22- subdirs = utils.system_output(cmd).split()
23- for subdir in subdirs:
24- dir_src = os.path.join(dir_root, category, subdir)
25- os.chdir(dir_src)
26- mk_src = os.path.join(dir_src, 'Makefile')
27- if os.path.isfile(mk_src):
28- cmd = 'make -f {} -f {} gettests'.format(mk_src, mk_helper)
29- tests = utils.system_output(cmd).split()
30- for item in tests:
31- test = "{}/{}:{}".format(category, subdir, item)
32- job.run_test_detail(NAME, test_name=test, tag=test, timeout=timeout_threshold)
33- elif os.path.isfile(mk_src):
34+ timeout_threshold = 60 * 10
35+ if arch == 'riscv64':
36+ # autotest timeout on riscv64 is incorrect (lp:1940080), disable it
37+ timeout_threshold = 0
38+
39+ if os.path.isfile(mk_src):
40 cmd = 'make -f {} -f {} gettests'.format(mk_src, mk_helper)
41 tests = utils.system_output(cmd).split()
42 for item in tests:
43- timeout_threshold = arch_scale*60*45
44- if item == 'ftracetest':
45- if arch == 'riscv64':
46- # autotest timeout on riscv64 is incorrect (lp:1940080), disable it
47- # It takes about 22 mins on 5.15 and about 35 mins on 5.13
48- timeout_threshold = 0
49- else:
50- # ftracetest will take about ~60 minutes to run on some instances (lp:2008063)
51- timeout_threshold = 60 * 75
52- test = "{}:{}".format(category, item)
53- job.run_test_detail(NAME, test_name=test, tag=test, timeout=timeout_threshold)
54+ # Get all sub-tests for ftracetest and run them one-by-one
55+ cmd = 'find {}/test.d/ -name *.tc | sort'.format(dir_src)
56+ ftrace_tests = utils.system_output(cmd).split()
57+ for ftrace_sub_test in ftrace_tests:
58+ # Replace the '/' in test pathname with '--' (as '-' and '_' have been used)
59+ # to avoid dir name collisions, otherwise autotest will try to create a dir
60+ # for it. The next test in the same dir will error out with:
61+ # "multiple tests cannot run with the same subdirectory"
62+ clean_name = ftrace_sub_test.split('/selftests/ftrace/')[1].replace('/', '--')
63+ test = "{}:{}".format(category, clean_name)
64+ job.run_test_detail(NAME, test_name=test, tag=test, timeout=timeout_threshold)
65 else:
66 print("ERROR: test failed to build, skipping all the sub tests")
67
68diff --git a/ubuntu_kselftests_ftrace/ubuntu_kselftests_ftrace.py b/ubuntu_kselftests_ftrace/ubuntu_kselftests_ftrace.py
69index 787e120..0f14def 100644
70--- a/ubuntu_kselftests_ftrace/ubuntu_kselftests_ftrace.py
71+++ b/ubuntu_kselftests_ftrace/ubuntu_kselftests_ftrace.py
72@@ -96,19 +96,11 @@ class ubuntu_kselftests_ftrace(test.test):
73
74 category = test_name.split(':')[0]
75 sub_test = test_name.split(':')[1]
76- dir_root = os.path.join(self.srcdir, 'linux', 'tools', 'testing', 'selftests')
77+ dir_root = os.path.join(self.srcdir, 'linux', 'tools', 'testing', 'selftests', 'ftrace')
78 os.chdir(dir_root)
79- cmd = "make run_tests -C {} TEST_PROGS={} TEST_GEN_PROGS='' TEST_CUSTOM_PROGS=''".format(category, sub_test)
80+ # Run sub-tests with ftracetest script, convert test name back to path
81+ test = sub_test.replace('--', '/')
82+ cmd = './ftracetest -vvv {}'.format(test)
83 result = utils.system_output(cmd, retain_output=True)
84
85- # Old pattern for Xenial
86- pattern = re.compile('selftests: *(?P<case>[\w\-\.]+) \[FAIL\]\n')
87- if re.search(pattern, result):
88- raise error.TestError(test_name + ' failed.')
89- # If the test was not end by previous check, check again with new pattern
90- pattern = re.compile('not ok [\d\.]* selftests: {}: {} # (?!.*SKIP)'.format(category, sub_test))
91- if re.search(pattern, result):
92- raise error.TestError(test_name + ' failed.')
93-
94-
95 # vi:set ts=4 sw=4 expandtab syntax=python:

Subscribers

People subscribed via source and target branches

to all changes: