[Bug 1647] Modular SuperCore Scheduler

bugzilla-daemon at rtems.org bugzilla-daemon at rtems.org
Wed Aug 11 17:32:34 UTC 2010


--- Comment #8 from Gedare <giddyup44 at yahoo.com> 2010-08-11 12:32:33 CDT ---
(In reply to comment #7)
> Some first comments ..
> + Need list of files to add.  :-D
> + Need list of files to delete. :-D
For some reason the mkChangeLogList didn't come up with these, I'll work on it.

> + We should not end up with all schedulers in an executable.  AFAIK
> the user will select a single scheduler for their application.  In
> my mind, this means that each scheduler is basically represented
> by a table of methods and values.  Then when selecting a scheduler
> at application configuration time, we just fill in a pointer so it
> points to the right table. This is the same as the malloc configuration
> for plugins.  A single pointer to a table of methods.
This support exists. The confdefs.h Scheduler portion is modelled after the
Filesystem configuration.  A user can select a single scheduler (By default,
priority scheduler which maintains backward compatibility) by adding
CONFIGURE_SCHEDULER_XXX.  The other scheduler code is not bundled in the
executable.  When I merge another scheduler this will be more apparent.

I also added the ability to add all schedulers to an executable. I'm not sure
if there is any use case for this, in fact I don't know if it is even possible
to change the scheduler being used.

> + Move scheduler policy out of the Configuration Table and make it
> so the user is setting a variable that we can directly use in
> the Score.  This ties in to simplifying the initialization of the
> scheduler below where you set some fields.  
I'm a little unsure here.  Is it ok to use Configuration.scheduler_policy in
the Score?  Otherwise I can also add a variable that is initialized during
confdefs time.  I don't think the user needs to have access to this variable,
its value is tied explicitly to the choice of CONFIGURE_SCHEDULER_XXX.

> + Since this is not settled yet, I am not surprised there is no
> documentation but this will HAVE to be documented.  Complicating
> this in my mind, is that we should not only explain the mechanics
> for configuring a particular scheduler, we should explain the
> scheduler itself and when it is good and bad.  Pointers to papers, etc.
Yes, I will work on this shortly. :)

> + There should also be documentation of some sort on writing your
> own scheduler.
I have done some of this, I will probably do more when I merge alternate
schedulers, since that will clarify the process.  Is there a good place for
this documentation? i.e. should it be in the docs/, wiki, or source code?

> + Why is rq_ops abbreviated?  Please avoid abbreviations.
ok. would 'operations' be appropriate?  trying to avoid anything to 'wordy'.

> + Your Doxygen comments do not document the parameters.
I'll go back over them.

> + readyqueue.h sometimes has an extra blank comment line in Doxygen comments.
> Check all files.
OK. does enable-maintainer-mode build doxygen?

> + readyqueue.h has cases where parameters do not line up.  Please check
> all and fix.
I don't understand.

> + s_ops abbreviation.
Is 'operations' ok here too?

> + "the sched field" in comments.
what's the problem? should it just be sched?

> + I can't see how scheduler could be per-cpu.  But make this and all other
> references to doing this as a Doxygen "@note".
I'll change these to notes.  I don't know how it might be done either.

> + _Scheduler is a uniquely named variable per our _Package_Member style
> but it makes sense. :)
Yes, _Scheduler_Scheduler seemed a little silly.

> + schedulerpriority - _Sched_xxx, spell out.
These functions update the sched field, which is why they are named as
_Scheduler_priority_Sched_xxx.  Is there a better way to do this, or should i
rename the sched field to be 'scheduler' instead?

> + Only single blank lines.  There is a way to do this with a shell program
> easily.

> + Only single "blanks" in comments.  This could be..
> /*
>  *
> OR
>  *
>  *
OK I'll check these.

> + return (X) --> return X;

> + Check all copyrights for 2010.  I see 200x and 199x.  These appear 
> to be ones you renamed and moved but should be updated to 2010.
Will do.

> + I need to purge "/*PAGE" since it is from an old print formatting script
> we used in the dark days of RTEMS but please remove it from all of your
> files.

> + Please explain what's happening with the assignments into operation
> fields in schedulerpriority.c.  Are these the only implementation currently?
> Should the "Scheduler Configuration Structure" just have them?  
Other scheduler implementations will have other implementations.  Keeping this
separation will make it easier to merge the next scheduler and for me to
generate a good 'howto' on creating new schedulers.

> + Search for immediatly -- fix spelling (not your fault I think).
Ah, I didn't notice those, from the original code. I'll fix it. :)

> + score/Makefile.am - formatting of additions.  Looks like tabs not spaces.
nice spot I'll fix.

> + Comment on _Thread_Calculate_heir.  Says "returns" .. really sets 
> _Thread_Heir. 

> Most of this is style issues which should be easy to address.  The
> Configuration and presentation to the user are more difficult but 
> just need discussion and evaluation.


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