Thanks very much Christophe

This is great, I'll use this tech in
odph_odpthread_setaffinity()/getaffinity() APIs.

Do you mean you think keeping underlying Linux pthread/process concepts out
of ODP thread concept is not good?

From many discussions I got a Feeling that by ODP thread we are really
trying to provide a higher level execution abstraction above linux
pthread/process units, underlying linux pthread/process are like containers
or boxes for our componentized/re-usable code pieces?

I'm not sure if this feeling is right :), we can talk in ARCH if got time.

Best Regards, Yi

On 23 May 2016 at 16:48, Christophe Milard <christophe.mil...@linaro.org>
wrote:

> Hi,
>
> I guess (not 100% sure if that civer all cases) if gettid() == getpid()
> you are on the main process thread.
> Having said that I do not like this solution very much: It really looks to
> me that we have a larger problem trying to keep these things out of ODP
>
> Something for ARCH call, maybe?
>
> Christophe.
>
> On 23 May 2016 at 10:16, Yi He <yi...@linaro.org> wrote:
>
>> Hi, Christophe
>>
>> Here I met a difficulty, if I unified the API into 
>> odph_odpthread_set_affinity(),
>> inside the function how can I determine whether the current context is a
>> pthread or process? So I cannot decide to call sched_setaffinity() or
>> pthread_setaffinity_np().
>>
>> thanks and best regards, Yi
>>
>>
>>
>> On 23 May 2016 at 14:53, Yi He <yi...@linaro.org> wrote:
>>
>>> Hi, Christophe
>>>
>>> Yes, I'll apply your series and send a new one later.
>>>
>>> Best Regards, Yi
>>>
>>> On 23 May 2016 at 14:33, Christophe Milard <christophe.mil...@linaro.org
>>> > wrote:
>>>
>>>>
>>>>
>>>> On 20 May 2016 at 10:48, Yi He <yi...@linaro.org> wrote:
>>>>
>>>>> Set affinity to 1st available control cpu for all odp
>>>>> validation programs in odp_cunit_common library.
>>>>>
>>>>> Signed-off-by: Yi He <yi...@linaro.org>
>>>>> ---
>>>>>  helper/include/odp/helper/linux.h         | 47 +++++++++++++++++++
>>>>>  helper/linux.c                            | 32 +++++++++++++
>>>>>  helper/test/thread.c                      | 76
>>>>> +++++++++++++++++++++++++++++--
>>>>>  test/validation/common/odp_cunit_common.c | 15 ++++--
>>>>>  4 files changed, 164 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/helper/include/odp/helper/linux.h
>>>>> b/helper/include/odp/helper/linux.h
>>>>> index e2dca35..fa815e1 100644
>>>>> --- a/helper/include/odp/helper/linux.h
>>>>> +++ b/helper/include/odp/helper/linux.h
>>>>> @@ -84,6 +84,29 @@ int odph_linux_pthread_create(odph_linux_pthread_t
>>>>> *pthread_tbl,
>>>>>   */
>>>>>  void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int
>>>>> num);
>>>>>
>>>>> +/**
>>>>> + * Set CPU affinity of the current thread
>>>>> + *
>>>>> + * CPU affinity determines the set of CPUs on which the thread is
>>>>> + * eligible to run.
>>>>> + *
>>>>> + * @param cpuset        A bitmask lists the affinity CPU cores
>>>>> + *
>>>>> + * @return 0 on success, -1 on failure
>>>>> + */
>>>>> +int odph_linux_pthread_setaffinity(const odp_cpumask_t *cpuset);
>>>>>
>>>>
>>>> odph_odpthread_set_affinity(), is I guess better, at least as long as
>>>> we try to keep thread and processes together. Dropping the linux prefix
>>>> makes also the code more portable (on some other OS, change provide new
>>>> helpers and the app does hopefully not need to change)
>>>>
>>>> I guess you can apply the "running things in  process mode" patch
>>>> series to see what I am after...
>>>>
>>>> This comment applies to all your function names, of course.
>>>>
>>>> Christophe
>>>>
>>>> +
>>>>> +/**
>>>>> + * Get CPU affinity of the current thread
>>>>> + *
>>>>> + * CPU affinity determines the set of CPUs on which the thread is
>>>>> + * eligible to run.
>>>>> + *
>>>>> + * @param cpuset[out]   A bitmask lists the affinity CPU cores
>>>>> + *
>>>>> + * @return 0 on success, -1 on failure
>>>>> + */
>>>>> +int odph_linux_pthread_getaffinity(odp_cpumask_t *cpuset);
>>>>>
>>>>>  /**
>>>>>   * Fork a process
>>>>> @@ -134,6 +157,30 @@ int
>>>>> odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
>>>>>  int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int
>>>>> num);
>>>>>
>>>>>  /**
>>>>> + * Set CPU affinity of the current process
>>>>> + *
>>>>> + * CPU affinity determines the set of CPUs on which the process is
>>>>> + * eligible to run.
>>>>> + *
>>>>> + * @param cpuset        A bitmask lists the affinity CPU cores
>>>>> + *
>>>>> + * @return 0 on success, -1 on failure
>>>>> + */
>>>>> +int odph_linux_process_setaffinity(const odp_cpumask_t *cpuset);
>>>>> +
>>>>> +/**
>>>>> + * Get CPU affinity of the current process
>>>>> + *
>>>>> + * CPU affinity determines the set of CPUs on which the process is
>>>>> + * eligible to run.
>>>>> + *
>>>>> + * @param cpuset[out]   A bitmask lists the affinity CPU cores
>>>>> + *
>>>>> + * @return 0 on success, -1 on failure
>>>>> + */
>>>>> +int odph_linux_process_getaffinity(odp_cpumask_t *cpuset);
>>>>> +
>>>>> +/**
>>>>>   * @}
>>>>>   */
>>>>>
>>>>> diff --git a/helper/linux.c b/helper/linux.c
>>>>> index 24e243b..6ce7e7d 100644
>>>>> --- a/helper/linux.c
>>>>> +++ b/helper/linux.c
>>>>> @@ -114,6 +114,22 @@ void odph_linux_pthread_join(odph_linux_pthread_t
>>>>> *thread_tbl, int num)
>>>>>         }
>>>>>  }
>>>>>
>>>>> +int odph_linux_pthread_setaffinity(const odp_cpumask_t *cpuset)
>>>>> +{
>>>>> +       const cpu_set_t *_cpuset = &cpuset->set;
>>>>> +
>>>>> +       return (0 == pthread_setaffinity_np(pthread_self(),
>>>>> +               sizeof(cpu_set_t), _cpuset)) ? 0 : -1;
>>>>> +}
>>>>> +
>>>>> +int odph_linux_pthread_getaffinity(odp_cpumask_t *cpuset)
>>>>> +{
>>>>> +       cpu_set_t *_cpuset = &cpuset->set;
>>>>> +
>>>>> +       return (0 == pthread_getaffinity_np(pthread_self(),
>>>>> +               sizeof(cpu_set_t), _cpuset)) ? 0 : -1;
>>>>> +}
>>>>> +
>>>>>  int odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
>>>>>                               const odp_cpumask_t *mask,
>>>>>                               const odph_linux_thr_params_t
>>>>> *thr_params)
>>>>> @@ -236,3 +252,19 @@ int
>>>>> odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
>>>>>
>>>>>         return 0;
>>>>>  }
>>>>> +
>>>>> +int odph_linux_process_setaffinity(const odp_cpumask_t *cpuset)
>>>>> +{
>>>>> +       const cpu_set_t *_cpuset = &cpuset->set;
>>>>> +
>>>>> +       return (0 == sched_setaffinity(0, /* pid zero means calling
>>>>> process */
>>>>> +               sizeof(cpu_set_t), _cpuset)) ? 0 : -1;
>>>>> +}
>>>>> +
>>>>> +int odph_linux_process_getaffinity(odp_cpumask_t *cpuset)
>>>>> +{
>>>>> +       cpu_set_t *_cpuset = &cpuset->set;
>>>>> +
>>>>> +       return (0 == sched_getaffinity(0, /* pid zero means calling
>>>>> process */
>>>>> +               sizeof(cpu_set_t), _cpuset)) ? 0 : -1;
>>>>> +}
>>>>> diff --git a/helper/test/thread.c b/helper/test/thread.c
>>>>> index b290753..97b4331 100644
>>>>> --- a/helper/test/thread.c
>>>>> +++ b/helper/test/thread.c
>>>>> @@ -4,19 +4,51 @@
>>>>>   * SPDX-License-Identifier:     BSD-3-Clause
>>>>>   */
>>>>>
>>>>> +#include <string.h>
>>>>> +
>>>>>  #include <test_debug.h>
>>>>>  #include <odp_api.h>
>>>>>  #include <odp/helper/linux.h>
>>>>>
>>>>>  #define NUMBER_WORKERS 16
>>>>> +
>>>>> +/* delayed assertion after threads collection */
>>>>> +static int worker_results[NUMBER_WORKERS];
>>>>> +
>>>>>  static void *worker_fn(void *arg TEST_UNUSED)
>>>>>  {
>>>>> -       /* depend on the odp helper to call odp_init_local */
>>>>> +       odp_cpumask_t workers, affinity;
>>>>> +       /* save the thread result for delayed assertion */
>>>>> +       int *result = &worker_results[odp_cpu_id() % NUMBER_WORKERS];
>>>>>
>>>>> +       /* depend on the odp helper to call odp_init_local */
>>>>>         printf("Worker thread on CPU %d\n", odp_cpu_id());
>>>>>
>>>>> -       /* depend on the odp helper to call odp_term_local */
>>>>> +       odp_cpumask_zero(&workers);
>>>>> +       odp_cpumask_zero(&affinity);
>>>>> +
>>>>> +       odp_cpumask_default_worker(&workers, NUMBER_WORKERS);
>>>>> +
>>>>> +       /* verify affinity works */
>>>>> +       if (odph_linux_pthread_getaffinity(&affinity) != 0) {
>>>>> +               printf("Read worker thread affinity failed %d.\n",
>>>>> +                       odp_cpu_id());
>>>>> +               *result = -1;
>>>>> +       } else if (!odp_cpumask_isset(&workers,
>>>>> +                       odp_cpumask_first(&affinity))) {
>>>>> +               printf("Verify worker thread affinity failed %d.\n",
>>>>> +                       odp_cpu_id());
>>>>> +               *result = -1;
>>>>> +       }
>>>>> +
>>>>> +       /* verify API is not broken */
>>>>> +       if (odph_linux_pthread_setaffinity(&affinity) != 0) {
>>>>> +               printf("Re-configure worker thread affinity failed
>>>>> %d.\n",
>>>>> +                       odp_cpu_id());
>>>>> +               *result = -1;
>>>>> +       }
>>>>>
>>>>> +       /* depend on the odp helper to call odp_term_local */
>>>>>         return 0;
>>>>>  }
>>>>>
>>>>> @@ -24,7 +56,7 @@ static void *worker_fn(void *arg TEST_UNUSED)
>>>>>  int main(int argc TEST_UNUSED, char *argv[] TEST_UNUSED)
>>>>>  {
>>>>>         odph_linux_pthread_t thread_tbl[NUMBER_WORKERS];
>>>>> -       odp_cpumask_t cpu_mask;
>>>>> +       odp_cpumask_t cpu_mask, cpuset;
>>>>>         int num_workers;
>>>>>         int cpu;
>>>>>         char cpumaskstr[ODP_CPUMASK_STR_SIZE];
>>>>> @@ -41,6 +73,36 @@ int main(int argc TEST_UNUSED, char *argv[]
>>>>> TEST_UNUSED)
>>>>>                 exit(EXIT_FAILURE);
>>>>>         }
>>>>>
>>>>> +       /* reset all worker thread results to success */
>>>>> +       memset(worker_results, 0, sizeof(worker_results));
>>>>> +       odp_cpumask_zero(&cpu_mask);
>>>>> +       odp_cpumask_zero(&cpuset);
>>>>> +
>>>>> +       /* allocate the 1st available control cpu to main process */
>>>>> +       if (odp_cpumask_default_control(&cpu_mask, 1) != 1) {
>>>>> +               LOG_ERR("Allocate main process affinity failed.\n");
>>>>> +               exit(EXIT_FAILURE);
>>>>> +       }
>>>>> +       if (odph_linux_process_setaffinity(&cpu_mask) != 0) {
>>>>> +               LOG_ERR("Set main process affinify (%d) failed.\n",
>>>>> +                       odp_cpumask_first(&cpu_mask));
>>>>> +               exit(EXIT_FAILURE);
>>>>> +       }
>>>>> +       /* read back affinity to verify */
>>>>> +       if ((odph_linux_process_getaffinity(&cpuset) != 0) ||
>>>>> +               !odp_cpumask_equal(&cpu_mask, &cpuset)) {
>>>>> +               odp_cpumask_to_str(&cpuset,
>>>>> +                       cpumaskstr, sizeof(cpumaskstr));
>>>>> +
>>>>> +               LOG_ERR("Verify main process affinity failed: "
>>>>> +                       "set(%d) read(%s).\n",
>>>>> +                       odp_cpumask_first(&cpu_mask), cpumaskstr);
>>>>> +               exit(EXIT_FAILURE);
>>>>> +       }
>>>>> +
>>>>> +       odp_cpumask_zero(&cpuset);
>>>>> +       odp_cpumask_zero(&cpu_mask);
>>>>> +
>>>>>         /* discover how many threads this system can support */
>>>>>         num_workers = odp_cpumask_default_worker(&cpu_mask,
>>>>> NUMBER_WORKERS);
>>>>>         if (num_workers < NUMBER_WORKERS) {
>>>>> @@ -73,6 +135,14 @@ int main(int argc TEST_UNUSED, char *argv[]
>>>>> TEST_UNUSED)
>>>>>         odph_linux_pthread_create(&thread_tbl[0], &cpu_mask,
>>>>> &thr_params);
>>>>>         odph_linux_pthread_join(thread_tbl, num_workers);
>>>>>
>>>>> +       /* assert all worker thread results */
>>>>> +       for (cpu = 0; cpu < num_workers; cpu++) {
>>>>> +               if (worker_results[cpu] < 0) {
>>>>> +                       LOG_ERR("Worker thread %d failed.\n", cpu);
>>>>> +                       exit(EXIT_FAILURE);
>>>>> +               }
>>>>> +       }
>>>>> +
>>>>>         if (odp_term_local()) {
>>>>>                 LOG_ERR("Error: ODP local term failed.\n");
>>>>>                 exit(EXIT_FAILURE);
>>>>> diff --git a/test/validation/common/odp_cunit_common.c
>>>>> b/test/validation/common/odp_cunit_common.c
>>>>> index 2712abe..535a9f9 100644
>>>>> --- a/test/validation/common/odp_cunit_common.c
>>>>> +++ b/test/validation/common/odp_cunit_common.c
>>>>> @@ -329,9 +329,18 @@ int odp_cunit_update(odp_suiteinfo_t testsuites[])
>>>>>  int odp_cunit_register(odp_suiteinfo_t testsuites[])
>>>>>  {
>>>>>         /* call test executable init hook, if any */
>>>>> -       if (global_init_term.global_init_ptr &&
>>>>> -           ((*global_init_term.global_init_ptr)(&instance) != 0))
>>>>> -               return -1;
>>>>> +       if (global_init_term.global_init_ptr) {
>>>>> +               if ((*global_init_term.global_init_ptr)(&instance) ==
>>>>> 0) {
>>>>> +                       odp_cpumask_t cpuset;
>>>>> +
>>>>> +                       odp_cpumask_zero(&cpuset);
>>>>> +                       /* set main process affinity after ODP
>>>>> initialization */
>>>>> +                       if (1 == odp_cpumask_default_control(&cpuset,
>>>>> 1))
>>>>> +
>>>>>  odph_linux_process_setaffinity(&cpuset);
>>>>> +               } else {
>>>>> +                       return -1;
>>>>> +               }
>>>>> +       }
>>>>>
>>>>>         CU_set_error_action(CUEA_ABORT);
>>>>>
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>> _______________________________________________
>>>>> lng-odp mailing list
>>>>> lng-odp@lists.linaro.org
>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>>>>
>>>>
>>>>
>>>
>>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to