Patches for classic affinity implementation and test

[Gedare Bloom]

I mostly focused on the 0002-score... file. The others seemed ok at a
glance... with respect to the 0002 patch I have the following:

Copyright should only be applied to code from the time it is written.
We should avoid copy-paste and extending copyright into the past.

fix the license address as rtems.org not rtems.com

"thread cpuset Handler" should be given a properly capitalized name,
such as "Thread CPU Set Handler". The Doxygen mnemonic should be
similarly defined like ScoreThreadCpuSet. However, I think the Thread
can be dropped from the name and we just call it the Cpu Set handler.

I would prefer to see symmetry between our choice to use cpu_set_t as
the opaque type for the cpu bit sets. Thus I recommend using the score
name _Cpu_set for the handler. (It is unfortunate that we can't use
_CPU_set because that would clash with the score CPU namespace.)

Is there a reason not to use the same names in the score
Cpuset_Control as in the pthread_attr_t? i.e. affinitysetsize,
affinityset, and affinitysetpreallocated.

+#if __RTEMS_HAVE_SYS_CPUSET_H__
I think we decided to prefer the more explicit "#if defined(...)" for
checking for CPP defines.
Also, what is this define checking against / where is it defined?

_Cpuset_Handler_is_valid():
Why include "Handler" in the function name? same goes for other
functions in this file. The only one that usually gets "Handler" is
the initialization function.

cpusetimpl.h:
this file seemingly includes some public API functions (in the
rtems_cpuset namespace), which it should not do. Either make these
functions internal to score, or provide them in a classic API handler.

+#if __RTEMS_HAVE_SYS_CPUSET_H__ && defined( RTEMS_SMP )
Is there no valid use case for cpu_set in uniprocessor mode? what if
an application can work in either uni or multicore setting, and it
determines thread placement in multicore with affinity (and in single
core it will just end up with one cpu available.)?

+static Cpuset_Control _Default_Cpuset;
Should use some local/file scope name for this variable if you like.

+#if HAVE_SYS_CPUSET_H
Here is another define that is different from the others. where is it
defined, and do we really need to check for it?


+  int max_cpus = 1;
+
+  /* We do not support a cpu count over CPU_SETSIZE  */
+  max_cpus = _SMP_Get_processor_count();
the initialization of max_cpus = 1 is unnecessary.

+ * _Affinity_Handler_is_valid
...
+int _Cpuset_Handler_is_valid(
mismatch between doco and code.

for Cpuset_Handler_is_valid() does it make sense to return more
informative error codes? I'm aware the posix_nr implementation does
not require it, but the classic interface could use it to make easier
debugging.

Do we need this cpusetprintsupport.c code?


On Thu, Feb 20, 2014 at 3:07 PM, Jennifer Averett
<Jennifer.Averett at oarcorp.com> wrote:
>
> Attached is a set of patches for review that implement classic affinity.
>
> Jennifer Averett
> On-Line Applications Research
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel
>