From a616fb4e0a0245ae2cbd4614910480df19aa23d6 Mon Sep 17 00:00:00 2001 From: Michael Jeanson Date: Wed, 27 Jul 2022 14:23:41 -0400 Subject: [PATCH] Clarify terminolgy around cpu ids and array length Rename 'num_possible_cpus' to 'possible_cpus_array_len' to make it clearer that we use this value to create arrays of per-CPU elements. Change-Id: Ie5dc9293a95bf321f8add7e9c44ac677bc1fe539 Signed-off-by: Michael Jeanson Signed-off-by: Mathieu Desnoyers --- src/common/counter/counter.c | 16 ++-- src/common/ringbuffer/ring_buffer_frontend.c | 12 +-- src/common/smp.c | 41 +++++----- src/common/smp.h | 20 +++-- src/lib/lttng-ust-ctl/ustctl.c | 4 +- tests/unit/libcommon/test_smp.c | 78 ++++++++++---------- 6 files changed, 91 insertions(+), 80 deletions(-) diff --git a/src/common/counter/counter.c b/src/common/counter/counter.c index b71b43ae..60edad0c 100644 --- a/src/common/counter/counter.c +++ b/src/common/counter/counter.c @@ -118,7 +118,7 @@ int lttng_counter_set_cpu_shm(struct lib_counter *counter, int cpu, int fd) struct lib_counter_config *config = &counter->config; struct lib_counter_layout *layout; - if (cpu < 0 || cpu >= num_possible_cpus()) + if (cpu < 0 || cpu >= get_possible_cpus_array_len()) return -EINVAL; if (!(config->alloc & COUNTER_ALLOC_PER_CPU)) @@ -171,7 +171,7 @@ int validate_args(const struct lib_counter_config *config, int nr_counter_cpu_fds, const int *counter_cpu_fds) { - int nr_cpus = num_possible_cpus(); + int nr_cpus = get_possible_cpus_array_len(); if (CAA_BITS_PER_LONG != 64 && config->counter_size == COUNTER_SIZE_64_BIT) { WARN_ON_ONCE(1); @@ -210,7 +210,7 @@ struct lib_counter *lttng_counter_create(const struct lib_counter_config *config size_t dimension, nr_elem = 1; int cpu, ret; int nr_handles = 0; - int nr_cpus = num_possible_cpus(); + int nr_cpus = get_possible_cpus_array_len(); if (validate_args(config, nr_dimensions, max_nr_elem, global_sum_step, global_counter_fd, nr_counter_cpu_fds, @@ -309,7 +309,7 @@ int lttng_counter_get_cpu_shm(struct lib_counter *counter, int cpu, int *fd, siz struct lib_counter_layout *layout; int shm_fd; - if (cpu >= num_possible_cpus()) + if (cpu >= get_possible_cpus_array_len()) return -1; layout = &counter->percpu_counters[cpu]; shm_fd = layout->shm_fd; @@ -335,13 +335,13 @@ int lttng_counter_read(const struct lib_counter_config *config, switch (config->alloc) { case COUNTER_ALLOC_PER_CPU: - if (cpu < 0 || cpu >= num_possible_cpus()) + if (cpu < 0 || cpu >= get_possible_cpus_array_len()) return -EINVAL; layout = &counter->percpu_counters[cpu]; break; case COUNTER_ALLOC_PER_CPU | COUNTER_ALLOC_GLOBAL: if (cpu >= 0) { - if (cpu >= num_possible_cpus()) + if (cpu >= get_possible_cpus_array_len()) return -EINVAL; layout = &counter->percpu_counters[cpu]; } else { @@ -469,13 +469,13 @@ int lttng_counter_clear_cpu(const struct lib_counter_config *config, switch (config->alloc) { case COUNTER_ALLOC_PER_CPU: - if (cpu < 0 || cpu >= num_possible_cpus()) + if (cpu < 0 || cpu >= get_possible_cpus_array_len()) return -EINVAL; layout = &counter->percpu_counters[cpu]; break; case COUNTER_ALLOC_PER_CPU | COUNTER_ALLOC_GLOBAL: if (cpu >= 0) { - if (cpu >= num_possible_cpus()) + if (cpu >= get_possible_cpus_array_len()) return -EINVAL; layout = &counter->percpu_counters[cpu]; } else { diff --git a/src/common/ringbuffer/ring_buffer_frontend.c b/src/common/ringbuffer/ring_buffer_frontend.c index 91c69b76..817cc842 100644 --- a/src/common/ringbuffer/ring_buffer_frontend.c +++ b/src/common/ringbuffer/ring_buffer_frontend.c @@ -982,7 +982,7 @@ struct lttng_ust_shm_handle *channel_create(const struct lttng_ust_ring_buffer_c int64_t blocking_timeout_ms; if (config->alloc == RING_BUFFER_ALLOC_PER_CPU) - nr_streams = num_possible_cpus(); + nr_streams = get_possible_cpus_array_len(); else nr_streams = 1; @@ -1011,7 +1011,7 @@ struct lttng_ust_shm_handle *channel_create(const struct lttng_ust_ring_buffer_c return NULL; /* Allocate table for channel + per-cpu buffers */ - handle->table = shm_object_table_create(1 + num_possible_cpus()); + handle->table = shm_object_table_create(1 + get_possible_cpus_array_len()); if (!handle->table) goto error_table_alloc; @@ -1095,7 +1095,7 @@ struct lttng_ust_shm_handle *channel_handle_create(void *data, return NULL; /* Allocate table for channel + per-cpu buffers */ - handle->table = shm_object_table_create(1 + num_possible_cpus()); + handle->table = shm_object_table_create(1 + get_possible_cpus_array_len()); if (!handle->table) goto error_table_alloc; /* Add channel object */ @@ -1190,7 +1190,7 @@ struct lttng_ust_ring_buffer *channel_get_ring_buffer( if (config->alloc == RING_BUFFER_ALLOC_GLOBAL) { cpu = 0; } else { - if (cpu >= num_possible_cpus()) + if (cpu >= get_possible_cpus_array_len()) return NULL; } ref = &chan->backend.buf[cpu].shmp._ref; @@ -1235,7 +1235,7 @@ int ring_buffer_stream_close_wait_fd(const struct lttng_ust_ring_buffer_config * if (config->alloc == RING_BUFFER_ALLOC_GLOBAL) { cpu = 0; } else { - if (cpu >= num_possible_cpus()) + if (cpu >= get_possible_cpus_array_len()) return -EINVAL; } ref = &chan->backend.buf[cpu].shmp._ref; @@ -1253,7 +1253,7 @@ int ring_buffer_stream_close_wakeup_fd(const struct lttng_ust_ring_buffer_config if (config->alloc == RING_BUFFER_ALLOC_GLOBAL) { cpu = 0; } else { - if (cpu >= num_possible_cpus()) + if (cpu >= get_possible_cpus_array_len()) return -EINVAL; } ref = &chan->backend.buf[cpu].shmp._ref; diff --git a/src/common/smp.c b/src/common/smp.c index 5bcbad81..8bbf9f6f 100644 --- a/src/common/smp.c +++ b/src/common/smp.c @@ -25,7 +25,7 @@ #define __max(a,b) ((a)>(b)?(a):(b)) -static int num_possible_cpus_cache; +static int possible_cpus_array_len_cache; /* * As a fallback to parsing the CPU mask in "/sys/devices/system/cpu/possible", @@ -146,14 +146,14 @@ end: } /* - * Get the number of CPUs from the possible cpu mask. + * Get the highest CPU id from a CPU mask. * * pmask: the mask to parse. * len: the len of the mask excluding '\0'. * - * Returns the number of possible CPUs from the mask or 0 on error. + * Returns the highest CPU id from the mask or -1 on error. */ -int get_num_possible_cpus_from_mask(const char *pmask, size_t len) +int get_max_cpuid_from_mask(const char *pmask, size_t len) { ssize_t i; unsigned long cpu_index; @@ -179,13 +179,13 @@ int get_num_possible_cpus_from_mask(const char *pmask, size_t len) * CPUs. */ if ((&pmask[i] != endptr) && (cpu_index < INT_MAX)) - return (int) cpu_index + 1; + return (int) cpu_index; error: - return 0; + return -1; } -static void _get_num_possible_cpus(void) +static void update_possible_cpus_array_len_cache(void) { int ret; char buf[LTTNG_UST_CPUMASK_SIZE]; @@ -196,9 +196,12 @@ static void _get_num_possible_cpus(void) goto fallback; /* Parse the possible cpu mask, on failure fallback to sysconf. */ - ret = get_num_possible_cpus_from_mask((char *) &buf, ret); - if (ret > 0) + ret = get_max_cpuid_from_mask((char *) &buf, ret); + if (ret >= 0) { + /* Add 1 to convert from max cpuid to an array len. */ + ret++; goto end; + } fallback: /* Fallback to sysconf. */ @@ -209,20 +212,24 @@ end: if (ret < 1) return; - num_possible_cpus_cache = ret; + possible_cpus_array_len_cache = ret; } /* - * Returns the total number of CPUs in the system. If the cache is not yet - * initialized, get the value from "/sys/devices/system/cpu/possible" or - * fallback to sysconf and cache it. + * Returns the length of an array that could contain a per-CPU element for each + * possible CPU id for the lifetime of the process. + * + * We currently assume CPU ids are contiguous up the maximum CPU id. + * + * If the cache is not yet initialized, get the value from + * "/sys/devices/system/cpu/possible" or fallback to sysconf and cache it. * * If all methods fail, don't populate the cache and return 0. */ -int num_possible_cpus(void) +int get_possible_cpus_array_len(void) { - if (caa_unlikely(!num_possible_cpus_cache)) - _get_num_possible_cpus(); + if (caa_unlikely(!possible_cpus_array_len_cache)) + update_possible_cpus_array_len_cache(); - return num_possible_cpus_cache; + return possible_cpus_array_len_cache; } diff --git a/src/common/smp.h b/src/common/smp.h index 51fbd2d7..f908b6c6 100644 --- a/src/common/smp.h +++ b/src/common/smp.h @@ -30,27 +30,31 @@ int get_num_possible_cpus_fallback(void) __attribute__((visibility("hidden"))); /* - * Get the number of CPUs from the possible cpu mask. + * Get the highest CPU id from a CPU mask. * * pmask: the mask to parse. * len: the len of the mask excluding '\0'. * - * Returns the number of possible CPUs from the mask or 0 on error. + * Returns the highest CPU id from the mask or -1 on error. */ -int get_num_possible_cpus_from_mask(const char *pmask, size_t len) +int get_max_cpuid_from_mask(const char *pmask, size_t len) __attribute__((visibility("hidden"))); /* - * Returns the total number of CPUs in the system. If the cache is not yet - * initialized, get the value from "/sys/devices/system/cpu/possible" or - * fallback to sysconf and cache it. + * Returns the length of an array that could contain a per-CPU element for each + * possible CPU id for the lifetime of the process. + * + * We currently assume CPU ids are contiguous up the maximum CPU id. + * + * If the cache is not yet initialized, get the value from + * "/sys/devices/system/cpu/possible" or fallback to sysconf and cache it. * * If all methods fail, don't populate the cache and return 0. */ -int num_possible_cpus(void) +int get_possible_cpus_array_len(void) __attribute__((visibility("hidden"))); #define for_each_possible_cpu(cpu) \ - for ((cpu) = 0; (cpu) < num_possible_cpus(); (cpu)++) + for ((cpu) = 0; (cpu) < get_possible_cpus_array_len(); (cpu)++) #endif /* _UST_COMMON_SMP_H */ diff --git a/src/lib/lttng-ust-ctl/ustctl.c b/src/lib/lttng-ust-ctl/ustctl.c index bcbd9c66..6faaef5e 100644 --- a/src/lib/lttng-ust-ctl/ustctl.c +++ b/src/lib/lttng-ust-ctl/ustctl.c @@ -1384,7 +1384,7 @@ error: int lttng_ust_ctl_get_nr_stream_per_channel(void) { - return num_possible_cpus(); + return get_possible_cpus_array_len(); } struct lttng_ust_ctl_consumer_channel * @@ -2904,7 +2904,7 @@ int lttng_ust_ctl_regenerate_statedump(int sock, int handle) int lttng_ust_ctl_get_nr_cpu_per_counter(void) { - return num_possible_cpus(); + return get_possible_cpus_array_len(); } struct lttng_ust_ctl_daemon_counter * diff --git a/tests/unit/libcommon/test_smp.c b/tests/unit/libcommon/test_smp.c index 8f9a4042..7180076b 100644 --- a/tests/unit/libcommon/test_smp.c +++ b/tests/unit/libcommon/test_smp.c @@ -21,43 +21,43 @@ struct parse_test_data { }; static struct parse_test_data parse_test_data[] = { - { "", 0 }, - { "abc", 0 }, - { ",,,", 0 }, - { "--", 0 }, - { ",", 0 }, - { "-", 0 }, - { "2147483647", 0 }, - { "18446744073709551615", 0 }, - { "0-2147483647", 0 }, - { "0-18446744073709551615", 0 }, - { "0", 1 }, - { "1", 2 }, - { "0-1", 2 }, - { "1-3", 4 }, - { "0,2", 3 }, - { "1,2", 3 }, - { "0,4-6,127", 128 }, - { "0-4095", 4096 }, + { "", -1 }, + { "abc", -1 }, + { ",,,", -1 }, + { "--", -1 }, + { ",", -1 }, + { "-", -1 }, + { "2147483647", -1 }, + { "18446744073709551615", -1 }, + { "0-2147483647", -1 }, + { "0-18446744073709551615", -1 }, + { "0", 0 }, + { "1", 1 }, + { "0-1", 1 }, + { "1-3", 3 }, + { "0,2", 2 }, + { "1,2", 2 }, + { "0,4-6,127", 127 }, + { "0-4095", 4095 }, - { "\n", 0 }, - { "abc\n", 0 }, - { ",,,\n", 0 }, - { "--\n", 0 }, - { ",\n", 0 }, - { "-\n", 0 }, - { "2147483647\n", 0 }, - { "18446744073709551615\n", 0 }, - { "0-2147483647\n", 0 }, - { "0-18446744073709551615\n", 0 }, - { "0\n", 1 }, - { "1\n", 2 }, - { "0-1\n", 2 }, - { "1-3\n", 4 }, - { "0,2\n", 3 }, - { "1,2\n", 3 }, - { "0,4-6,127\n", 128 }, - { "0-4095\n", 4096 }, + { "\n", -1 }, + { "abc\n", -1 }, + { ",,,\n", -1 }, + { "--\n", -1 }, + { ",\n", -1 }, + { "-\n", -1 }, + { "2147483647\n", -1 }, + { "18446744073709551615\n", -1 }, + { "0-2147483647\n", -1 }, + { "0-18446744073709551615\n", -1 }, + { "0\n", 0 }, + { "1\n", 1 }, + { "0-1\n", 1 }, + { "1-3\n", 3 }, + { "0,2\n", 2 }, + { "1,2\n", 2 }, + { "0,4-6,127\n", 127 }, + { "0-4095\n", 4095 }, }; static int parse_test_data_len = sizeof(parse_test_data) / sizeof(parse_test_data[0]); @@ -71,14 +71,14 @@ int main(void) diag("Testing smp helpers"); for (i = 0; i < parse_test_data_len; i++) { - ret = get_num_possible_cpus_from_mask(parse_test_data[i].buf, + ret = get_max_cpuid_from_mask(parse_test_data[i].buf, strlen(parse_test_data[i].buf)); ok(ret == parse_test_data[i].expected, - "get_num_possible_cpus_from_mask '%s', expected: '%d', result: '%d'", + "get_max_cpuid_from_mask '%s', expected: '%d', result: '%d'", parse_test_data[i].buf, parse_test_data[i].expected, ret); } - ok(num_possible_cpus() > 0, "num_possible_cpus (%d > 0)", num_possible_cpus()); + ok(get_possible_cpus_array_len() > 0, "get_possible_cpus_array_len (%d > 0)", get_possible_cpus_array_len()); return exit_status(); } -- 2.34.1