agent: refactor agent to be more useful #1855

tlauda · 2019-09-23T08:35:21Z

Refactors agent's functionality to really verify DSP
responsiveness. Instead of updating last_idle time
on passive level before entering waiti, let's change it
to last_check and update it on actual scheduler task
execution. Task is executed above passive level, so it
should preempt every other long running processing.
Since we don't have any additional precautions to guarantee
that low latency tick will happen exactly at the scheduled time,
let's define warning and panic thresholds to control stability
of the system.

Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com

src/lib/agent.c

mrajwa · 2019-09-23T09:21:48Z

src/lib/agent.c

+		/* warning timeout */
+		if (delta > sa->warn_timeout)
+			trace_sa_error("validate(), ll drift detected, delta = "
+				       "%u", delta);


What about logging a time of the delay in microseconds instead of bare delta? I am afraid during debugging delta won't tell us much and we will end up calculating it manually anyway.

tlauda · 2019-09-24T06:10:21Z

SOFCI TEST

tlauda · 2019-09-24T09:20:48Z

SOFCI TEST

tlauda · 2019-09-25T11:07:28Z

SOFCI TEST

tlauda · 2019-09-25T15:47:14Z

SOFCI TEST

tlauda · 2019-09-25T17:00:41Z

@xiulipan This change causes agent panic on cAVS platforms with QEMU, however works perfectly fine on real HW. Can you look into this?

xiulipan · 2019-09-26T05:30:27Z

@tlauda Sure will check the issue with QEMU to see if it QEMU issue or FW issue.

xiulipan · 2019-09-27T02:12:14Z

@tlauda Checked the code with QEMU, it seems QEMU can not work was the 1ms agent scheduler.
Do we really need to make an agent to update itself every 1ms?

I am still working on the QEMU code to make it work as fast as HW but it seems very difficult to achieve to goal. @lgirdwood any idea about how to make the QEMU work as the requirement of this high rate timer setting.

tlauda · 2019-09-27T07:14:29Z

@xiulipan It's intended to detect drift in our system tick. I've already added huge panic threshold until some additional tweaks won't be implemented, which will stabilize it more. Why it isn't failing on other platforms? The requirement is the same.

mmaka1 · 2019-09-27T08:30:11Z

@xiulipan @tlauda @lgirdwood Perhaps this is the place where things might be configured more flexibly per specific platform/environment requirements:

ticks = clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1) * timeout / 1000;

As Tomek explained, the intention was to add a heartbeat to something we may call a system tick. Typically it is set to 1ms but may be different per platform. Therefore I'd rephrase the formula to use CONFIG_ kind of timeout configured to longer periods in other environments if they are unable to keep that pace.

xiulipan · 2019-09-27T09:11:13Z

@tlauda @macchian
The QEMU is used to run a very short time and sometimes the agent is not even take effect(BYT and HSW).

I tried to refine the cavs timer implementation in QEME to make sure timer ticks can work as we expected but it seems not that easy. The older agent have a through-hold of 750ms but now we have 1.5ms.
And I am not sure if QEMU can support run that fast to make the heartbeat work. I will try to have a sync with you next monday to talk about my finding

tlauda · 2019-10-11T11:40:35Z

@xiulipan When will you have time to build separate config for QEMU? It's the only thing blocking this PR.

xiulipan · 2019-10-12T03:49:45Z

@tlauda for that I will need to refine the whole logic for both Travis and Jenkins CI. It would not be that easy. Best ETA is next week.

tlauda · 2019-10-12T09:16:16Z

@xiulipan Not a problem. Go for it.

xiulipan · 2019-10-15T07:53:11Z

@tlauda Can you check https://sof-ci.01.org/sofpr/PR1855/build3509/boottest/dump-skl.txt it seems 5ms will still have random issue. Will try with 10ms or 100ms.

tlauda · 2019-10-17T11:38:29Z

@xiulipan 10 ms helped?

tlauda · 2019-10-22T21:59:06Z

@xiulipan Any updates?

xiulipan · 2019-10-23T04:07:06Z

@tlauda could you rebase, I will retry the test about 10 more times to avoid regression in CI.

tlauda · 2019-10-23T08:18:30Z

@xiulipan Done.

xiulipan · 2019-10-23T09:09:59Z

@tlauda Let merge this. I have already make the QEMU test not blocking with 10ms agent time.

I also run manually test with 100 times. No dsp panic happen. But be aware that the qemu test may be failed by the agent. Please check the QEMU test result avoid false alarm blocking test.

xiulipan

The agent works much better than old one. Nice patch.

tlauda · 2019-10-23T10:55:33Z

@xiulipan I'm not sure if I understand correctly. So from now on the Travis QEMU will always fail?

jajanusz · 2019-10-23T12:53:23Z

@xiulipan Where did you adjust new config for qemu? In scripts I see that you still build firmware with just one flag changed for qemu CONFIG_BUILD_VM_ROM, I guess there should be also some custom CONFIG_SYSTICK_PERIOD ?

xiulipan · 2019-10-24T04:58:11Z

@tlauda Forget about the travis, will send the patch to for Travis.
But you may need to re-base after the scripts merged to make sure it will take effect on this PR.

xiulipan · 2019-10-24T04:59:41Z

@jajanusz I did some find and replace in the source code to change the CONFIG_SYSTICK_PERIOD default value.

xiulipan · 2019-10-24T07:51:59Z

@tlauda Travis update is here: #1996

local test with PR1855 and the travis update is here https://travis-ci.org/xiulipan/sof/builds/602186422
All platform passed the QEMU test.

Please make sure we get the right order to merge these PRs

Adds additional CONFIG_SYSTICK_PERIOD to Kconfig, which is used to drive timer based low latency scheduler and also will be used as a timeout check value for system agent. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

Removes PLATFORM_LL_DEFAULT_TIMEOUT definition. It has been replaced by CONFIG_SYSTICK_PERIOD. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

Refactors agent's functionality to really verify DSP responsiveness. Instead of updating last_idle time on passive level before entering waiti, let's change it to last_check and update it on actual scheduler task execution. Task is executed above passive level, so it should preempt every other long running processing. Since we don't have any additional precautions to guarantee that low latency tick will happen exactly at the scheduled time, let's define warning and panic thresholds to control stability of the system. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

Removes disable and enable functions, because they are no longer needed. Agent will now update its lat_check time above passive level, so every long running task will be preempted. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

Removes PLATFORM_IDLE_TIME definition, because it's no longer used. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

Changes scheduler_init_edf and task_main_init function definitions as sof structure is no longer needed. It was only used to pass agent object to main idle task. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

tlauda requested review from lbetlej, lgirdwood, libinyang, mmaka1, mrajwa, plbossart, RanderWang and xiulipan as code owners September 23, 2019 08:35

mrajwa reviewed Sep 23, 2019

View reviewed changes

src/lib/agent.c Outdated Show resolved Hide resolved

tlauda force-pushed the topic/agent-change-check branch from 2cb6c03 to c82408b Compare September 23, 2019 09:18

mrajwa reviewed Sep 23, 2019

View reviewed changes

mmaka1 approved these changes Sep 23, 2019

View reviewed changes

tlauda force-pushed the topic/agent-change-check branch 2 times, most recently from 5c427a8 to a702441 Compare September 24, 2019 08:44

tlauda force-pushed the topic/agent-change-check branch 3 times, most recently from 7614dd8 to 1ee5b4a Compare September 25, 2019 10:00

tlauda force-pushed the topic/agent-change-check branch from 1ee5b4a to b47cb70 Compare September 26, 2019 09:15

tlauda force-pushed the topic/agent-change-check branch from b47cb70 to 19cc72b Compare September 27, 2019 09:07

tlauda requested a review from jajanusz as a code owner September 27, 2019 09:07

tlauda force-pushed the topic/agent-change-check branch from 51597b0 to 37282e8 Compare October 9, 2019 10:21

tlauda force-pushed the topic/agent-change-check branch from 37282e8 to 89fb047 Compare October 17, 2019 11:47

tlauda force-pushed the topic/agent-change-check branch from 89fb047 to 3d8e030 Compare October 23, 2019 08:18

xiulipan approved these changes Oct 23, 2019

View reviewed changes

jajanusz approved these changes Oct 23, 2019

View reviewed changes

tlauda force-pushed the topic/agent-change-check branch from 3d8e030 to 6f851c5 Compare October 23, 2019 10:23

tlauda added 7 commits October 24, 2019 10:16

platform: define and use CONFIG_SYSTICK_PERIOD

e2c24e8

Adds additional CONFIG_SYSTICK_PERIOD to Kconfig, which is used to drive timer based low latency scheduler and also will be used as a timeout check value for system agent. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

platform: remove PLATFORM_LL_DEFAULT_TIMEOUT definition

153c9e0

Removes PLATFORM_LL_DEFAULT_TIMEOUT definition. It has been replaced by CONFIG_SYSTICK_PERIOD. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

agent: add error tracing macro

a9957dc

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

agent: remove disable and enable functions

f5754c4

Removes disable and enable functions, because they are no longer needed. Agent will now update its lat_check time above passive level, so every long running task will be preempted. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

platform: remove PLATFORM_IDLE_TIME definition

f3253eb

Removes PLATFORM_IDLE_TIME definition, because it's no longer used. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

edf_schedule: do not pass sof structure anymore

368a739

Changes scheduler_init_edf and task_main_init function definitions as sof structure is no longer needed. It was only used to pass agent object to main idle task. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>

tlauda force-pushed the topic/agent-change-check branch from 6f851c5 to 368a739 Compare October 24, 2019 08:19

tlauda merged commit f724ef1 into thesofproject:master Oct 24, 2019

tlauda deleted the topic/agent-change-check branch November 22, 2019 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

agent: refactor agent to be more useful #1855

agent: refactor agent to be more useful #1855

tlauda commented Sep 23, 2019

mrajwa Sep 23, 2019

tlauda commented Sep 24, 2019

tlauda commented Sep 24, 2019

tlauda commented Sep 25, 2019

tlauda commented Sep 25, 2019

tlauda commented Sep 25, 2019

xiulipan commented Sep 26, 2019

xiulipan commented Sep 27, 2019

tlauda commented Sep 27, 2019

mmaka1 commented Sep 27, 2019 •

edited

Loading

xiulipan commented Sep 27, 2019

tlauda commented Oct 11, 2019

xiulipan commented Oct 12, 2019

tlauda commented Oct 12, 2019

xiulipan commented Oct 15, 2019

tlauda commented Oct 17, 2019

tlauda commented Oct 22, 2019

xiulipan commented Oct 23, 2019

tlauda commented Oct 23, 2019

xiulipan commented Oct 23, 2019

xiulipan left a comment

tlauda commented Oct 23, 2019

jajanusz commented Oct 23, 2019

xiulipan commented Oct 24, 2019

xiulipan commented Oct 24, 2019

xiulipan commented Oct 24, 2019

agent: refactor agent to be more useful #1855

agent: refactor agent to be more useful #1855

Conversation

tlauda commented Sep 23, 2019

mrajwa Sep 23, 2019

Choose a reason for hiding this comment

tlauda commented Sep 24, 2019

tlauda commented Sep 24, 2019

tlauda commented Sep 25, 2019

tlauda commented Sep 25, 2019

tlauda commented Sep 25, 2019

xiulipan commented Sep 26, 2019

xiulipan commented Sep 27, 2019

tlauda commented Sep 27, 2019

mmaka1 commented Sep 27, 2019 • edited Loading

xiulipan commented Sep 27, 2019

tlauda commented Oct 11, 2019

xiulipan commented Oct 12, 2019

tlauda commented Oct 12, 2019

xiulipan commented Oct 15, 2019

tlauda commented Oct 17, 2019

tlauda commented Oct 22, 2019

xiulipan commented Oct 23, 2019

tlauda commented Oct 23, 2019

xiulipan commented Oct 23, 2019

xiulipan left a comment

Choose a reason for hiding this comment

tlauda commented Oct 23, 2019

jajanusz commented Oct 23, 2019

xiulipan commented Oct 24, 2019

xiulipan commented Oct 24, 2019

xiulipan commented Oct 24, 2019

mmaka1 commented Sep 27, 2019 •

edited

Loading