Patches for classic affinity implementation and test

[Joel Sherrill]

On 2/20/2014 3:11 PM, Gedare Bloom wrote:
> 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
Ok. But Chris has a sweep script pending for this and there is a URL
redirector
in place for the indefinite future.
> "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.)
If we drop the _, it could be _CPUSet_Xxx.
> 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.
No. It would be better if they were the same.
> +#if __RTEMS_HAVE_SYS_CPUSET_H__
> I think we decided to prefer the more explicit "#if defined(...)" for
> checking for CPP defines.
OK. But this is far from universal. There is a warning in gcc which
warns about this
and when i tried it, there were >10K warnings.
> Also, what is this define checking against / where is it defined?
config.h from cpukit/configure.ac
> _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.
No reason to include it.
> 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.
They are useful helpers and we didn't know what to do. We could make them
score named and add a wrapper. Is that OK with you?
> +#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.)?
The way the cpu set error checking is defined on Linux, it would be an
error to
ever set more than 1 bit. The user APIs COULD be present and the score code
pretty much be non-existent.

I don't know.
> +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?
The tests use it for debug output and it knows the implementation. I
don't mind
it being wrapped at the API level and being a score helper.

--joel
>
> 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
>>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel

-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available                (256) 722-9985