Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions posix/include/rtos/task.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

struct comp_dev;
struct sof;
struct schedule_data;

/** \brief Predefined LL task priorities. */
#define SOF_TASK_PRI_HIGH 0 /* priority level 0 - high */
Expand Down Expand Up @@ -60,6 +61,7 @@ struct task {
uint16_t priority; /**< priority of the task (used by LL) */
uint16_t core; /**< execution core */
uint16_t flags; /**< custom flags */
struct schedule_data *sch; /**< scheduler bound to task */
enum task_state state; /**< current state */
void *data; /**< custom data passed to all ops */
struct list_item list; /**< used by schedulers to hold tasks */
Expand Down
116 changes: 37 additions & 79 deletions src/include/sof/schedule/schedule.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,149 +202,107 @@ static inline void *scheduler_get_data(uint16_t type)
/** See scheduler_ops::schedule_task_running */
static inline int schedule_task_running(struct task *task)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type) {
/* optional operation */
if (!sch->ops->schedule_task_running)
return 0;
assert(task);
sch = task->sch;

return sch->ops->schedule_task_running(sch->data, task);
}
}
if (!sch->ops->schedule_task_running)
Comment on lines 203 to +210
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Some of the schedule.h functions are used on fast path, so I think we need to keep some of the checks on caller side. schedule_task_running() is probably one case where we could have more protective checks. I'll go through the other cases and maybe add assert in the performance sensitive functions.

return 0;

return -ENODEV;
return sch->ops->schedule_task_running(sch->data, task);
}

/** See scheduler_ops::schedule_task */
static inline int schedule_task(struct task *task, uint64_t start,
uint64_t period)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

if (!task)
return -EINVAL;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type)
return sch->ops->schedule_task(sch->data, task, start,
period);
}
assert(task);
sch = task->sch;

Comment on lines 222 to 227
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.

return -ENODEV;
return sch->ops->schedule_task(sch->data, task, start, period);
}

/** See scheduler_ops::schedule_task_before */
static inline int schedule_task_before(struct task *task, uint64_t start,
uint64_t period, struct task *before)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

if (!task || !before)
return -EINVAL;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type) {
if (sch->ops->schedule_task_before)
return sch->ops->schedule_task_before(sch->data, task, start,
period, before);

return sch->ops->schedule_task(sch->data, task, start,
period);
}
}
assert(task);
sch = task->sch;

Comment on lines 237 to 242
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.

return -ENODEV;
if (sch->ops->schedule_task_before)
return sch->ops->schedule_task_before(sch->data, task, start,
period, before);

return sch->ops->schedule_task(sch->data, task, start, period);
}

/** See scheduler_ops::schedule_task_after */
static inline int schedule_task_after(struct task *task, uint64_t start,
uint64_t period, struct task *after)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

if (!task || !after)
return -EINVAL;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type) {
if (sch->ops->schedule_task_after)
return sch->ops->schedule_task_after(sch->data, task, start,
period, after);

return sch->ops->schedule_task(sch->data, task, start,
period);
}
}
assert(task);
sch = task->sch;

Comment on lines 256 to +261
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.

if (sch->ops->schedule_task_after)
return sch->ops->schedule_task_after(sch->data, task, start,
period, after);

return -ENODEV;
return sch->ops->schedule_task(sch->data, task, start, period);
}

/** See scheduler_ops::reschedule_task */
static inline int reschedule_task(struct task *task, uint64_t start)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type) {
/* optional operation */
if (!sch->ops->reschedule_task)
return 0;

return sch->ops->reschedule_task(sch->data, task,
start);
}
}
assert(task);
sch = task->sch;

/* optional operation */
if (!sch->ops->reschedule_task)
return 0;
Comment on lines 270 to +279
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.


return -ENODEV;
return sch->ops->reschedule_task(sch->data, task, start);
}

/** See scheduler_ops::schedule_task_cancel */
static inline int schedule_task_cancel(struct task *task)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type)
return sch->ops->schedule_task_cancel(sch->data, task);
}
assert(task);
sch = task->sch;

return -ENODEV;
return sch->ops->schedule_task_cancel(sch->data, task);
Comment on lines 285 to +292
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.

}

/** See scheduler_ops::schedule_task_free */
static inline int schedule_task_free(struct task *task)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch;
struct list_item *slist;

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type)
return sch->ops->schedule_task_free(sch->data, task);
}
if (!task || !task->sch)
return -EINVAL;

sch = task->sch;

return -ENODEV;
return sch->ops->schedule_task_free(sch->data, task);
Comment on lines 296 to +305
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.

}

/** See scheduler_ops::scheduler_free */
Expand Down
20 changes: 20 additions & 0 deletions src/platform/library/schedule/schedule.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,29 @@ int schedule_task_init(struct task *task,
uint16_t priority, enum task_state (*run)(void *data),
void *data, uint16_t core, uint32_t flags)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch = NULL;
struct list_item *slist;

if (type >= SOF_SCHEDULE_COUNT)
return -EINVAL;

if (!schedulers)
return -ENODEV;

task->sch = NULL;
list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (type == sch->type) {
task->sch = sch;
break;
}
}

if (!task->sch) {
return -ENODEV;
}

task->uid = uid;
task->type = type;
task->priority = priority;
Expand Down
21 changes: 21 additions & 0 deletions src/schedule/schedule.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,32 @@ int schedule_task_init(struct task *task,
uint16_t priority, enum task_state (*run)(void *data),
void *data, uint16_t core, uint32_t flags)
{
struct schedulers *schedulers = *arch_schedulers_get();
struct schedule_data *sch = NULL;
struct list_item *slist;

Comment on lines +30 to +33
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.

if (type >= SOF_SCHEDULE_COUNT) {
tr_err(&sch_tr, "invalid task type");
return -EINVAL;
}

if (!schedulers)
return -ENODEV;

task->sch = NULL;
list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (type == sch->type) {
task->sch = sch;
break;
}
}

if (!task->sch) {
tr_err(&sch_tr, "unable to bind scheduler to task %p (type %d)", task, type);
return -ENODEV;
}
Comment on lines +51 to +54
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in V2.


task->uid = uid;
task->type = type;
task->priority = priority;
Expand Down
6 changes: 6 additions & 0 deletions src/schedule/zephyr_ll.c
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,12 @@ int zephyr_ll_task_init(struct task *task,
if (task->priv_data)
return -EEXIST;

/*
* schedule_task_init() must be run on target core, see
* sof/zephyr/schedule.c:arch_schedulers_get()
*/
Comment on lines +546 to +549
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is incorrect. SOF is checked out in work/sof, so the path is correct. I want to keep "sof/" prefix to emphasize this code is in SOF, not in Zephyr (that will sit in work/zephyr).

assert(cpu_get_id() == core);

ret = schedule_task_init(task, uid, type, priority, run, data, core,
flags);
if (ret < 0)
Expand Down
2 changes: 2 additions & 0 deletions zephyr/include/rtos/task.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

struct comp_dev;
struct sof;
struct schedule_data;

/** \brief Predefined LL task priorities. */
#define SOF_TASK_PRI_HIGH 0 /* priority level 0 - high */
Expand Down Expand Up @@ -59,6 +60,7 @@ struct task {
uint16_t priority; /**< priority of the task (used by LL) */
uint16_t core; /**< execution core */
uint16_t flags; /**< custom flags */
struct schedule_data *sch; /**< scheduler bound to task */
enum task_state state; /**< current state */
void *data; /**< custom data passed to all ops */
struct list_item list; /**< used by schedulers to hold tasks */
Expand Down
Loading