Thanks Justin - this patch looks very correct.

However I just have one question - in libc/cpu_set.cc we already have a
function __sched_cpucount() which is basically the same as your
CPU_COUNT_S().

The comment explains that the glibc macros CPU_COUNT() (and CPU_COUNT_S)
call this __sched_cpucount() function, which is why we need it.

So shouldn't we perhaps just do the same? Make CPU_COUNT and CPU_COUNT_S
macros (or inline functions) which calls this __sched_cpucount() function,
instead of replicating it?




--
Nadav Har'El
n...@scylladb.com

On Mon, Aug 8, 2016 at 6:52 AM, Justin Cinkelj <justin.cink...@xlab.si>
wrote:

> Signed-off-by: Justin Cinkelj <justin.cink...@xlab.si>
> ---
>  include/api/sched.h | 11 +++++++++--
>  libc/pthread.cc     |  2 +-
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/include/api/sched.h b/include/api/sched.h
> index 14f24de..9968a03 100644
> --- a/include/api/sched.h
> +++ b/include/api/sched.h
> @@ -5,6 +5,7 @@ extern "C" {
>  #endif
>
>  #include <features.h>
> +#include <assert.h>
>
>  #define __NEED_struct_timespec
>  #define __NEED_pid_t
> @@ -118,15 +119,21 @@ inline static int CPU_ISSET(size_t n, const
> cpu_set_t *cpuset) {
>      return (cpuset->__bits[__cpuset_word(n)] & __cpuset_mask(n)) != 0;
>  }
>
> -inline static int CPU_COUNT(const cpu_set_t *cpuset) {
> +inline static int CPU_COUNT_S(size_t setsize, const cpu_set_t *cpuset) {
>      size_t i;
>      int count = 0;
> -    for (i = 0; i < _NCPUWORDS; i++) {
> +    // CPU_ALLOC_SIZE should ensure we have whole words only
> +    assert((setsize % sizeof (__cpu_mask)) == 0);
> +    for (i = 0; i < setsize / sizeof (__cpu_mask); i++) {
>          count += __builtin_popcountl(cpuset->__bits[i]);
>      }
>      return count;
>  }
>
> +inline static int CPU_COUNT(const cpu_set_t *cpuset) {
> +    return CPU_COUNT_S(_NCPUWORDS*sizeof (__cpu_mask), cpuset);
> +}
> +
>  inline static size_t CPU_ALLOC_SIZE(size_t count) {
>      return ((count + __NCPUBITS - 1) / __NCPUBITS) * sizeof (__cpu_mask);
>  }
> diff --git a/libc/pthread.cc b/libc/pthread.cc
> index 1e6d357..ac7b3f0 100644
> --- a/libc/pthread.cc
> +++ b/libc/pthread.cc
> @@ -943,7 +943,7 @@ int pthread_attr_setaffinity_np(pthread_attr_t *attr,
> size_t cpusetsize,
>  static int setaffinity(sched::thread* t, size_t cpusetsize,
>          const cpu_set_t *cpuset)
>  {
> -    int count = CPU_COUNT(cpuset);
> +    int count = CPU_COUNT_S(cpusetsize, cpuset);
>      if (count == 0) {
>          // Having a cpuset with no CPUs in it is invalid.
>          return EINVAL;
> --
> 2.5.5
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to