Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for scheduler bugs #360

Merged
merged 3 commits into from
Sep 18, 2018
Merged

Conversation

xiulipan
Copy link
Contributor

@xiulipan xiulipan commented Sep 12, 2018

Now both ipc process task and pipeline copy task are using scheduler. Need to care about the run order of the task. Fix some bugs in scheduler that will cause the wrong order of task.

  1. use different IRQ to let Ppl copy run in higher IRQ level.
  2. clear scheduler IRQ earlier to let DAI IRQ schedule can run in time.
  3. clear arch task IRQ earlier to make sure all task IRQ goes on time.

@@ -187,14 +187,14 @@ static struct task *schedule_edf(void)

tracev_pipe("edf");

interrupt_clear(PLATFORM_SCHEDULE_IRQ);
Copy link
Member

Choose a reason for hiding this comment

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

Can you check master for a similar patch by @tlauda and backport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not see any similar patch.
@tlauda
Do you make some change like this in master?

Copy link
Member

Choose a reason for hiding this comment

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

Can you change the commit message, the real fix here is to move irq_clear before edf_get_next as there is a race when adding a new task.

@xiulipan
Copy link
Contributor Author

@lgirdwood
Ping.
Checked PRs and found no similar patches.

Lower IPC process task priority to run the process with lower level
interrupt.

Signed-off-by: Pan Xiuli <[email protected]>
@xiulipan
Copy link
Contributor Author

@lgirdwood
Fix one more potential risk in arch task.

task: clear interrupt earlier

list_item_for_safe is safe for item deletion, but when a new item is
appended to the list. It seems the item could not be go through.
To fix this limitation, move interrupt clear to make sure every IRQ is
handled.
Do not try to handle two task with one IRQ handler.

Signed-off-by: Pan Xiuli <[email protected]>

@@ -187,14 +187,14 @@ static struct task *schedule_edf(void)

tracev_pipe("edf");

interrupt_clear(PLATFORM_SCHEDULE_IRQ);
Copy link
Member

Choose a reason for hiding this comment

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

Can you change the commit message, the real fix here is to move irq_clear before edf_get_next as there is a race when adding a new task.

@xiulipan
Copy link
Contributor Author

@lgirdwood
I have test with this patches with tracev enable.
There will still be a race that new task added in
tracev_pipe("edf");

I am now try to refine the schedule_edf function to let it can handle multiple task that have be added.
Or just close the IRQ when we have some task to run.

list_item_for_safe is safe for item deletion, but when a new item is
appended to the list. It seems the item could not be go through.
To fix this limitation, move interrupt clear to make sure every IRQ is
handled.
Do not try to handle two task with one IRQ handler.

Signed-off-by: Pan Xiuli <[email protected]>
@xiulipan
Copy link
Contributor Author

@lgirdwood
PR updated.
Test with
aplay -D hw:0,0 -f dat /dev/zero & arecord -D hw:0,0 -f dat /dev/null -vv -i and pressing space
No firmware xrun now.

There will be new task added in schedule_edf function, refine the
function to handle multiple task.

Signed-off-by: Pan Xiuli <[email protected]>
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