SMP Patches

[mrybczyn]

On Tue, 24 May 2011 17:37:44 +0200, Gedare Bloom <gedare at gwmail.gwu.edu>
wrote:
> 
> I do have a few concerns that you might be able to address
pre-emptively:
> * Naming: score naming convention pattern is _Package_Method so in C++
> package::method(), with only the first letter of the Method name to be
> capitalized. Some of the new functions being added by these patches do
> not follow the naming convention.

I think that this scheme requires an update/review for SMP. Part of this
is that we were not able to find an already-existing naming scheme for
SMP, so we appliedd our own.. If the scheme you mention is applied
directly, SMP will normally be the Package and the Method is the non-SMP
Package. That doesn't seem right. Additionally, it seems to me that the
relation between all _Thread functions is more important that the relation
between the SMP functions...

No strong preferences here, however, as long as a clear rule exists.

> 
> * API explosion: It isn't clear to me what the right way to extend the
> existing API functions should be. The approach taken so far has been
> to implement new functions that are called only when the SMP is
> configured, so that uniprocessor RTEMS is unaffected by SMP code. I
> glanced at the thread initialization patch (PR 1806) and it adds a new
> function that is wrapped by the existing _Thread_Initialize(...),
> which adds a function call to the uniprocessor RTEMS.

If the additional function call is a problem, we can easily add common
(SMP) inlined version and create uni- and multiprocessor version with
the help of the compiler.

> This is
> initialization so not a big deal, but a better pattern would be to
> separately initialize the SMP-related thread metadata, for example in
> cpukit/rtems/src/taskcreate.c:
>  status = _Thread_Initialize(...);
>  ...
>  #if defined(RTEMS_SMP)
>    status = _SMP_Thread_initialize(...);
>  #endif
> or something similar.

We won't avoid API explosion, I'm afraid. In this case (and some similar
ones) you either have one additional function that must be always run
after the main one (you need to change all calls and take care of all
new ones) or you add a new "main" function. The second approach
seems less bug-prone. Additionally, the affinity value is transfered
to the Thread_Control structure just like priority or scheduler
parameters, so why should it be set in a different place?

> 
> I will give detailed feedback on each PR when I can get to them.

Great. Thanks,

Marta