Hi,

On 16/03/17 14:42, Stephen Boyd wrote:
> From: Olav Haugan <ohaugan@codeaurora.org>
>
> Allow setting of cpusets at all levels of the spec.
>
> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
> Tested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Tested-by: Viresh Kumar <viresh.kumar@linaro.org>
> Acked-by: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

Thanks for addressing my comments.

Vincent, are you OK with me pushing this to master?

Thanks,

- Juri

> ---
>
> v4:
> -Rebased
> -Renamed example7.json to example8.json
> -Addressed comment formatting
> -Added check for # of cpus specified with bailout
> -Minor changes to example json
>
> v3:
> -Rebased
> -Removed unused cpuset and locks variable
> -Removed include unistd.h
>
> v2 changes:
> -Added documentation and example json
> -Clarified comments
> -Added check for invalid CPU
> -Reduced logging level for affinity setting message
> -Code fixes based on feedback
>
>  doc/examples/tutorial/example8.json |  40 ++++++++++++
>  doc/tutorial.txt                    |  37 +++++++++++
>  src/rt-app.c                        | 126 ++++++++++++++++++++++++++++++++----
>  src/rt-app_parse_config.c           |  64 ++++++++++++------
>  src/rt-app_types.h                  |  12 +++-
>  5 files changed, 243 insertions(+), 36 deletions(-)
>  create mode 100644 doc/examples/tutorial/example8.json
>
> diff --git a/doc/examples/tutorial/example8.json b/doc/examples/tutorial/example8.json
> new file mode 100644
> index 000000000000..9ffc594b4f47
> --- /dev/null
> +++ b/doc/examples/tutorial/example8.json
> @@ -0,0 +1,40 @@
> +{
> +     /*
> +      * Simple use case which creates a thread with 3 phases each running for
> +      * 1.5ms. In phase 1 the thread will be affined to CPU 0, in phase 2 it
> +      * will be affined to CPU 1 and in the third phase it will be affined to
> +      * CPUs 2 (inherit from "cpus" at task level).
> +      */
> +     "tasks" : {
> +             "thread0" : {
> +                     "cpus" : [2],
> +                     "phases" : {
> +                             "phase1" : {
> +                                     "cpus" : [0],
> +                                     "loop" : 1,
> +                                     "run" : 1500
> +                             },
> +                             "phase2" : {
> +                                     "cpus" : [1],
> +                                     "loop" : 1,
> +                                     "run" : 1500
> +                             },
> +                             "phase3" : {
> +                                     "loop" : 1,
> +                                     "run" : 1500
> +                             }
> +                     }
> +             }
> +     },
> +     "global" : {
> +             "duration" : 2,
> +             "calibration" : "CPU0",
> +             "default_policy" : "SCHED_OTHER",
> +             "pi_enabled" : false,
> +             "lock_pages" : false,
> +             "logdir" : "./",
> +             "log_basename" : "rt-app1",
> +             "ftrace" : true,
> +             "gnuplot" : true
> +     }
> +}
> diff --git a/doc/tutorial.txt b/doc/tutorial.txt
> index 3af2c57b2844..36d79ffe0d15 100644
> --- a/doc/tutorial.txt
> +++ b/doc/tutorial.txt
> @@ -232,6 +232,43 @@ below are equals:
>       }
>  }
>
> +"cpus" can be specified at task level or phase level. As an example, below
> +creates two threads. One thread will run with affinity of CPU 2 and 3. The
> +second task will run first phase with affinity to CPU 0 and 1, second phase with
> +affinity to CPU 2, and the third phase with affinity to CPU 4, 5, and 6 (it will
> +inherit the affinity from the task level):
> +
> +"task" : {
> +     "thread1" : {
> +             "cpus" : [2,3],
> +             "phases" : {
> +                     "phase1" : {
> +                             "run" : 10,
> +                             "sleep" : 10
> +                     }
> +             }
> +     }
> +     "thread2" : {
> +             "cpus" : [4,5,6],
> +             "phases" : {
> +                     "phase1" : {
> +                             "cpus" : [0,1],
> +                             "run" : 10,
> +                             "sleep" : 10
> +                     }
> +                     "phase2" : {
> +                             "cpus" : [2],
> +                             "run" : 10,
> +                             "sleep" : 10
> +                     }
> +                     "phase3" : {
> +                             "run" : 10,
> +                             "sleep" : 10
> +                     }
> +             }
> +     }
> +}
> +
>  * loop: Integer. Define the number of times the parent object must be run.
>  The parent object can be a phase or a thread. For phase object, the loop
>  defines the number of time the phase will be executed before starting the next
> diff --git a/src/rt-app.c b/src/rt-app.c
> index ec9710cfcc3f..981ed6f244c8 100644
> --- a/src/rt-app.c
> +++ b/src/rt-app.c
> @@ -474,6 +474,116 @@ shutdown(int sig)
>       exit(EXIT_SUCCESS);
>  }
>
> +static void create_cpuset_str(cpuset_data_t *cpu_data)
> +{
> +     unsigned int cpu_count = CPU_COUNT_S(cpu_data->cpusetsize,
> +                                                     cpu_data->cpuset);
> +     unsigned int i;
> +     unsigned int idx = 0;
> +
> +     /*
> +      * Assume we can go up to 9999 cpus. Each cpu would take up to 4 + 2
> +      * bytes (4 for the number and 2 for the comma and space). 2 bytes
> +      * for beginning bracket + space and 2 bytes for end bracket and space
> +      * and finally null-terminator.
> +      */
> +     unsigned int size_needed = cpu_count * 6 + 2 + 2 + 1;
> +
> +     if (cpu_count > 9999) {
> +             log_error("Too many cpus specified. Up to 9999 cpus supported");
> +             exit(EXIT_FAILURE);
> +     }
> +
> +     cpu_data->cpuset_str = malloc(size_needed);
> +     strcpy(cpu_data->cpuset_str, "[ ");
> +     idx += 2;
> +
> +     for (i = 0; i < 10000 && cpu_count; ++i) {
> +             unsigned int n;
> +
> +             if (CPU_ISSET(i, cpu_data->cpuset)) {
> +                     --cpu_count;
> +                     if (size_needed <= (idx + 1)) {
> +                             log_error("Not enough memory for array");
> +                             exit(EXIT_FAILURE);
> +                     }
> +                     n = snprintf(&cpu_data->cpuset_str[idx],
> +                                             size_needed - idx - 1, "%d", i);
> +                     if (n > 0) {
> +                             idx += n;
> +                     } else {
> +                             log_error("Error creating array");
> +                             exit(EXIT_FAILURE);
> +                     }
> +                     if (size_needed <= (idx + 1)) {
> +                             log_error("Not enough memory for array");
> +                             exit(EXIT_FAILURE);
> +                     }
> +                     if (cpu_count) {
> +                             strncat(cpu_data->cpuset_str, ", ",
> +                                                     size_needed - idx - 1);
> +                             idx += 2;
> +                     }
> +             }
> +     }
> +     strncat(cpu_data->cpuset_str, " ]", size_needed - idx - 1);
> +}
> +
> +static void set_thread_affinity(thread_data_t *data, cpuset_data_t *cpu_data)
> +{
> +     int ret;
> +     cpuset_data_t *actual_cpu_data = &data->cpu_data;
> +
> +     if (data->def_cpu_data.cpuset == NULL) {
> +             /* Get default affinity */
> +             cpu_set_t cpuset;
> +             unsigned int cpu_count;
> +             unsigned int cpu = 0;
> +
> +             ret = pthread_getaffinity_np(pthread_self(),
> +                                                 sizeof(cpu_set_t), &cpuset);
> +             if (ret != 0) {
> +                     errno = ret;
> +                     perror("pthread_get_affinity");
> +                     exit(EXIT_FAILURE);
> +             }
> +             cpu_count = CPU_COUNT(&cpuset);
> +             data->def_cpu_data.cpusetsize = CPU_ALLOC_SIZE(cpu_count);
> +             data->def_cpu_data.cpuset = CPU_ALLOC(cpu_count);
> +             memcpy(data->def_cpu_data.cpuset, &cpuset,
> +                                             data->def_cpu_data.cpusetsize);
> +             create_cpuset_str(&data->def_cpu_data);
> +             data->curr_cpu_data = &data->def_cpu_data;
> +     }
> +
> +     /*
> +      * Order of preference:
> +      * 1. Phase cpuset
> +      * 2. Task level cpuset
> +      * 3. Default cpuset
> +      */
> +     if (cpu_data->cpuset != NULL)
> +             actual_cpu_data = cpu_data;
> +
> +     if (actual_cpu_data->cpuset == NULL)
> +             actual_cpu_data = &data->def_cpu_data;
> +
> +     if (!CPU_EQUAL(actual_cpu_data->cpuset, data->curr_cpu_data->cpuset))
> +     {
> +             log_debug("[%d] setting cpu affinity to CPU(s) %s", data->ind,
> +                     actual_cpu_data->cpuset_str);
> +             ret = pthread_setaffinity_np(pthread_self(),
> +                                             actual_cpu_data->cpusetsize,
> +                                             actual_cpu_data->cpuset);
> +             if (ret != 0) {
> +                     errno = ret;
> +                     perror("pthread_setaffinity_np");
> +                     exit(EXIT_FAILURE);
> +             }
> +             data->curr_cpu_data = actual_cpu_data;
> +     }
> +}
> +
>  void *thread_body(void *arg)
>  {
>       thread_data_t *data = (thread_data_t*) arg;
> @@ -501,20 +611,6 @@ void *thread_body(void *arg)
>       /* Get the 1st phase's data */
>       pdata = &data->phases[0];
>
> -     /* Set thread affinity */
> -     if (data->cpuset != NULL)
> -     {
> -             log_notice("[%d] setting cpu affinity to CPU(s) %s", data->ind,
> -                      data->cpuset_str);
> -             ret = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t),
> -                                             data->cpuset);
> -             if (ret < 0) {
> -                     errno = ret;
> -                     perror("pthread_setaffinity_np");
> -                     exit(EXIT_FAILURE);
> -             }
> -     }
> -
>       /* Set scheduling policy and print pretty info on stdout */
>       log_notice("[%d] Using %s policy with priority %d", data->ind, data->sched_policy_descr, data->sched_prio);
>       switch (data->sched_policy)
> @@ -662,6 +758,8 @@ void *thread_body(void *arg)
>       while (continue_running && (i != data->loop)) {
>               struct timespec t_diff, t_rel_start;
>
> +             set_thread_affinity(data, &pdata->cpu_data);
> +
>               if (opts.ftrace)
>                       log_ftrace(ft_data.marker_fd, "[%d] begins loop %d phase %d step %d", data->ind, i, j, loop);
>               log_debug("[%d] begins loop %d phase %d step %d", data->ind, i, j, loop);;
> diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c
> index bdb11a6a2460..f0706ae6a13f 100644
> --- a/src/rt-app_parse_config.c
> +++ b/src/rt-app_parse_config.c
> @@ -648,6 +648,41 @@ obj_is_event(char *name)
>      return 0;
>  }
>
> +static void parse_cpuset_data(struct json_object *obj, cpuset_data_t *data)
> +{
> +     struct json_object *cpuset_obj, *cpu;
> +     unsigned int max_cpu = sysconf(_SC_NPROCESSORS_CONF) - 1;
> +
> +     /* cpuset */
> +     cpuset_obj = get_in_object(obj, "cpus", TRUE);
> +     if (cpuset_obj) {
> +             unsigned int i;
> +             unsigned int cpu_idx;
> +
> +             assure_type_is(cpuset_obj, obj, "cpus", json_type_array);
> +             data->cpuset_str = strdup(json_object_to_json_string(cpuset_obj));
> +             data->cpusetsize = sizeof(cpu_set_t);
> +             data->cpuset = malloc(data->cpusetsize);
> +             CPU_ZERO(data->cpuset);
> +             for (i = 0; i < json_object_array_length(cpuset_obj); i++) {
> +                     cpu = json_object_array_get_idx(cpuset_obj, i);
> +                     cpu_idx = json_object_get_int(cpu);
> +                     if (cpu_idx > max_cpu) {
> +                             log_critical(PIN2 "Invalid cpu %d in cpuset %s", cpu_idx, data->cpuset_str);
> +                             free(data->cpuset);
> +                             free(data->cpuset_str);
> +                             exit(EXIT_INV_CONFIG);
> +                     }
> +                     CPU_SET(cpu_idx, data->cpuset);
> +             }
> +     } else {
> +             data->cpuset_str = strdup("-");
> +             data->cpuset = NULL;
> +             data->cpusetsize = 0;
> +     }
> +     log_info(PIN "key: cpus %s", data->cpuset_str);
> +}
> +
>  static void
>  parse_thread_phase_data(struct json_object *obj,
>                 phase_data_t *data, rtapp_options_t *opts, long tag)
> @@ -681,6 +716,7 @@ parse_thread_phase_data(struct json_object *obj,
>                       i++;
>               }
>       }
> +     parse_cpuset_data(obj, &data->cpu_data);
>  }
>
>  static void
> @@ -689,8 +725,8 @@ parse_thread_data(char *name, struct json_object *obj, int index,
>  {
>       char *policy;
>       char def_policy[RTAPP_POLICY_DESCR_LENGTH];
> -     struct json_object *cpuset_obj, *phases_obj, *cpu, *resources, *locks;
> -     int i, cpu_idx, prior_def;
> +     struct json_object *phases_obj, *resources;
> +     int prior_def;
>
>       log_info(PFX "Parsing thread %s [%d]", name, index);
>
> @@ -699,8 +735,11 @@ parse_thread_data(char *name, struct json_object *obj, int index,
>       data->ind = index;
>       data->name = strdup(name);
>       data->lock_pages = opts->lock_pages;
> -     data->cpuset = NULL;
> -     data->cpuset_str = NULL;
> +     data->cpu_data.cpuset = NULL;
> +     data->cpu_data.cpuset_str = NULL;
> +     data->curr_cpu_data = NULL;
> +     data->def_cpu_data.cpuset = NULL;
> +     data->def_cpu_data.cpuset_str = NULL;
>
>       /* policy */
>       policy_to_string(opts->policy, def_policy);
> @@ -734,22 +773,7 @@ parse_thread_data(char *name, struct json_object *obj, int index,
>               data->deadline = get_int_value_from(obj, "deadline", TRUE, data->period);
>
>       /* cpuset */
> -     cpuset_obj = get_in_object(obj, "cpus", TRUE);
> -     if (cpuset_obj) {
> -             assure_type_is(cpuset_obj, obj, "cpus", json_type_array);
> -             data->cpuset_str = strdup(json_object_to_json_string(cpuset_obj));
> -             data->cpuset = malloc(sizeof(cpu_set_t));
> -             CPU_ZERO(data->cpuset);
> -             for (i = 0; i < json_object_array_length(cpuset_obj); i++) {
> -                     cpu = json_object_array_get_idx(cpuset_obj, i);
> -                     cpu_idx = json_object_get_int(cpu);
> -                     CPU_SET(cpu_idx, data->cpuset);
> -             }
> -     } else {
> -             data->cpuset_str = strdup("-");
> -             data->cpuset = NULL;
> -     }
> -     log_info(PIN "key: cpus %s", data->cpuset_str);
> +     parse_cpuset_data(obj, &data->cpu_data);
>
>       /* initial delay */
>       data->delay = get_int_value_from(obj, "delay", TRUE, 0);
> diff --git a/src/rt-app_types.h b/src/rt-app_types.h
> index 7f3ba58cfce9..06271c2d26c8 100644
> --- a/src/rt-app_types.h
> +++ b/src/rt-app_types.h
> @@ -141,10 +141,17 @@ typedef struct _event_data_t {
>       int count;
>  } event_data_t;
>
> +typedef struct _cpuset_data_t {
> +     cpu_set_t *cpuset;
> +     char *cpuset_str;
> +     size_t cpusetsize;
> +} cpuset_data_t;
> +
>  typedef struct _phase_data_t {
>       int loop;
>       event_data_t *events;
>       int nbevents;
> +     cpuset_data_t cpu_data;
>  } phase_data_t;
>
>  typedef struct _thread_data_t {
> @@ -153,8 +160,9 @@ typedef struct _thread_data_t {
>       int lock_pages;
>       int duration;
>       rtapp_resource_t **resources;
> -     cpu_set_t *cpuset;
> -     char *cpuset_str;
> +     cpuset_data_t cpu_data; /* cpu set information */
> +     cpuset_data_t *curr_cpu_data; /* Current cpu set being used */
> +     cpuset_data_t def_cpu_data; /* Default cpu set for task */
>
>       unsigned long runtime, deadline, period;
>
> --
> 2.10.0.297.gf6727b0
>