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

[Jennifer Averett]

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.

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

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.