Skip to content

Conversation

vicLin8712
Copy link
Collaborator

Hi @jserv ,

This is still a draft. I’ve implemented O(1) task selection using a
priority bitmap and per-priority queues. Key design points:

  • mo_task_spawn() enqueues new tasks, except the very first one which
    is assigned to task_current with state TASK_RUNNING.
  • sched_select_next_task() reinserts the running task into its ready
    queue, then scans ready_bitmap to select the highest-priority ready
    queue in O(1).

Could you confirm if this approach aligns with the project’s design
expectations?
In particular, I’d like feedback on:

  1. The use of the current sched_t data structure.
  2. Whether revisions to sched_t or kcb would be preferred to better
    support future SMP design.

Introduce sched_t structure to encapsulate O(1) scheduler state.
sched_t maintains ready_queues separated by priority, enabling task
selection in O(1) complexity.

kcb now contains a sched_t instance, allowing per-hart scheduler
control and future SMP extensions.
New tasks created by `mo_task_spawn()` are enqueued into their priority
ready queue. The very first task is special-cased: it becomes
`task_current` and its state is set to `TASK_RUNNING` (not enqueued).

`sched_select_next_task()` first returns the running task  to its
ready queue, then finds the highest non-empty priority by scanning the
`ready_bitmap`, pops one node from that queue, and assigns it to
`task_current`. The corresponding priority bit position in
bitmap will be cleared when the queue becomes empty.

This reduces selection from O(n) to O(1) and remains existing function
calls.
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Validate with proper statistics.

@@ -84,6 +84,28 @@ typedef struct tcb {
void *rt_prio; /* Opaque pointer for custom real-time scheduler hook */
} tcb_t;

/* Scheduler attribution */
typedef struct sched {
volatile uint32_t ready_bitmap; /* 8-bit priority bitmap */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this variable set to volatile?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ISR might modify this value, so I added volatile to ensure bitmap always read directly from memory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit doubtful whether this is the right approach, since it sounds like you're using volatile to handle a synchronization problem. Even with the volatile keyword, accesses to the variable are not guaranteed to be atomic, so a race condition can still occur. Should we reconsider how synchronization is handled here? Maybe something like CRITICAL_ENTER/LEAVE() would be more appropriate.

Reference: https://docs.kernel.org/process/volatile-considered-harmful.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I appreciate your feedback and agree with your point of view. CRITICAL_ENTER()/LEAVE() method is a better approach to protect bitmap correctness. I'll make sure the bitmap manipulated in the critical section.

`sched_enqueue_task`/`sched_dequeue_task` also put/remove task
from corresponding prior queue and check the length of queue after
put/remove task to setup bitmap.

bitmap marco used here for simple operation purpose, it can be modified
for the further development.

The enqueue and dequeue process in `mo_task_spawn`/`sched_select_next_task`
are all replaced by `sched_enqueue_task` and `sched_dequeue_task`.
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.

3 participants