Skip to content

multiple-pipeline: replace pidof->ps to catch uninterruptible sleep - #538

Merged
xiulipan merged 2 commits into
thesofproject:masterfrom
marc-hb:multi-pidof
Dec 2, 2020
Merged

multiple-pipeline: replace pidof->ps to catch uninterruptible sleep#538
xiulipan merged 2 commits into
thesofproject:masterfrom
marc-hb:multi-pidof

Conversation

@marc-hb

Copy link
Copy Markdown
Collaborator

pidof ignores processes in uninterruptable sleep by default, probably
because sending them signals is futile. pidof has a -z option but it's
just simpler to use "ps".

Also de-duplicate code into new ps_checks() function

Fixes: #472

Any aplay or arecord process can be in uninterruptable sleep when doing
I/O but the easiest way to reproduce is to wait a few seconds and let the
DSP go to sleep.

Break the strange dependency between how many processes to start and how
many were found in the previous run.
Also move it next to func_run_pipeline_with_type() to make it clearer
it's actually a parameter of that function.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
pidof ignores processes in uninterruptable sleep by default, probably
because sending them signals is futile. pidof has a -z option but it's
just simpler to use "ps".
Also de-duplicate code into new ps_checks() function
Fixes: thesofproject#472
Any aplay or arecord process can be in uninterruptable sleep when doing
I/O but the easiest way to reproduce is to wait a few seconds and let the
DSP go to sleep.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb

Copy link
Copy Markdown
CollaboratorAuthor

The only way to make sure races are fixed is to make them worse. This is what TEST #537 did.

@marc-hb

Copy link
Copy Markdown
CollaboratorAuthor

@marc-hb
marc-hb marked this pull request as ready for review December 1, 2020 17:39
@marc-hb
marc-hb requested a review from a team as a code ownerDecember 1, 2020 17:39
@marc-hb

Copy link
Copy Markdown
CollaboratorAuthor

Note @keqiaozhang moved uninterruptible sleep to the "normal" state category in commit 180b5d7

@xiulipanxiulipan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, I think we already find the process status issue in

# aplay prepare: 'D'; aplay playing: 'S'; aplay pause: 'R';
[[ "$state"=='D'||"$state"=='S'||"$state"=='R' ]] && abnormal_status=$[ $abnormal_status - 1 ]

But the difference of pidof and ps is really difficult to debug.

@xiulipan
xiulipan merged commit e5c6f98 into thesofproject:masterDec 2, 2020
@marc-hb
marc-hb deleted the multi-pidof branch December 2, 2020 06:16
@fredoh9

Copy link
Copy Markdown
Contributor

This is really good catch and fix! Thanks Marc!

@marc-hb

Copy link
Copy Markdown
CollaboratorAuthor

Follow-up commit #543 should save about 20 minutes in nightly tests.

Sign up for freeto 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.

multiple-pipeline-capture: Fail to find 4 pipelines still alive, only 3 left

3 participants

@marc-hb@fredoh9@xiulipan