mux: remove the wrong error message by keyonjie · Pull Request #4174 · thesofproject/sof

keyonjie · 2021-05-12T01:08:33Z

A wrong error message was added in helper get_stream_index(), remove to
fix it.

Fixes: 80666b7 ("ipc: component: abstract the component creation API")
Signed-off-by: Keyon Jie yang.jie@linux.intel.com

keyonjie · 2021-05-12T01:12:53Z

@lgirdwood @keqiaozhang @XiaoyunWu6666 this is a quick fix to issues like below happens in daily report:

[        1988.281171] (        1988.281128) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[        2000.937420] (          12.656249) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[        3988.437342] (        1987.499878) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[        4001.145674] (          12.708333) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[        5988.385179] (        1987.239502) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[        6001.249762] (          12.864583) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[        7987.083016] (        1985.833252) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[        7999.687182] (          12.604166) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[        9988.541270] (        1988.854126) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[       10001.145436] (          12.604166) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[       11988.645357] (        1987.499878) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[       12001.405773] (          12.760416) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[       13988.593194] (        1987.187378) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[       14001.405694] (          12.812499) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[       15988.020198] (        1986.614502) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[       16000.624364] (          12.604166) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[       17987.811785] (        1987.187378) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5
[       18000.520118] (          12.708333) c1 mux                     src/audio/mux/mux.c:217  ERROR got 6
[       19988.488789] (        1987.968628) c1 mux                     src/audio/mux/mux.c:217  ERROR got 5

ranj063 · 2021-05-12T04:19:15Z

@keyonjie the change looks good. But I wonder if the return 0 at the of the function is correct? If a match is not found, we should return a negative value isn't it?

@keyonjie the change looks good. But I wonder if the return 0 at the of the function is correct? If a match is not found, we should return a negative value isn't it?

That's a good point. I think the intention of the original author just wanted to return and use the first index (0) if the matched one not found, please check in the caller side, they don't check the return value and just use it as index blindly.

Actually this is unsafe as it's used for an array lookup and may assign a source/sink for usage that is not in the correct state (and may crash etc). Can we check the result and return an error here (and handle it in the caller.)

Actually this is unsafe as it's used for an array lookup and may assign a source/sink for usage that is not in the correct state (and may crash etc). Can we check the result and return an error here (and handle it in the caller.)

Sure, let me add a commit to address this.

lgirdwood

Good find and review comments, but we need to treat and handle the error.

keyonjie · 2021-05-13T04:10:51Z

updated.

Hi @marc-hb looks the imx qemu-boot-test fail, how can I get more info about that?

keyonjie · 2021-05-13T05:21:03Z

SOFCI TEST

dbaluta · 2021-05-13T07:34:16Z

Please use idx instead of i.

dbaluta · 2021-05-13T07:34:43Z

SOFCI TEST

dbaluta · 2021-05-13T07:35:53Z

qemu boot test failure for imx doesn't make much sense. Because this patch doesn't touch anything sensible to boot.

Did something change with docker?

dbaluta · 2021-05-13T07:36:08Z

please test this

dbaluta · 2021-05-13T07:37:07Z

@lgirdwood what's the magic incantation to retrigger CI?

marc-hb · 2021-05-13T07:44:52Z

Did something change with docker?

Yes there's been an Ubuntu upgrade very recently https://github.com/thesofproject/sof/commits/main/scripts/docker_build

https://hub.docker.com/r/thesofproject/sof/tags

@lgirdwood what's the magic incantation to retrigger CI?

It depends which CI. The only way to retrigger all of them is to git commit --amend and force push.

Github Actions can be re-run from the "checks" tab.

marc-hb · 2021-05-13T07:53:56Z

Hi @marc-hb looks the imx qemu-boot-test fail, how can I get more info about that?

The QEMU test waits for some time, then it checks whether some expected output got printed. That expected output never came so the test failed for IMX.

It should be easy to reproduce locally by running the commands at:
https://github.com/thesofproject/sof/blob/main/.github/workflows/pull-request.yml

If using the previous docker image tag gets this test passing again then we can just revert CI to that previous tag until the problem is fixed.

cc: @fredoh9

keyonjie · 2021-05-13T08:07:19Z

Github Actions can be re-run from the "checks" tab.

Thanks, but seems it re-run the "On-device Testing" for me also...

A wrong error message was added in helper get_stream_index(), remove to fix it. Fixes: 80666b7 ("ipc: component: abstract the component creation API") Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>

We should return error if the matching in get_stream_index() fail, add check in the callers, stop using it and return failure when needed. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>

keyonjie · 2021-05-13T08:17:27Z

@lgirdwood what's the magic incantation to retrigger CI?

It depends which CI. The only way to retrigger all of them is to git commit --amend and force push.

Oh, I just realized that 'SOFCI TEST' doesn't work for me anymore while it worked previously, this is not a user friendly change IMHO.

lgirdwood · 2021-05-13T09:57:43Z

I did not realize that SOFCI TEST was going away, but I can now see the check tab and I've clicked rerun tests (which I assume everyone can see ?) Please shout if you cant see this button in the check tab.

lgirdwood · 2021-05-13T10:04:54Z

@fredoh9 @marc-hb ok - I see 2 CI problems.

We seem to install a lot of packages in the container for every Zephyr build. Can we not use a container with these packages already installed ?
The IMX build has no reason to fail with this PR so I'm suspecting the docker update is missing something for imx ?

lgirdwood

There is nothing here that should break IMX or Zephyr build. Safe to merge until we figure out whats breaking CI infrastructure here.

keyonjie · 2021-05-13T10:08:13Z

I did not realize that SOFCI TEST was going away, but I can now see the check tab and I've clicked rerun tests (which I assume everyone can see ?) Please shout if you cant see this button in the check tab.

I can see it but looks what I can choose is only "Re-run all jobs", is that same at your end?

lgirdwood · 2021-05-13T10:19:55Z

I can see it but looks what I can choose is only "Re-run all jobs", is that same at your end?

Yep - I can see the same.

lgirdwood · 2021-05-13T12:09:18Z

Ok, Zephyr CI fix is now merged.
Jenkins showing a DUT HW issue (unrelated).
@dbaluta the IMX8 for some reason is timing out on qemu ? So either our timeout is too short it's really not booting here ?

dbaluta · 2021-05-13T12:39:21Z

@lgirdwood will try to increase the timeout. but i'm quite busy this days.

@paulstelian97 if you have some spare cycles maybe you can investigate.

I think you can run the entire usecase as CI does:
https://github.com/thesofproject/sof/blob/main/.github/workflows/pull-request.yml

Just try to see if increasing the timeout fixes the problem.

marc-hb · 2021-05-13T14:09:41Z

Oh, I just realized that 'SOFCI TEST' doesn't work for me anymore while it worked previously, this is not a user friendly change IMHO.

What makes you think it does not work?

dbaluta · 2021-05-13T14:11:07Z

@marc-hb i tried it several times and none of the CI tasks started.

marc-hb · 2021-05-13T14:13:55Z

These keywords have only ever been scanned by Jenkins and there are many Jenkins run in https://sof-ci.01.org/sofpr/PR4174/

marc-hb · 2021-05-13T15:25:34Z

@lgirdwood will try to increase the timeout. but i'm quite busy this days.
@paulstelian97 if you have some spare cycles maybe you can investigate.

@fredoh9 reproduced the issue, reverted the docker image and file new QEMU IMX issue #4186. See you there!

fredoh9 · 2021-05-13T17:40:53Z

For the keyword, we never disable or change it. But SOF Jenkins server have been throttled, so response may be slow sometimes.

marc-hb · 2021-05-13T19:40:06Z

These SOFCI TEST keywords have only ever been scanned by Jenkins and there are many Jenkins run in https://sof-ci.01.org/sofpr/PR4174/

I just tried it here again and it worked: https://sof-ci.01.org/softestpr/PR544/ , thesofproject/sof-test#544

If you have examples with specific URLs and exact timings then please share.

keyonjie requested a review from akloniex as a code owner May 12, 2021 01:08

keyonjie requested review from RanderWang, XiaoyunWu6666, keqiaozhang and lgirdwood May 12, 2021 01:08

keyonjie requested a review from bardliao May 12, 2021 03:13

RanderWang approved these changes May 12, 2021

View reviewed changes

ranj063 reviewed May 12, 2021

View reviewed changes

dbaluta approved these changes May 12, 2021

View reviewed changes

lgirdwood added this to the v1.8 milestone May 12, 2021

lgirdwood requested changes May 12, 2021

View reviewed changes

keyonjie force-pushed the master_fix branch from 268806b to 72f0e05 Compare May 13, 2021 03:00

keyonjie requested a review from lgirdwood May 13, 2021 03:00

dbaluta reviewed May 13, 2021

View reviewed changes

Comment thread src/audio/mux/mux.c Outdated

dbaluta May 13, 2021

Copy link
Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

Please use idx instead of i.

keyonjie added 2 commits May 13, 2021 16:10

mux: remove the wrong error message

090c9cb

A wrong error message was added in helper get_stream_index(), remove to fix it. Fixes: 80666b7 ("ipc: component: abstract the component creation API") Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>

mux: stop copy if get_stream_index() fail

b7b1d38

We should return error if the matching in get_stream_index() fail, add check in the callers, stop using it and return failure when needed. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>

keyonjie force-pushed the master_fix branch from 72f0e05 to b7b1d38 Compare May 13, 2021 08:15

lgirdwood approved these changes May 13, 2021

View reviewed changes

lgirdwood merged commit d4b8fa8 into thesofproject:main May 13, 2021

Conversation

keyonjie commented May 12, 2021

Uh oh!

keyonjie commented May 12, 2021

Uh oh!

ranj063 May 12, 2021

Choose a reason for hiding this comment

Uh oh!

keyonjie May 12, 2021

Choose a reason for hiding this comment

Uh oh!

lgirdwood May 12, 2021

Choose a reason for hiding this comment

Uh oh!

keyonjie May 13, 2021

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

keyonjie commented May 13, 2021

Uh oh!

keyonjie commented May 13, 2021

Uh oh!

dbaluta May 13, 2021

Choose a reason for hiding this comment

Uh oh!

dbaluta commented May 13, 2021

Uh oh!

dbaluta commented May 13, 2021

Uh oh!

dbaluta commented May 13, 2021

Uh oh!

dbaluta commented May 13, 2021

Uh oh!

marc-hb commented May 13, 2021

Uh oh!

marc-hb commented May 13, 2021

Uh oh!

keyonjie commented May 13, 2021

Uh oh!

keyonjie commented May 13, 2021

Uh oh!

lgirdwood commented May 13, 2021

Uh oh!

lgirdwood commented May 13, 2021

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

keyonjie commented May 13, 2021

Uh oh!

lgirdwood commented May 13, 2021

Uh oh!

lgirdwood commented May 13, 2021

Uh oh!

dbaluta commented May 13, 2021

Uh oh!

marc-hb commented May 13, 2021

Uh oh!

dbaluta commented May 13, 2021

Uh oh!

marc-hb commented May 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc-hb commented May 13, 2021

Uh oh!

fredoh9 commented May 13, 2021

Uh oh!

marc-hb commented May 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

marc-hb commented May 13, 2021 •

edited

Loading