[PATCH 01/10] score: Add get/set affinity to Scheduler Framework.

[Sebastian Huber]

On 04/01/2014 07:42 PM, Jennifer Averett wrote:
> I just spoke with Joel and he had these comments to add:
>
> " I agree with Seb's comment on the default set/get behavior.
>
> If we can get by with a boolean return value, that's OK.
>
> Do a man pthread_setaffinity_np and check the errors listed.
> My response to Seb on that is that we must ensure that the
> API code follows the Linux code we are trying to be compatible
> with.
>
> I don't see the case moving base API checks into the schedulers.
> "
>
> The set affinity errors are
> EFAULT A supplied memory address was invalid.
> EINVAL (pthread_setaffinity_np()) The affinity bit mask mask contains no processors that are currently physically on the system and permitted to the thread
>                according to any restrictions that may be imposed by the "cpuset" mechanism described in cpuset(7).
> EINVAL (pthread_setaffinity_np()) cpuset specified a CPU that was outside the set supported by the kernel.  (The kernel configuration option CONFIG_NR_CPUS
>                defines the range of the set supported by the kernel data type used to represent CPU sets.)
> EINVAL (pthread_getaffinity_np()) cpusetsize is smaller than the size of the affinity mask used by the kernel.
> ESRCH  No thread with the ID thread could be found.
>
>
> I'd like to add for the classic we have
> RTEMS_INVALID_ADDRESS
> RTEMS_INVALID_NUMBER
> RTEMS_INVALID_ID
>
> At a minimum the NULL address check must be distinguished from the
> Cpuset having incorrect values.  If all of those checks get moved down into
> the scheduler a bool can not be used.

The trivial NULL pointer checks can be done in the high level 
functions.  The scheduler operations should not bother with NULL 
pointers.  I think all these NULL pointer checks are superfluous, but 
all right, we have this stuff now.

>
> Additionally, just as a side note, all the other scheduler routines do not return
> bool in a pass fail situation but return an int.

There is only one scheduler operation that returns an int, and this is 
the compare priority compare function.  A lot of compare functions use 
the same semantics.

>
> Jennifer
>
>> -----Original Message-----
>> From: Sebastian Huber [mailto:sebastian.huber at embedded-brains.de]
>> Sent: Tuesday, April 01, 2014 3:02 AM
>> To: Jennifer Averett
>> Cc: Gedare Bloom; RTEMS Devel
>> Subject: Re: [PATCH 01/10] score: Add get/set affinity to Scheduler
>> Framework.
>>
>> On 2014-04-01 15:50, Jennifer Averett wrote:
>>>>>>>>> +  /**
>>>>>>>>> +   * @brief Get affinity for the default scheduler.
>>>>>>>>> +   *
>>>>>>>>> +   * @param[in] thread The associated thread.
>>>>>>>>> +   * @param[in] cpusetsize The size of the cpuset.
>>>>>>>>> +   * @param[out] cpuset Affinity set containing all CPUs.
>>>>>>>>> +   *
>>>>>>>>> +   * @retval 0 Successfully got cpuset
>>>>>>>>> +   * @retval -1 The cpusetsize is invalid for the system
>>>>>>>>> +   */
>>>>>>>>> +  int _Scheduler_default_Get_affinity(
>>>>>>>>> +    Thread_Control *thread,
>>>>>>>>> +    size_t          cpusetsize,
>>>>>>>>> +    cpu_set_t      *cpuset
>>>>>>>>> +  );
>>>>>>>>> +
>>>>>>>>> +  /**
>>>>>>>>> +   * @brief Set affinity for the default scheduler.
>>>>>>>>> +   *
>>>>>>>>> +   * @param[in] thread The associated thread.
>>>>>>>>> +   * @param[in] cpusetsize The size of the cpuset.
>>>>>>>>> +   * @param[in] cpuset Affinity new affinity set.
>>>>>>>>> +   *
>>>>>>>>> +   * @retval 0 Successful
>>>>>>>>> +   *
>>>>>>>>> +   *  This method always returns successful and does not save
>>>>>>>>> +   *  the cpuset.
>>>>>>>>> +   */
>>>>>>>>> +  int _Scheduler_default_Set_affinity(
>>>>>>>>> +    Thread_Control *thread,
>>>>>>>>> +    size_t          cpusetsize,
>>>>>>>>> +    cpu_set_t      *cpuset
>>>>>>>>> +  );
>>>>>>>>> +#endif
>>>>>>>
>>>>>>> I would rather use bool or an enum for the return status instead
>>>>>>> of this int.
>>>>>>>
>>>>>>> The default set affinity operation should accept only CPU sets
>>>>>>> that specify the complete set of available processors, e.g. for a
>>>>>>> three processor system it should be bits 0, 1, and 2 set, all
>>>>>>> other bits cleared (e.g. (1 << CPU
>>>>>>> count) - 1).
>>>>>>>
>>>>>>> The default get affinity operation should return the CPU sets
>>>>>>> reflecting all available processors.
>>>>>>>
>>>>> I agree with the logic for the defaults Sebastian suggests.
>>> Actually no scheduler should be allowed to set the affinity to A cpu
>>> that the system doesn't support.  Currently those checks are called
>>> from the API methods.
>>>
>> I thought this is check in the high level code is only a temporary solution?
>> The responsibility for thread processor affinity management should move
>> entirely to the scheduler.
>>
>> --
>> Sebastian Huber, embedded brains GmbH
>>
>> Address : Dornierstr. 4, D-82178 Puchheim, Germany
>> Phone   : +49 89 189 47 41-16
>> Fax     : +49 89 189 47 41-09
>> E-Mail  : sebastian.huber at embedded-brains.de
>> PGP     : Public key available on request.
>>
>> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.


-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.