Hi all,
Does anyone have some feedback about this patch, please?
Best regards,
Quentin
2016-05-20 (11:20 +0200) ~ Quentin Monnet
> From: David Marchand <david.march...@6wind.com>
>
> Relying on /proc/cpuinfo to count the number of available cores is not
> the best option:
>
> - The code is x86-specific.
> - If the process is started with a different CPU affinity, then it will
> wrongly try to start too many threads (for an example, imagine an OVS
> daemon restricted to 4 CPU threads on a 128 threads system).
>
> This commit removes /proc/cpuinfo parsing, and introduces instead a call
> to sched_getaffinity(), which is architecture-independant, in order to
> retrieve the list of CPU threads available to the current process and to
> count them.
>
> Signed-off-by: David Marchand <david.march...@6wind.com>
> Signed-off-by: Liu Xiaofeng <xiaofeng....@6wind.com>
> Signed-off-by: Quentin Monnet <quentin.mon...@6wind.com>
> ---
> lib/ovs-thread.c | 75
> +++++++++-----------------------------------------------
> 1 file changed, 12 insertions(+), 63 deletions(-)
>
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index 3c065cf15fb7..f084ed1f72ad 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -545,67 +545,8 @@ ovs_thread_stats_next_bucket(const struct
> ovsthread_stats *stats, size_t i)
> }
>
>
> -/* Parses /proc/cpuinfo for the total number of physical cores on this system
> - * across all CPU packages, not counting hyper-threads.
> - *
> - * Sets *n_cores to the total number of cores on this system, or 0 if the
> +/* Returns the total number of cores available to this process, or 0 if the
> * number cannot be determined. */
> -static void
> -parse_cpuinfo(long int *n_cores)
> -{
> - static const char file_name[] = "/proc/cpuinfo";
> - char line[128];
> - uint64_t cpu = 0; /* Support up to 64 CPU packages on a single system. */
> - long int cores = 0;
> - FILE *stream;
> -
> - stream = fopen(file_name, "r");
> - if (!stream) {
> - VLOG_DBG("%s: open failed (%s)", file_name, ovs_strerror(errno));
> - return;
> - }
> -
> - while (fgets(line, sizeof line, stream)) {
> - unsigned int id;
> -
> - /* Find the next CPU package. */
> - if (ovs_scan(line, "physical id%*[^:]: %u", &id)) {
> - if (id > 63) {
> - VLOG_WARN("Counted over 64 CPU packages on this system. "
> - "Parsing %s for core count may be inaccurate.",
> - file_name);
> - cores = 0;
> - break;
> - }
> -
> - if (cpu & (1ULL << id)) {
> - /* We've already counted this package's cores. */
> - continue;
> - }
> - cpu |= 1ULL << id;
> -
> - /* Find the number of cores for this package. */
> - while (fgets(line, sizeof line, stream)) {
> - int count;
> -
> - if (ovs_scan(line, "cpu cores%*[^:]: %u", &count)) {
> - cores += count;
> - break;
> - }
> - }
> - }
> - }
> - fclose(stream);
> -
> - *n_cores = cores;
> -}
> -
> -/* Returns the total number of cores on this system, or 0 if the number
> cannot
> - * be determined.
> - *
> - * Tries not to count hyper-threads, but may be inaccurate - particularly on
> - * platforms that do not provide /proc/cpuinfo, but also if /proc/cpuinfo is
> - * formatted different to the layout that parse_cpuinfo() expects. */
> int
> count_cpu_cores(void)
> {
> @@ -614,9 +555,17 @@ count_cpu_cores(void)
>
> if (ovsthread_once_start(&once)) {
> #ifndef _WIN32
> - parse_cpuinfo(&n_cores);
> - if (!n_cores) {
> - n_cores = sysconf(_SC_NPROCESSORS_ONLN);
> + n_cores = sysconf(_SC_NPROCESSORS_ONLN);
> + if (n_cores > 0) {
> + cpu_set_t *set = CPU_ALLOC(n_cores);
> +
> + if (set) {
> + size_t size = CPU_ALLOC_SIZE(n_cores);
> +
> + if (!sched_getaffinity(0, size, set)) {
> + n_cores = CPU_COUNT_S(size, set);
> + }
> + }
> }
> #else
> SYSTEM_INFO sysinfo;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
