Skip to content

scheduler: add pending_cnt for task to make pending scheduling possible #778

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.

Sign up for GitHub

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

Closed
keyonjie wants to merge 2 commits into from
Closed

scheduler: add pending_cnt for task to make pending scheduling possible #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63cd931
scheduler: add pending_cnt for task to make pending scheduling possible
keyonjie Dec 24, 2018
b711fab
pipeline: switch schedule_task_idle to schedule_task
keyonjie Dec 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/arch/xtensa/include/arch/task.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ static void _irq_task(void *arg)
task = container_of(clist, struct task, irq_list);
list_item_del(clist);

if (task->func && task->state == TASK_STATE_PENDING) {
if (task->func) {
schedule_task_running(task);

@libinyang libinyang Jan 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

run_task = 1;
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/audio/pipeline.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,8 @@ void pipeline_schedule_copy(struct pipeline *p, uint64_t start)
* buffers prior to trigger start. */
void pipeline_schedule_copy_idle(struct pipeline *p)
{
schedule_task_idle(&p->pipe_task, p->ipc_pipe.deadline);
// schedule_task_idle(&p->pipe_task, p->ipc_pipe.deadline);
schedule_task(&p->pipe_task, 0, p->ipc_pipe.deadline);
}

@libinyang libinyang Jan 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.


void pipeline_schedule_cancel(struct pipeline *p)
Expand Down
4 changes: 4 additions & 0 deletions src/include/sof/schedule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ struct task {
/* runtime duration in scheduling clock base */
uint64_t max_rtime; /* max time taken to run */
completion_t complete;

uint32_t pending_cnt;
};

struct schedule_data **arch_schedule_get(void);
Expand All @@ -111,6 +113,7 @@ static inline void schedule_task_init(struct task *task, void (*func)(void *),
void *data)
{
task->core = 0;
task->pending_cnt = 0;
task->state = TASK_STATE_INIT;
task->func = func;
task->data = data;
Expand All @@ -119,6 +122,7 @@ static inline void schedule_task_init(struct task *task, void (*func)(void *),
static inline void schedule_task_free(struct task *task)
{
task->state = TASK_STATE_FREE;
task->pending_cnt = 0;
task->func = NULL;
task->data = NULL;
}
Expand Down
34 changes: 23 additions & 11 deletions src/lib/schedule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,17 +263,17 @@ static int _schedule_task(struct task *task, uint64_t start, uint64_t deadline)

spin_lock_irq(&sch->lock, flags);

/* is task already pending ? - not enough MIPS to complete ? */
if (task->state == TASK_STATE_PENDING) {
trace_schedule("_schedule_task(), task already pending");
spin_unlock_irq(&sch->lock, flags);
return 0;
}

/* is task already queued ? - not enough MIPS to complete ? */
if (task->state == TASK_STATE_QUEUED) {
trace_schedule("_schedule_task(), task already queued");
spin_unlock_irq(&sch->lock, flags);
/*
* increase pending_cnt, will schedule it after this
* pending one is finished.
*/
if (task->state == TASK_STATE_PENDING ||
task->state == TASK_STATE_QUEUED ||
task->state == TASK_STATE_RUNNING ||
task->state == TASK_STATE_PREEMPTED) {
trace_schedule("task 0x%xalready WIP, schedule it later...",
(uint32_t)(uint32_t *)task);
task->pending_cnt++;

@libinyang libinyang Jan 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spin_unlock_irq()

return 0;
}

Expand All @@ -292,6 +292,9 @@ static int _schedule_task(struct task *task, uint64_t start, uint64_t deadline)
/* calculate deadline - TODO: include MIPS */
task->deadline = task->start + ticks_per_ms * deadline / 1000;

if (task->pending_cnt)
task->pending_cnt--;

/* add task to list */
list_item_append(&task->list, &sch->list);
task->state = TASK_STATE_QUEUED;
Expand Down Expand Up @@ -337,6 +340,8 @@ void schedule_task(struct task *task, uint64_t start, uint64_t deadline)
void schedule_task_complete(struct task *task)
{
struct schedule_data *sch = *arch_schedule_get();
uint64_t ticks_per_ms;
uint64_t deadline;
uint32_t flags;

tracev_schedule("schedule_task_complete()");
Expand Down Expand Up @@ -367,6 +372,13 @@ void schedule_task_complete(struct task *task)

/* tell any waiter that task has completed */
wait_completed(&task->complete);

/* schedule the same pending task */
if (task->pending_cnt) {
ticks_per_ms = clock_ms_to_ticks(sch->clock, 1);

@libinyang libinyang Jan 15, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

deadline = (task->deadline - task->start) * 1000 / ticks_per_ms;
schedule_task(task, 0, deadline);
}
}

/* Update task state to running */
Expand Down