-
Notifications
You must be signed in to change notification settings - Fork 349
edf_schedule: don't assert if there is no extra task to run #4874
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
Conversation
There could be no more EDF task to be run if the last one in the list is finished, we don't need to assert in this case. If the next EDF task gonna be scheduled, interrupt will be generated again by calling to schedule_edf() in schedule_edf_task(), so it should be safe to remove the schedule_edf_task_running() in no pending task scenarios. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
| /* having next task is mandatory */ | ||
| assert(task_next); | ||
|
|
||
| schedule_edf_task_running(data, task_next); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sounds fishy @keyonjie. If the last one in the list is completed, the main audio task would be in WFI isnt it? How will we ever get here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no explanation why the task_next could become NULL as well. But from the scheduler design POV, we should not depends on or make assumption that the main task is always in the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this bug presented today ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lgirdwood here: #4538
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks @keyonjie - I think the correct fix is to make sure we disable all schedulers, timers, IRQs, notifiers prior to shutdown assembly code executing. This should also include cache WB/INV too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, just draft some commits wrt cleanup on secondary cores shutting down here (not tested yet):
#4718
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this should be folded into the power down flow, but I'm curious why the edf scheduler is being run here (if all the task have been removed or the EDF has been shutdown for this core). This could just be a race in the shutdown path and if so we should add this to the comment and explain that no task may be present in this flow.
|
The issue is not reproduced anymore with latest main, let's close and reopen if it is needed in future. |
|
reopen as it might be needed for 2.0 also. |
|
@mwasko fyi |
|
Unrelated RTC process scheduling error on APL. |
There could be no more EDF task to be run if the last one in the list is
finished, we don't need to assert in this case.
If the next EDF task gonna be scheduled, interrupt will be generated
again by calling to schedule_edf() in schedule_edf_task(), so it should
be safe to remove the schedule_edf_task_running() in no pending task
scenarios.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com