* [PATCH] bpf: selftests: handle sparse CPU allocations
@ 2019-01-30 9:19 Martynas Pumputis
2019-01-31 5:25 ` Y Song
0 siblings, 1 reply; 3+ messages in thread
From: Martynas Pumputis @ 2019-01-30 9:19 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, m
Previously, bpf_num_possible_cpus() had a bug when calculating a
number of possible CPUs in the case of sparse CPU allocations, as
it was considering only the first range or element of
/sys/devices/system/cpu/possible.
E.g. in the case of "0,2-3" (CPU 1 is not available), the function
returned 1 instead of 3.
This patch fixes the function by making it parse all CPU ranges and
elements.
Signed-off-by: Martynas Pumputis <m@lambda.lt>
---
tools/testing/selftests/bpf/bpf_util.h | 29 +++++++++++++++++---------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
index 315a44fa32af..8cab50408204 100644
--- a/tools/testing/selftests/bpf/bpf_util.h
+++ b/tools/testing/selftests/bpf/bpf_util.h
@@ -13,7 +13,7 @@ static inline unsigned int bpf_num_possible_cpus(void)
unsigned int start, end, possible_cpus = 0;
char buff[128];
FILE *fp;
- int n;
+ int n, i, j = 0;
fp = fopen(fcpu, "r");
if (!fp) {
@@ -21,17 +21,26 @@ static inline unsigned int bpf_num_possible_cpus(void)
exit(1);
}
- while (fgets(buff, sizeof(buff), fp)) {
- n = sscanf(buff, "%u-%u", &start, &end);
- if (n == 0) {
- printf("Failed to retrieve # possible CPUs!\n");
- exit(1);
- } else if (n == 1) {
- end = start;
+ if (!fgets(buff, sizeof(buff), fp)) {
+ printf("Failed to read %s!\n", fcpu);
+ exit(1);
+ }
+
+ for (i = 0; i <= strlen(buff); 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;
}
- possible_cpus = start == 0 ? end + 1 : 0;
- break;
}
+
fclose(fp);
return possible_cpus;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: selftests: handle sparse CPU allocations
2019-01-30 9:19 [PATCH] bpf: selftests: handle sparse CPU allocations Martynas Pumputis
@ 2019-01-31 5:25 ` Y Song
2019-01-31 8:57 ` Martynas
0 siblings, 1 reply; 3+ messages in thread
From: Y Song @ 2019-01-31 5:25 UTC (permalink / raw)
To: Martynas Pumputis; +Cc: netdev, Alexei Starovoitov, Daniel Borkmann
[ My reply somehow rejected by netdev, this is to send it again. ]
On Wed, Jan 30, 2019 at 1:19 AM Martynas Pumputis <m@lambda.lt> wrote:
>
> Previously, bpf_num_possible_cpus() had a bug when calculating a
> number of possible CPUs in the case of sparse CPU allocations, as
> it was considering only the first range or element of
> /sys/devices/system/cpu/possible.
>
> E.g. in the case of "0,2-3" (CPU 1 is not available), the function
> returned 1 instead of 3.
>
> This patch fixes the function by making it parse all CPU ranges and
> elements.
>
> Signed-off-by: Martynas Pumputis <m@lambda.lt>
> ---
> tools/testing/selftests/bpf/bpf_util.h | 29 +++++++++++++++++---------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
> index 315a44fa32af..8cab50408204 100644
> --- a/tools/testing/selftests/bpf/bpf_util.h
> +++ b/tools/testing/selftests/bpf/bpf_util.h
> @@ -13,7 +13,7 @@ static inline unsigned int bpf_num_possible_cpus(void)
> unsigned int start, end, possible_cpus = 0;
> char buff[128];
> FILE *fp;
> - int n;
> + int n, i, j = 0;
>
> fp = fopen(fcpu, "r");
> if (!fp) {
> @@ -21,17 +21,26 @@ static inline unsigned int bpf_num_possible_cpus(void)
> exit(1);
> }
>
> - while (fgets(buff, sizeof(buff), fp)) {
> - n = sscanf(buff, "%u-%u", &start, &end);
> - if (n == 0) {
> - printf("Failed to retrieve # possible CPUs!\n");
> - exit(1);
> - } else if (n == 1) {
> - end = start;
> + if (!fgets(buff, sizeof(buff), fp)) {
> + printf("Failed to read %s!\n", fcpu);
> + exit(1);
> + }
> +
> + for (i = 0; i <= strlen(buff); i++) {
> + if (buff[i] == ',' || buff[i] == '\0') {
> + buff[i] = '\0';
This does not sound right. For example, the cpu list "0,2-3",
you will change "," to '\0" so buffer becomes "0\02-3".
The next iteration you will get strlen(buff) = 1.
The "2-3" will be skipped.
> + 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;
> }
> - possible_cpus = start == 0 ? end + 1 : 0;
> - break;
> }
> +
> fclose(fp);
>
> return possible_cpus;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: selftests: handle sparse CPU allocations
2019-01-31 5:25 ` Y Song
@ 2019-01-31 8:57 ` Martynas
0 siblings, 0 replies; 3+ messages in thread
From: Martynas @ 2019-01-31 8:57 UTC (permalink / raw)
To: Y Song; +Cc: netdev, Alexei Starovoitov, Daniel Borkmann
Oops, thanks for noticing the issue with strlen in the for loop. Changing to "int len = strlen(buff); for (i = 0; i < len; i++) <..>" should fix the issue. I'm going to re-submit the patch.
Also, I put some testing results: https://gist.github.com/brb/5369b5cfd08babb80cf2c4081dc19762
On Thu, Jan 31, 2019, at 7:25 AM, Y Song wrote:
> [ My reply somehow rejected by netdev, this is to send it again. ]
>
> On Wed, Jan 30, 2019 at 1:19 AM Martynas Pumputis <m@lambda.lt> wrote:
> >
> > Previously, bpf_num_possible_cpus() had a bug when calculating a
> > number of possible CPUs in the case of sparse CPU allocations, as
> > it was considering only the first range or element of
> > /sys/devices/system/cpu/possible.
> >
> > E.g. in the case of "0,2-3" (CPU 1 is not available), the function
> > returned 1 instead of 3.
> >
> > This patch fixes the function by making it parse all CPU ranges and
> > elements.
> >
> > Signed-off-by: Martynas Pumputis <m@lambda.lt>
> > ---
> > tools/testing/selftests/bpf/bpf_util.h | 29 +++++++++++++++++---------
> > 1 file changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
> > index 315a44fa32af..8cab50408204 100644
> > --- a/tools/testing/selftests/bpf/bpf_util.h
> > +++ b/tools/testing/selftests/bpf/bpf_util.h
> > @@ -13,7 +13,7 @@ static inline unsigned int bpf_num_possible_cpus(void)
> > unsigned int start, end, possible_cpus = 0;
> > char buff[128];
> > FILE *fp;
> > - int n;
> > + int n, i, j = 0;
> >
> > fp = fopen(fcpu, "r");
> > if (!fp) {
> > @@ -21,17 +21,26 @@ static inline unsigned int bpf_num_possible_cpus(void)
> > exit(1);
> > }
> >
> > - while (fgets(buff, sizeof(buff), fp)) {
> > - n = sscanf(buff, "%u-%u", &start, &end);
> > - if (n == 0) {
> > - printf("Failed to retrieve # possible CPUs!\n");
> > - exit(1);
> > - } else if (n == 1) {
> > - end = start;
> > + if (!fgets(buff, sizeof(buff), fp)) {
> > + printf("Failed to read %s!\n", fcpu);
> > + exit(1);
> > + }
> > +
> > + for (i = 0; i <= strlen(buff); i++) {
> > + if (buff[i] == ',' || buff[i] == '\0') {
> > + buff[i] = '\0';
>
> This does not sound right. For example, the cpu list "0,2-3",
> you will change "," to '\0" so buffer becomes "0\02-3".
> The next iteration you will get strlen(buff) = 1.
> The "2-3" will be skipped.
>
> > + 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;
> > }
> > - possible_cpus = start == 0 ? end + 1 : 0;
> > - break;
> > }
> > +
> > fclose(fp);
> >
> > return possible_cpus;
> > --
> > 2.20.1
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-31 8:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 9:19 [PATCH] bpf: selftests: handle sparse CPU allocations Martynas Pumputis
2019-01-31 5:25 ` Y Song
2019-01-31 8:57 ` Martynas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.