Hi,
On 24/02/17 17:20, 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 Signed-off-by: Stephen Boyd stephen.boyd@linaro.org
Apart from the very minor comments below the patch looks good to me, so
Acked-by: Juri Lelli juri.lelli@arm.com
Could you please create a pull request on the github repo so that we can easily merge this?
[...]
diff --git a/doc/examples/tutorial/example7.json b/doc/examples/tutorial/example7.json new file mode 100644 index 000000000000..093e4ab3a93b --- /dev/null +++ b/doc/examples/tutorial/example7.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 0-3 (inherit from "cpus" at task level).
*/
- "tasks" : {
"thread0" : {
"cpus" : [0,1,2,3],
Do you think it would be simpler to put "cpus" : [2] here so that it's easier to check if default affinity is applied when looking at trace with kernelshark (task usually stays on CPU1 one during next phase otherwise).
"phases" : {
"start" : {
Maybe just call them "phase{1,2,3}"? Whay these names otherwise?
"cpus" : [0],
"loop" : 1,
"run" : 1500
},
"stop" : {
"cpus" : [1],
"loop" : 1,
"run" : 1500
},
"next" : {
"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
- }
+}
[...]
+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
Start comment on new line please.
Also, do we actually want to assert cpu_count < 10000 and exit otherwise?
* 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, 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:
Ditto.
* 1. Phase cpuset
* 2. Task level cpuset
* 3. Default cpuset
- */
White space damage?
- 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;
- }
+}
As said, no need to respin a new version IMHO, a pull request once comments above have been addressed or considered would work, but Vincent or others could have additional comments.
Thanks a lot for this.
Best,
- Juri