[keyonjie]

This series is [4/5] of fixes of #800.

When one instance of a task is queued(queued, pending, running, or
preempted), it is possible that another instance of this same task
scheduling require will come before the previous one is completed. In
today's design, this second instance may be dropped(e.g. for
pipeline_copy() task) and unexpected error will happen.

Here, we add a pending_cnt for the task, accumulate it when needed, and
schedule it when the previous one is finished.

Signed-off-by: Keyon Jie yang.jie@linux.intel.com

[ranj063]

@keyonjie im having a hard time imagining a scenario where this would work properly. If a new task instance arrives before the previous instance completes cleanly, we have lost all the context of the previous task, so keeping a pending count to re-schedule this task will only lead to errors in my opinion.

Also, have you verified this is still an actual issue? could you please share some logs to show the issue?

[keyonjie]

@keyonjie im having a hard time imagining a scenario where this would work properly. If a new task instance arrives before the previous instance completes cleanly, we have lost all the context of the previous task, so keeping a pending count to re-schedule this task will only lead to errors in my opinion.

Also, have you verified this is still an actual issue? could you please share some logs to show the issue?

1. this is not for suspend/resume, it is for simultaneous pipelines, you can easily see that issues(xrun, let me get some logs and attach) with simultaneous pipelines.
2. we don't have too much context for task, both for pipeline and IPC, I think the deadline is the only one context we need to save, I have already add handling to deadline in this patch.

[lgirdwood]

@keyonjie lets have some logs of the issue before we commit any speculative changes.

[wenqingfu]

1. it is for simultaneous pipelines, you can easily see that issues(xrun, let me get some logs and attach) with simultaneous pipelines.

If your PR helps with any Issue, please name it in the PR description, so that that Issue's problem statement help people understand what problem end users will run into without your PR.

[keyonjie]

this PR helps for #743
I can't say it fix that, but it reduce the possibility of it.

[keyonjie]

@keyonjie lets have some logs of the issue before we commit any speculative changes.

@lgirdwood I captured logs like "task 0xXXX already WIP, schedule it later..." from time to time(e.g. happens every 100s~200s playback) where we will drop a schedule copy, then Xrun happens.

I captured this weeks back with messed debugging code changes, I didn't attach the logs here as it may be confusing. Let me try to re-do that later and attach logs.

[libinyang]

I'm not similar with schedule. Just give some comments purely based on the code.

[libinyang]

why remove check state? Even in TASK_STATE_FREE, TASK_STATE_CANEL, TAKS_STATE_INIT and so on without our any cnt > 0?

[libinyang]

spin_unlock_irq()

[libinyang]

The pending_cnt may be wrong now.
Let's image if pending_cnt is 2, and the state is PENDING. schedule_task() will be called. In _schedule_task(), the pending_cnt will be added again. But in this case, the cnt should not be added, right?

[libinyang]

So it should not be named pipeline_schedule_copy_idle()?
And this simple code may impact a lot. Please do a stress test, especially with encoding/decoding and other tasks together. I'm not confident on the side effect of this code.

[keyonjie]

we are seeing error logs in sof-logger without the fix:

    0      1      unknown         651246064.895833         6.041667 /home/keyon/hdd/github/keyonjie/sof/src/lib/schedule.c:362  unexpected task state 5 at task completion
    0      1      unknown         651246070.937500         6.041667 /home/keyon/hdd/github/keyonjie/sof/src/lib/schedule.c:362  unexpected task state 5 at task completion
    0      1      unknown         651246077.135417         6.197917 /home/keyon/hdd/github/keyonjie/sof/src/lib/schedule.c:362  unexpected task state 5 at task completion
    0      1      unknown         651246083.177083         6.041667 /home/keyon/hdd/github/keyonjie/sof/src/lib/schedule.c:362  unexpected task state 5 at task complet

[lgirdwood]

@keyonjie can you confirm the intention here is to allow a task to be scheduled twice ? Wouldn't this mean that it was being rescheduled before it was finished ?

[keyonjie]

@keyonjie can you confirm the intention here is to allow a task to be scheduled twice ? Wouldn't this mean that it was being rescheduled before it was finished ?

The intention here is to allow a task(e.g. schedule_copy for Pipeline 1) can be add to list at the second time(before the first time done) and be scheduled later, it(the 2nd one) won't be rescheduled before the 1st one was finished.

Without this, the 2nd task might be dropped, or impact the 1st instance which is under scheduling.

[lgirdwood]

@keyonjie ok, but my question is why is Pipe1 being rescheduled before it's completed ?This implies there is not enough free cycles to complete the pipe1 processing in this case ?

[keyonjie]

@keyonjie ok, but my question is why is Pipe1 being rescheduled before it's completed ?This implies there is not enough free cycles to complete the pipe1 processing in this case ?

@lgirdwood according to my test, some other cases such as pipeline_schedule_idle(not scheduled at once) might also hit to this.

[lgirdwood]

@keyonjie ok, is this only coming from idle scheduling or from regular runtime pipeline scheduling. If it's idle only I'm tempted just to wait for scheduler updates coming from @tlauda and @mmaka1. @mmaka1 @tlauda do you guys agree ?

[keyonjie]

@keyonjie ok, is this only coming from idle scheduling or from regular runtime pipeline scheduling. If it's idle only I'm tempted just to wait for scheduler updates coming from @tlauda and @mmaka1. @mmaka1 @tlauda do you guys agree ?

@lgirdwood I only found it from idle scheduling till now and no bandwidth to try it more now, I am fine to close this PR and waiting for scheduler updates if it will fix this issue.

[lgirdwood]

@keyonjie ok, lets close now and keep the issue as under investigation

[keyonjie]

OK, closing it now.