[Bug 1896] EDF scheduler implementation

[bugzilla-daemon at rtems.org]

https://www.rtems.org/bugzilla/show_bug.cgi?id=1896

--- Comment #3 from Joel Sherrill <joel.sherrill at oarcorp.com> 2011-09-01 17:18:31 CDT ---
Please address and update patch.

I would like to get this merged.

Also need some documentation.

(In reply to comment #2)
> Some more notes:
> 
> I would change
> 
> +typedef struct {
> +  /**
> +   * Pointer to corresponding Thread Control Block.
> +   */
> +  Thread_Control *the_thread;
> +  /**
> +   * Rbtree node related to this thread.
> +   */
> +  RBTree_Node the_node;
> +  /**
> +   * State of the thread with respect to ready queue.
> +   */
> +  Scheduler_EDF_Queue_state is_enqueued;
> +} Scheduler_EDF_Per_thread;
> 
> into
> 
> +typedef struct {
> +  RBTree_Node Node;
> +  Thread_Control *thread;
> +  Scheduler_EDF_Queue_state queue_state;
> +} Scheduler_EDF_Per_thread;

Why remove the comments.  Just make them one line each.

> +extern RBTree_Control _Scheduler_EDF_Ready_queue;
> 
> Maybe use SCORE_EXTERN here?

Up until we started getting optional schedulers, this would have been the
answer.  You don't want this scheduler's data in a default build.  So either
extern it or use something like SCHEDULER_EDF_EXTERN and mimic the magic.
Since it is only one data item, it is fine to me as extern.


> Please avoid TRUE and FALSE.

Definitely.

And Marta's

> 1. Defining YES in an enum may be risky, huge risk of a name conflict.

Agreed.  Use fully namespace safe names and even adding SCHEDULER_EDF_
to the front isn't helping this.  Is there a better way to name it?

> 2. _Scheduler_EDF_Allocate will crash/overwrite memory if _Workspace_Allocate
> fails.

Agreed.  Dangerous not to check and needs to return NULL on failure.

> 3. Scheduler_EDF_Allocate. Wouldn't it be easier to read with
> schinfo = sched; 
> instead of the cast of just set variable?

Yes and the way it is now assumes the compiler will optimize out the extra
memory operation.

-- 
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.