[Bug 1647] Modular SuperCore Scheduler

[bugzilla-daemon at rtems.org]

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

--- 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.
> 
OK.

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

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

> + 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.
> 
Gladly.

> + 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. 
> 
OK.

> 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.
>
Yes.

Thanks.

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