[Bug 1896] EDF scheduler implementation
bugzilla-daemon at rtems.org
bugzilla-daemon at rtems.org
Thu Sep 1 22:18:33 UTC 2011
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.
More information about the bugs
mailing list