Re: [PATCH v2 bpf-next 2/2] bpf: use libbpf_num_possible_cpus in bpftool and selftests

[Song Liu <songliubraving@fb.com>]

> On Jun 6, 2019, at 9:58 AM, Hechao Li <hechaol@fb.com> wrote:
> 
> Use the newly added bpf_num_possible_cpus() in bpftool and selftests
> and remove duplicate implementations.
> 
> Signed-off-by: Hechao Li <hechaol@fb.com>
> ---
> tools/bpf/bpftool/common.c             | 53 +++-----------------------
> tools/testing/selftests/bpf/bpf_util.h | 37 +++---------------
> 2 files changed, 10 insertions(+), 80 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index f7261fad45c1..0b1c56758cd9 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -21,6 +21,7 @@
> #include <sys/vfs.h>
> 
> #include <bpf.h>
> +#include <libbpf.h> /* libbpf_num_possible_cpus */
> 
> #include "main.h"
> 
> @@ -439,57 +440,13 @@ unsigned int get_page_size(void)
> 
> unsigned int get_possible_cpus(void)
> {
> -	static unsigned int result;
> -	char buf[128];
> -	long int n;
> -	char *ptr;
> -	int fd;
> -
> -	if (result)
> -		return result;
> -
> -	fd = open("/sys/devices/system/cpu/possible", O_RDONLY);
> -	if (fd < 0) {
> -		p_err("can't open sysfs possible cpus");
> -		exit(-1);
> -	}
> -
> -	n = read(fd, buf, sizeof(buf));
> -	if (n < 2) {
> -		p_err("can't read sysfs possible cpus");
> -		exit(-1);
> -	}
> -	close(fd);
> +	int cpus = libbpf_num_possible_cpus();
> 
> -	if (n == sizeof(buf)) {
> -		p_err("read sysfs possible cpus overflow");
> +	if (cpus <= 0) {
> +		p_err("can't get # of possible cpus");
> 		exit(-1);
> 	}
> -
> -	ptr = buf;
> -	n = 0;
> -	while (*ptr && *ptr != '\n') {
> -		unsigned int a, b;
> -
> -		if (sscanf(ptr, "%u-%u", &a, &b) == 2) {
> -			n += b - a + 1;
> -
> -			ptr = strchr(ptr, '-') + 1;
> -		} else if (sscanf(ptr, "%u", &a) == 1) {
> -			n++;
> -		} else {
> -			assert(0);
> -		}
> -
> -		while (isdigit(*ptr))
> -			ptr++;
> -		if (*ptr == ',')
> -			ptr++;
> -	}
> -
> -	result = n;
> -
> -	return result;
> +	return cpus;
> }
> 
> static char *
> diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
> index a29206ebbd13..9ad9c7595f93 100644
> --- a/tools/testing/selftests/bpf/bpf_util.h
> +++ b/tools/testing/selftests/bpf/bpf_util.h
> @@ -6,44 +6,17 @@
> #include <stdlib.h>
> #include <string.h>
> #include <errno.h>
> +#include <libbpf.h>
> 
> static inline unsigned int bpf_num_possible_cpus(void)
> {
> -	static const char *fcpu = "/sys/devices/system/cpu/possible";
> -	unsigned int start, end, possible_cpus = 0;
> -	char buff[128];
> -	FILE *fp;
> -	int len, n, i, j = 0;
> +	int possible_cpus = libbpf_num_possible_cpus();
> 
> -	fp = fopen(fcpu, "r");
> -	if (!fp) {
> -		printf("Failed to open %s: '%s'!\n", fcpu, strerror(errno));
> +	if (possible_cpus <= 0) {
> +		printf("Failed to get # of possible cpus: '%s'!\n",
> +		       strerror(-possible_cpus));

strerror(0) is "Success". This is a little weird. 

Thanks,
Song

> 		exit(1);
> 	}
> -
> -	if (!fgets(buff, sizeof(buff), fp)) {
> -		printf("Failed to read %s!\n", fcpu);
> -		exit(1);
> -	}
> -
> -	len = strlen(buff);
> -	for (i = 0; i <= len; i++) {
> -		if (buff[i] == ',' || buff[i] == '\0') {
> -			buff[i] = '\0';
> -			n = sscanf(&buff[j], "%u-%u", &start, &end);
> -			if (n <= 0) {
> -				printf("Failed to retrieve # possible CPUs!\n");
> -				exit(1);
> -			} else if (n == 1) {
> -				end = start;
> -			}
> -			possible_cpus += end - start + 1;
> -			j = i + 1;
> -		}
> -	}
> -
> -	fclose(fp);
> -
> 	return possible_cpus;
> }
> 
> -- 
> 2.17.1
>