Skip to content

Conversation

@fredoh9
Copy link
Contributor

@fredoh9 fredoh9 commented Jun 14, 2023

…sent

SOF soundcard is already listed it is ready to start the test. If not, set the KERNEL_CHECKPOINT and start polling for FW boot complete. This is a bug fix for fceeee0.

@fredoh9 fredoh9 requested a review from a team as a code owner June 14, 2023 21:48
@fredoh9 fredoh9 requested a review from marc-hb June 14, 2023 21:48
@fredoh9
Copy link
Contributor Author

fredoh9 commented Jun 14, 2023

without --since=@"$KERNEL_CHECKPOINT" is the biggest the problem from fceeee0

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 14, 2023

Summary of an off-line review with @fredoh9 :

  • There's a tiny TOCTOU race window between is_sof_used() and setup_kernel_check_point. But @fredoh9 found that there's already a -1 in setup_kernel_check_point, so we're good. Just need a comment.
  • By fixing a bug, this PR makes it impossible to design any test where the modules are not already loaded. Fred will add an escape. there is already one.

@fredoh9
Copy link
Contributor Author

fredoh9 commented Jun 14, 2023

There is no TOCTOU race condition problem, because setup_kernel_check_point() has -3 or -1 second

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 15, 2023

There is no TOCTOU race condition problem, because setup_kernel_check_point() has -3 or -1 second

That's a bit of hack :-) It's never going to happen but in computer science theory you could have things like:

  • the lib.sh process being suspended by the OS for more than 1 second, or
  • the firmware being UNloaded just after the -3 seconds and before the is_sof_used() grep.

Current code is good enough in practice but please add a comment, something like "TOCTOU race avoided thanks to -1 second inside setup_kernel_check_point()

…sent

SOF soundcard is already listed it is ready to start the test. If not,
set the KERNEL_CHECKPOINT and start polling for FW boot complete.
This is a bug fix for fceeee0.

Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
@fredoh9 fredoh9 force-pushed the fix/sof_card_at_start_test branch from 0dfe51e to 0ce4dd2 Compare June 15, 2023 17:34
@fredoh9
Copy link
Contributor Author

fredoh9 commented Jun 15, 2023

Note, NO_POLL_FW_LOADING variable is already available to skip this polling

@fredoh9
Copy link
Contributor Author

fredoh9 commented Jun 15, 2023

Looks good for device test results also. Merging now

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 21, 2023

Here's an interesting side-effect.

thesofproject/sof#7698 broke the MTL build. Tests were started anyway without firmware (tracked in internal issue 139). Each test took 70s to timeout in https://sof-ci.01.org/sofpr/PR7698/build9948/devicetest

The whole PR test plan uselessly hogged 3 MTL devices for more than 200 minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants