From: Olav Haugan ohaugan@codeaurora.org
Allow setting of cpusets at all levels of the spec.
Signed-off-by: Olav Haugan ohaugan@codeaurora.org Signed-off-by: Stephen Boyd stephen.boyd@linaro.org --- src/rt-app.c | 119 ++++++++++++++++++++++++++++++++++++++++------ src/rt-app_parse_config.c | 62 +++++++++++++++--------- src/rt-app_types.h | 12 ++++- 3 files changed, 155 insertions(+), 38 deletions(-)
diff --git a/src/rt-app.c b/src/rt-app.c index 0f992f5748c4..50922aa2ee03 100644 --- a/src/rt-app.c +++ b/src/rt-app.c @@ -448,6 +448,109 @@ shutdown(int sig) exit(EXIT_SUCCESS); }
+static void create_cpuset_str(cpu_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; + + 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, cpu_data_t *cpu_data) +{ + int ret; + cpu_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. Global 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_notice("[%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; @@ -475,20 +578,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) @@ -636,6 +725,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 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } }
+static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{ + struct json_object *cpuset_obj, *cpu; + + /* cpuset */ + cpuset_obj = get_in_object(obj, "cpus", TRUE); + if (cpuset_obj) { + struct array_list *cpuset; + 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); + cpuset = json_object_get_array(cpuset_obj); + 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; + data->cpusetsize = 0; + } + log_info(PIN "key: cpus %s", data->cpuset_str); +} + static void parse_thread_data(char *name, struct json_object *obj, int index, thread_data_t *data, rtapp_options_t *opts) { char *policy; char def_policy[RTAPP_POLICY_DESCR_LENGTH]; - struct array_list *cpuset; - struct json_object *cpuset_obj, *phases_obj, *cpu, *resources, *locks; - int i, cpu_idx, prior_def; + struct json_object *phases_obj, *resources, *locks; + int prior_def;
log_info(PFX "Parsing thread %s [%d]", name, index);
@@ -648,8 +677,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); @@ -683,23 +715,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)); - cpuset = json_object_get_array(cpuset_obj); - 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); @@ -726,6 +742,7 @@ parse_thread_data(char *name, struct json_object *obj, int index, foreach(phases_obj, entry, key, val, idx) { log_info(PIN "Parsing phase %s", key); parse_thread_phase_data(val, &data->phases[idx], opts, (long)data); + parse_cpuset_data(val, &data->phases[idx].cpu_data); }
/* Get loop number */ @@ -735,6 +752,7 @@ parse_thread_data(char *name, struct json_object *obj, int index, data->nphases = 1; data->phases = malloc(sizeof(phase_data_t) * data->nphases); parse_thread_phase_data(obj, &data->phases[0], opts, (long)data); + parse_cpuset_data(obj, &data->phases[0].cpu_data); /* Get loop number */ data->loop = 1; } diff --git a/src/rt-app_types.h b/src/rt-app_types.h index 5b8345fcf6aa..918a2ccc616c 100644 --- a/src/rt-app_types.h +++ b/src/rt-app_types.h @@ -123,10 +123,17 @@ typedef struct _event_data_t { int count; } event_data_t;
+typedef struct _cpu_data_t { + cpu_set_t *cpuset; + char *cpuset_str; + size_t cpusetsize; +} cpu_data_t; + typedef struct _phase_data_t { int loop; event_data_t *events; int nbevents; + cpu_data_t cpu_data; } phase_data_t;
typedef struct _thread_data_t { @@ -135,8 +142,9 @@ typedef struct _thread_data_t { int lock_pages; int duration; rtapp_resource_t **resources; - cpu_set_t *cpuset; - char *cpuset_str; + cpu_data_t cpu_data; /* cpu set information */ + cpu_data_t *curr_cpu_data; /* Current cpu set being used */ + cpu_data_t def_cpu_data; /* Default cpu set for task */
unsigned long runtime, deadline, period;
Hi Olav and Stephen,
I'm using a similar implementation for restricting the cpuset per phase. I ran your patch on https://github.com/scheduler-tools/rt-app.git master.
On 12/01/17 19:28, 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 Signed-off-by: Stephen Boyd stephen.boyd@linaro.org
[...]
+static void set_thread_affinity(thread_data_t *data, cpu_data_t *cpu_data) +{
- int ret;
- cpu_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. Global cpuset* 3. Default cpuset
Does Global mean here "global" config or "tasks" level? When I use
{ "global": { "calibration": 329, "duration": -1, "cpus": [1], }, "tasks": { "task0": { "loop": 2, "phases": { "p1": { "loop": 10, "run": 1000, "timer": { "period": 16000, "ref": "task0" } }, "p2": { "loop": 10, "run": 8000, "cpus": [8], "timer": { "period": 16000, "ref": "task0" } }, }, } } }
I get
root@juno:/# rt-app ~/test2.json [rt-app] <notice> pLoad = 329ns [rt-app] <notice> [0] Using SCHED_OTHER policy with priority 0 [rt-app] <notice> [0] starting thread ...
[rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 0, 1, 2, 3, 4, 5 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] Exiting.
But when I use:
{ "global": { "calibration": 329, "duration": -1, }, "tasks": { "task0": { "loop": 2, "cpus": [2], "phases": { "p1": { "loop": 10, "run": 1000, "timer": { "period": 16000, "ref": "task0" } }, "p2": { "loop": 10, "run": 8000, "cpus": [8], "timer": { "period": 16000, "ref": "task0" } }, }, } } }
I get:
root@juno:/# rt-app ~/test2.json [rt-app] <notice> pLoad = 329ns [rt-app] <notice> [0] Using SCHED_OTHER policy with priority 0 [rt-app] <notice> [0] starting thread ...
[rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 2 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 2 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] Exiting.
[...]
Hi Dietmar,
On 17-01-19 17:54:19, Dietmar Eggemann wrote:
Hi Olav and Stephen,
I'm using a similar implementation for restricting the cpuset per phase. I ran your patch on https://github.com/scheduler-tools/rt-app.git master.
On 12/01/17 19:28, 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 Signed-off-by: Stephen Boyd stephen.boyd@linaro.org
[...]
+static void set_thread_affinity(thread_data_t *data, cpu_data_t *cpu_data) +{
- int ret;
- cpu_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. Global cpuset* 3. Default cpusetDoes Global mean here "global" config or "tasks" level? When I use
{ "global": { "calibration": 329, "duration": -1, "cpus": [1], }, "tasks": { "task0": { "loop": 2, "phases": { "p1": { "loop": 10, "run": 1000, "timer": { "period": 16000, "ref": "task0" } }, "p2": { "loop": 10, "run": 8000, "cpus": [8], "timer": { "period": 16000, "ref": "task0" } }, }, } } }
I get
root@juno:/# rt-app ~/test2.json [rt-app] <notice> pLoad = 329ns [rt-app] <notice> [0] Using SCHED_OTHER policy with priority 0 [rt-app] <notice> [0] starting thread ...
[rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 0, 1, 2, 3, 4, 5 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] Exiting.
But when I use:
{ "global": { "calibration": 329, "duration": -1, }, "tasks": { "task0": { "loop": 2, "cpus": [2], "phases": { "p1": { "loop": 10, "run": 1000, "timer": { "period": 16000, "ref": "task0" } }, "p2": { "loop": 10, "run": 8000, "cpus": [8], "timer": { "period": 16000, "ref": "task0" } }, }, } } }
I get:
root@juno:/# rt-app ~/test2.json [rt-app] <notice> pLoad = 329ns [rt-app] <notice> [0] Using SCHED_OTHER policy with priority 0 [rt-app] <notice> [0] starting thread ...
[rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 2 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 2 ] [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 8 ] [rt-app] <notice> [0] Exiting.
[...]
Realized that "global" is not the right description. By global I meant at the task level. So you can specify either inside the phase (not supported before) or at task level (existing behavior). The example above is what I intended to support. Do you want me to change the description to say task level?
1. phase cpuset 2. task level cpuset 3. Default cpuset
On 21/01/17 01:32, Olav Haugan wrote:
Hi Dietmar,
On 17-01-19 17:54:19, Dietmar Eggemann wrote:
Hi Olav and Stephen,
[...]
Realized that "global" is not the right description. By global I meant at the task level. So you can specify either inside the phase (not supported before) or at task level (existing behavior). The example above is what I intended to support. Do you want me to change the description to say task level?
- phase cpuset
- task level cpuset
- Default cpuset
Would be nice. BTW, where do we document these changes for the humble rt-app user?
Another thing, by default we spit out a <notice> level log for each phase we change the affinity. Can we make this less noisy?
Couldn't you call parse_cpuset_data() from inside parse_thread_phase_data() instead of parse_thread_data() (for each phases or phase0)?
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 63f69b0c3a15..e58f62982e50 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -596,6 +596,36 @@ obj_is_event(char *name) return 0; }
+static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{ + struct json_object *cpuset_obj, *cpu; + + /* cpuset */ + cpuset_obj = get_in_object(obj, "cpus", TRUE); + if (cpuset_obj) { + struct array_list *cpuset; + 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); + cpuset = json_object_get_array(cpuset_obj); + 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; + 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) @@ -629,36 +659,9 @@ parse_thread_phase_data(struct json_object *obj, i++; } } -} - -static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) -{ - struct json_object *cpuset_obj, *cpu;
/* cpuset */ - cpuset_obj = get_in_object(obj, "cpus", TRUE); - if (cpuset_obj) { - struct array_list *cpuset; - 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); - cpuset = json_object_get_array(cpuset_obj); - 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; - data->cpusetsize = 0; - } - log_info(PIN "key: cpus %s", data->cpuset_str); + parse_cpuset_data(obj, &data->cpu_data); }
static void @@ -742,7 +745,6 @@ parse_thread_data(char *name, struct json_object *obj, int index, foreach(phases_obj, entry, key, val, idx) { log_info(PIN "Parsing phase %s", key); parse_thread_phase_data(val, &data->phases[idx], opts, (long)data); - parse_cpuset_data(val, &data->phases[idx].cpu_data); }
/* Get loop number */ @@ -752,7 +754,6 @@ parse_thread_data(char *name, struct json_object *obj, int index, data->nphases = 1; data->phases = malloc(sizeof(phase_data_t) * data->nphases); parse_thread_phase_data(obj, &data->phases[0], opts, (long)data); - parse_cpuset_data(obj, &data->phases[0].cpu_data); /* Get loop number */ data->loop = 1; }
Hi,
On 23/01/17 11:13, Dietmar Eggemann wrote:
On 21/01/17 01:32, Olav Haugan wrote:
Hi Dietmar,
On 17-01-19 17:54:19, Dietmar Eggemann wrote:
Hi Olav and Stephen,
[...]
Realized that "global" is not the right description. By global I meant at the task level. So you can specify either inside the phase (not supported before) or at task level (existing behavior). The example above is what I intended to support. Do you want me to change the description to say task level?
- phase cpuset
- task level cpuset
- Default cpuset
Would be nice. BTW, where do we document these changes for the humble rt-app user?
We usually improve doc/tutorial.txt.
Adding an example json file using the new feature would be really convenient, IMHO.
Thanks!
- Juri
On 17-01-23 11:13:04, Dietmar Eggemann wrote:
On 21/01/17 01:32, Olav Haugan wrote:
Hi Dietmar,
On 17-01-19 17:54:19, Dietmar Eggemann wrote:
Hi Olav and Stephen,
[...]
Realized that "global" is not the right description. By global I meant at the task level. So you can specify either inside the phase (not supported before) or at task level (existing behavior). The example above is what I intended to support. Do you want me to change the description to say task level?
- phase cpuset
- task level cpuset
- Default cpuset
Would be nice. BTW, where do we document these changes for the humble rt-app user?
Absolutely. Let me improve doc/tutorial.txt and add a json example.
Another thing, by default we spit out a <notice> level log for each phase we change the affinity. Can we make this less noisy?
Will do.
Couldn't you call parse_cpuset_data() from inside parse_thread_phase_data() instead of parse_thread_data() (for each phases or phase0)?
Will do.
[...]
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
@@ -475,20 +578,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) {
s/ret < 0/ret != 0/ ?
[...]
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } } +static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{
- struct json_object *cpuset_obj, *cpu;
- /* cpuset */
- cpuset_obj = get_in_object(obj, "cpus", TRUE);
- if (cpuset_obj) {
struct array_list *cpuset;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);
This allows for situations where the "cpus" field specifies cpus which are not available on the system. A "cpus": [15, 16, 17] on my 16 core systems is not detected as a faulty .json file. We should probe the max core # and compare it within the for loop and bail if we out of range. I guess using a json file on another platform with different topology is a quite common case.
cpuset = json_object_get_array(cpuset_obj);
cpuset is initialized here but never used. Just sent out a patch to fix this on master.
[...]
diff --git a/src/rt-app_types.h b/src/rt-app_types.h index 5b8345fcf6aa..918a2ccc616c 100644 --- a/src/rt-app_types.h +++ b/src/rt-app_types.h @@ -123,10 +123,17 @@ typedef struct _event_data_t { int count; } event_data_t; +typedef struct _cpu_data_t {
- cpu_set_t *cpuset;
- char *cpuset_str;
- size_t cpusetsize;
+} cpu_data_t;
Can we not call this structure _cpuset_data_t since it only contains cpuset related data.
[...]
On 17-01-25 13:26:12, Dietmar Eggemann wrote:
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
@@ -475,20 +578,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) {s/ret < 0/ret != 0/ ?
Ok.
[...]
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } } +static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{
- struct json_object *cpuset_obj, *cpu;
- /* cpuset */
- cpuset_obj = get_in_object(obj, "cpus", TRUE);
- if (cpuset_obj) {
struct array_list *cpuset;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);This allows for situations where the "cpus" field specifies cpus which are not available on the system. A "cpus": [15, 16, 17] on my 16 core systems is not detected as a faulty .json file. We should probe the max core # and compare it within the for loop and bail if we out of range. I guess using a json file on another platform with different topology is a quite common case.
ok, I can do that. The right way of getting max cpus available is sysconf(_SC_NPROCESSORS_CONF), right? I don't want to use _SC_NPROCESSORS_ONLN since some may be hotplugged out at the moment the check is performed.
cpuset = json_object_get_array(cpuset_obj);cpuset is initialized here but never used. Just sent out a patch to fix this on master.
[...]
diff --git a/src/rt-app_types.h b/src/rt-app_types.h index 5b8345fcf6aa..918a2ccc616c 100644 --- a/src/rt-app_types.h +++ b/src/rt-app_types.h @@ -123,10 +123,17 @@ typedef struct _event_data_t { int count; } event_data_t; +typedef struct _cpu_data_t {
- cpu_set_t *cpuset;
- char *cpuset_str;
- size_t cpusetsize;
+} cpu_data_t;
Can we not call this structure _cpuset_data_t since it only contains cpuset related data.
Yeah, that sounds good.
Thanks for the review.
[...]
On 26/01/17 00:03, Olav Haugan wrote:
On 17-01-25 13:26:12, Dietmar Eggemann wrote:
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } } +static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{
- struct json_object *cpuset_obj, *cpu;
- /* cpuset */
- cpuset_obj = get_in_object(obj, "cpus", TRUE);
- if (cpuset_obj) {
struct array_list *cpuset;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);This allows for situations where the "cpus" field specifies cpus which are not available on the system. A "cpus": [15, 16, 17] on my 16 core systems is not detected as a faulty .json file. We should probe the max core # and compare it within the for loop and bail if we out of range. I guess using a json file on another platform with different topology is a quite common case.
ok, I can do that. The right way of getting max cpus available is sysconf(_SC_NPROCESSORS_CONF), right? I don't want to use _SC_NPROCESSORS_ONLN since some may be hotplugged out at the moment the check is performed.
It depends what we want to allow to happen.
If we only consider a default target cpuset (including sparse cpumask due to hotplug) then we could work with a cpu_set_t rtapp_options_t::cpuset which we initialize via sched_getaffinity(0, sizeof(opts.cpuset), &opts.cpuset) before the parsing and then use it later to check the individual cpusets ("cpus": [ x,y,z ],) from the configuration.
Example: ARM JUNO w/ cpu4 hotplug-out
root@juno:~# cat /proc/cpuinfo | grep ^processor processor : 0 processor : 1 processor : 2 processor : 3 processor : 5
So a "cpus": [ 3, 4, 5 ] or [ 6 ] could be detected as an erroneous setup during parsing with this approach:
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json [rt-app] <crit> [json] Invalid cpuset [ 3, 4, 5 ]
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json [rt-app] <crit> [json] Invalid cpuset [ 6 ]
OTAH, "cpus": [ 3,5 ] would be fine:
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json ... [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 3, 5 ] ...
But this approach would fail if we allow rt-app being started with taskset imposed limitations for the main rt-app task:
root@juno:~# taskset -c 1,2 /root/devlib-target/bin/rt-app ~/test2.json [rt-app] <crit> [json] Invalid cpuset [ 3, 5 ]
We could argue though that the restriction to [ 1, 2 ] is global so [ 3, 5] for a task or task_phase should be detected as configuration error as well.
Opinions?
Cheers,
-- Dietmar
[...]
On 31/01/17 13:23, Dietmar Eggemann wrote:
On 26/01/17 00:03, Olav Haugan wrote:
On 17-01-25 13:26:12, Dietmar Eggemann wrote:
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } } +static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{
- struct json_object *cpuset_obj, *cpu;
- /* cpuset */
- cpuset_obj = get_in_object(obj, "cpus", TRUE);
- if (cpuset_obj) {
struct array_list *cpuset;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);This allows for situations where the "cpus" field specifies cpus which are not available on the system. A "cpus": [15, 16, 17] on my 16 core systems is not detected as a faulty .json file. We should probe the max core # and compare it within the for loop and bail if we out of range. I guess using a json file on another platform with different topology is a quite common case.
ok, I can do that. The right way of getting max cpus available is sysconf(_SC_NPROCESSORS_CONF), right? I don't want to use _SC_NPROCESSORS_ONLN since some may be hotplugged out at the moment the check is performed.
It depends what we want to allow to happen.
How about we check for hotplug events that might conflict with the specified mask at runtime, instead of at parsing time? And exit with an error or display a warining if we detect such an event (which in general might be just a transient state).
The check against max cpu available is something that we want to do once at parsing time instead, I guess.
Best,
- Juri
If we only consider a default target cpuset (including sparse cpumask due to hotplug) then we could work with a cpu_set_t rtapp_options_t::cpuset which we initialize via sched_getaffinity(0, sizeof(opts.cpuset), &opts.cpuset) before the parsing and then use it later to check the individual cpusets ("cpus": [ x,y,z ],) from the configuration.
Example: ARM JUNO w/ cpu4 hotplug-out
root@juno:~# cat /proc/cpuinfo | grep ^processor processor : 0 processor : 1 processor : 2 processor : 3 processor : 5
So a "cpus": [ 3, 4, 5 ] or [ 6 ] could be detected as an erroneous setup during parsing with this approach:
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json [rt-app] <crit> [json] Invalid cpuset [ 3, 4, 5 ]
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json [rt-app] <crit> [json] Invalid cpuset [ 6 ]
OTAH, "cpus": [ 3,5 ] would be fine:
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json ... [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 3, 5 ] ...
But this approach would fail if we allow rt-app being started with taskset imposed limitations for the main rt-app task:
root@juno:~# taskset -c 1,2 /root/devlib-target/bin/rt-app ~/test2.json [rt-app] <crit> [json] Invalid cpuset [ 3, 5 ]
We could argue though that the restriction to [ 1, 2 ] is global so [ 3, 5] for a task or task_phase should be detected as configuration error as well.
Opinions?
Cheers,
-- Dietmar
[...] _______________________________________________ Sched-tools mailing list Sched-tools@lists.linaro.org https://lists.linaro.org/mailman/listinfo/sched-tools
On 17-02-01 10:12:54, Juri Lelli wrote:
On 31/01/17 13:23, Dietmar Eggemann wrote:
On 26/01/17 00:03, Olav Haugan wrote:
On 17-01-25 13:26:12, Dietmar Eggemann wrote:
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } } +static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{
- struct json_object *cpuset_obj, *cpu;
- /* cpuset */
- cpuset_obj = get_in_object(obj, "cpus", TRUE);
- if (cpuset_obj) {
struct array_list *cpuset;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);This allows for situations where the "cpus" field specifies cpus which are not available on the system. A "cpus": [15, 16, 17] on my 16 core systems is not detected as a faulty .json file. We should probe the max core # and compare it within the for loop and bail if we out of range. I guess using a json file on another platform with different topology is a quite common case.
ok, I can do that. The right way of getting max cpus available is sysconf(_SC_NPROCESSORS_CONF), right? I don't want to use _SC_NPROCESSORS_ONLN since some may be hotplugged out at the moment the check is performed.
It depends what we want to allow to happen.
How about we check for hotplug events that might conflict with the specified mask at runtime, instead of at parsing time? And exit with an error or display a warining if we detect such an event (which in general might be just a transient state).
The check against max cpu available is something that we want to do once at parsing time instead, I guess.
Yes, lets not get too complicated here. Its not that critical. I can check against ONLN at parsetime.
If we only consider a default target cpuset (including sparse cpumask due to hotplug) then we could work with a cpu_set_t rtapp_options_t::cpuset which we initialize via sched_getaffinity(0, sizeof(opts.cpuset), &opts.cpuset) before the parsing and then use it later to check the individual cpusets ("cpus": [ x,y,z ],) from the configuration.
Example: ARM JUNO w/ cpu4 hotplug-out
root@juno:~# cat /proc/cpuinfo | grep ^processor processor : 0 processor : 1 processor : 2 processor : 3 processor : 5
So a "cpus": [ 3, 4, 5 ] or [ 6 ] could be detected as an erroneous setup during parsing with this approach:
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json [rt-app] <crit> [json] Invalid cpuset [ 3, 4, 5 ]
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json [rt-app] <crit> [json] Invalid cpuset [ 6 ]
OTAH, "cpus": [ 3,5 ] would be fine:
root@juno:~# /root/devlib-target/bin/rt-app ~/test.json ... [rt-app] <notice> [0] setting cpu affinity to CPU(s) [ 3, 5 ] ...
Agree with the above.
But this approach would fail if we allow rt-app being started with taskset imposed limitations for the main rt-app task:
root@juno:~# taskset -c 1,2 /root/devlib-target/bin/rt-app ~/test2.json [rt-app] <crit> [json] Invalid cpuset [ 3, 5 ]
We could argue though that the restriction to [ 1, 2 ] is global so [ 3, 5] for a task or task_phase should be detected as configuration error as well.
Opinions?
I would prefer not to have this restriction. Sometimes I really want the rt-app thread itself run on a separate CPU to not conflict with the actual usecase that I am trying to test using the json.
Thanks,
On 02/02/17 15:59, Olav Haugan wrote:
On 17-02-01 10:12:54, Juri Lelli wrote:
On 31/01/17 13:23, Dietmar Eggemann wrote:
On 26/01/17 00:03, Olav Haugan wrote:
On 17-01-25 13:26:12, Dietmar Eggemann wrote:
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
diff --git a/src/rt-app_parse_config.c b/src/rt-app_parse_config.c index 83794f35fe97..63f69b0c3a15 100644 --- a/src/rt-app_parse_config.c +++ b/src/rt-app_parse_config.c @@ -631,15 +631,44 @@ parse_thread_phase_data(struct json_object *obj, } } +static void parse_cpuset_data(struct json_object *obj, cpu_data_t *data) +{
- struct json_object *cpuset_obj, *cpu;
- /* cpuset */
- cpuset_obj = get_in_object(obj, "cpus", TRUE);
- if (cpuset_obj) {
struct array_list *cpuset;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);This allows for situations where the "cpus" field specifies cpus which are not available on the system. A "cpus": [15, 16, 17] on my 16 core systems is not detected as a faulty .json file. We should probe the max core # and compare it within the for loop and bail if we out of range. I guess using a json file on another platform with different topology is a quite common case.
ok, I can do that. The right way of getting max cpus available is sysconf(_SC_NPROCESSORS_CONF), right? I don't want to use _SC_NPROCESSORS_ONLN since some may be hotplugged out at the moment the check is performed.
It depends what we want to allow to happen.
How about we check for hotplug events that might conflict with the specified mask at runtime, instead of at parsing time? And exit with an error or display a warining if we detect such an event (which in general might be just a transient state).
The check against max cpu available is something that we want to do once at parsing time instead, I guess.
Yes, lets not get too complicated here. Its not that critical. I can check against ONLN at parsetime.
Maybe I managed to confuse myself, but don't we actually want to use CONF for the static parsing and checking once before start time?
Dealing with hotplug situations is something that we might want to add as e separate patch afterwards, IMHO.
[...]
But this approach would fail if we allow rt-app being started with taskset imposed limitations for the main rt-app task:
root@juno:~# taskset -c 1,2 /root/devlib-target/bin/rt-app ~/test2.json [rt-app] <crit> [json] Invalid cpuset [ 3, 5 ]
We could argue though that the restriction to [ 1, 2 ] is global so [ 3, 5] for a task or task_phase should be detected as configuration error as well.
Opinions?
I would prefer not to have this restriction. Sometimes I really want the rt-app thread itself run on a separate CPU to not conflict with the actual usecase that I am trying to test using the json.
Right, +1.
Thanks,
- Juri
On 02/03/2017 10:34 AM, Juri Lelli wrote:
On 02/02/17 15:59, Olav Haugan wrote:
On 17-02-01 10:12:54, Juri Lelli wrote:
On 31/01/17 13:23, Dietmar Eggemann wrote:
On 26/01/17 00:03, Olav Haugan wrote:
On 17-01-25 13:26:12, Dietmar Eggemann wrote:
On 12/01/17 19:28, Stephen Boyd wrote:
[...]
Yes, lets not get too complicated here. Its not that critical. I can check against ONLN at parsetime.
Maybe I managed to confuse myself, but don't we actually want to use CONF for the static parsing and checking once before start time?
Yes, we should use sysconf(_SC_NPROCESSORS_CONF) for boundary check only (no hotplug).
On my Juno with [0 1 2 3 5] online I get:
SYSC_sched_getaffinity=8 _SC_NPROCESSORS_CONF=6 _SC_NPROCESSORS_ONLN=5
So it seems that only CONF works here.
[...]
I would prefer not to have this restriction. Sometimes I really want the rt-app thread itself run on a separate CPU to not conflict with the actual usecase that I am trying to test using the json.
Right, +1.
Agreed.
On 17-02-03 11:08:38, Dietmar Eggemann wrote:
On 02/03/2017 10:34 AM, Juri Lelli wrote:
On 02/02/17 15:59, Olav Haugan wrote:
On 17-02-01 10:12:54, Juri Lelli wrote:
On 31/01/17 13:23, Dietmar Eggemann wrote:
On 26/01/17 00:03, Olav Haugan wrote:
On 17-01-25 13:26:12, Dietmar Eggemann wrote: > On 12/01/17 19:28, Stephen Boyd wrote:
[...]
Yes, lets not get too complicated here. Its not that critical. I can check against ONLN at parsetime.
Maybe I managed to confuse myself, but don't we actually want to use CONF for the static parsing and checking once before start time?
Yes, we should use sysconf(_SC_NPROCESSORS_CONF) for boundary check only (no hotplug).
On my Juno with [0 1 2 3 5] online I get:
SYSC_sched_getaffinity=8 _SC_NPROCESSORS_CONF=6 _SC_NPROCESSORS_ONLN=5
So it seems that only CONF works here.
[...]
I would prefer not to have this restriction. Sometimes I really want the rt-app thread itself run on a separate CPU to not conflict with the actual usecase that I am trying to test using the json.
Right, +1.
Agreed.
Great, I think we are in agreement. I will submit another patch for review.
Thanks.
-
Dietmar Eggemann -
Juri Lelli -
Olav Haugan -
Stephen Boyd