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: Merged
Merge reported by: Po-Hsu Lin
Merged at revision: 3a2b62aeb3323a2ec074a1480d063b429899fb50
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
Francis Ginther Pending
Sean Feole Pending
Review via email: mp+447331@code.staging.launchpad.net

This proposal supersedes a proposal from 2023-07-14.

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/

V2:
After testing on a RISCV instance, I recalled that we had this autotest timeout issue on RISCV (bug 1940080), the timeout threshold must be set to 0 consequently.

I have also remove the unused part for grepping test in sub-dir (if statement for cmd = 'grep SUB_DIRS {}'.format(mk_src)), which is only useful in ubuntu_kernel_selftests as we don't have sub-dir with Makefiles in ftrace test.

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

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 : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

> 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 : Posted in a previous version of this proposal

> > 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 : Posted in a previous version of this proposal

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

Revision history for this message
Francis Ginther (fginther) wrote : Posted in a previous version of this proposal

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) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Po-Hsu Lin (cypressyew) wrote :

Note: the -vvv flag will take the test output to about 36MB, but there is nothing really interesting with -v and -vv so...

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

Copy forward the acks.
Applied and pushed, thank for the feedback!

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: