[PATCH v2 1/3] perf tool: fix reading new topology attribute "core_cpus"

[PATCH v2 1/3] perf tool: fix reading new topology attribute "core_cpus"

[Konstantin Khlebnikov]

Check access("devices/system/cpu/cpu%d/topology/core_cpus", F_OK) fails,
unless current directory is "/sys". Simply try read this file first.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Fixes: 0ccdb8407a46 ("perf tools: Apply new CPU topology sysfs attributes")
---
 tools/perf/util/smt.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
index 3b791ef2cd50..8481842e9edb 100644
--- a/tools/perf/util/smt.c
+++ b/tools/perf/util/smt.c
@@ -24,13 +24,13 @@ int smt_on(void)
 
 		snprintf(fn, sizeof fn,
 			"devices/system/cpu/cpu%d/topology/core_cpus", cpu);
-		if (access(fn, F_OK) == -1) {
+		if (sysfs__read_str(fn, &str, &strlen) < 0) {
 			snprintf(fn, sizeof fn,
 				"devices/system/cpu/cpu%d/topology/thread_siblings",
 				cpu);
+			if (sysfs__read_str(fn, &str, &strlen) < 0)
+				continue;
 		}
-		if (sysfs__read_str(fn, &str, &strlen) < 0)
-			continue;
 		/* Entry is hex, but does not have 0x, so need custom parser */
 		siblings = strtoull(str, NULL, 16);
 		free(str);

[PATCH v2 2/3] perf tool: fix detecting smt at machines with more than 32 cpus

[Konstantin Khlebnikov]

Cpu bitmap is split into 32 bit words. For system with more than 32 cores
threads are always in different words thus first word never has two bits:
cpu0: "0000,00000100,00000001", cpu 79: "8000,00000080,00000000".

Instead of parsing bitmap read "core_cpus_list" or "thread_siblings_list"
and simply check presence of ',' or '-' in it.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Fixes: de5077c4e38f ("perf tools: Add utility function to detect SMT status")
---
 tools/perf/util/smt.c |   37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
index 8481842e9edb..dc37b5abd1c3 100644
--- a/tools/perf/util/smt.c
+++ b/tools/perf/util/smt.c
@@ -1,6 +1,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <string.h>
 #include <linux/bitops.h>
 #include "api/fs/fs.h"
 #include "smt.h"
@@ -9,39 +10,35 @@ int smt_on(void)
 {
 	static bool cached;
 	static int cached_result;
+	int active;
 	int cpu;
 	int ncpu;
+	char *str = NULL;
+	size_t strlen;
 
 	if (cached)
 		return cached_result;
 
 	ncpu = sysconf(_SC_NPROCESSORS_CONF);
 	for (cpu = 0; cpu < ncpu; cpu++) {
-		unsigned long long siblings;
-		char *str;
-		size_t strlen;
 		char fn[256];
 
-		snprintf(fn, sizeof fn,
-			"devices/system/cpu/cpu%d/topology/core_cpus", cpu);
-		if (sysfs__read_str(fn, &str, &strlen) < 0) {
-			snprintf(fn, sizeof fn,
-				"devices/system/cpu/cpu%d/topology/thread_siblings",
-				cpu);
-			if (sysfs__read_str(fn, &str, &strlen) < 0)
-				continue;
-		}
-		/* Entry is hex, but does not have 0x, so need custom parser */
-		siblings = strtoull(str, NULL, 16);
-		free(str);
-		if (hweight64(siblings) > 1) {
-			cached_result = 1;
-			cached = true;
+		snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
+			 cpu, "core_cpus_list");
+		if (sysfs__read_str(fn, &str, &strlen) > 0)
+			break;
+
+		snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
+			 cpu, "thread_siblings_list");
+		if (sysfs__read_str(fn, &str, &strlen) > 0)
 			break;
-		}
 	}
+
+	active = str && (strchr(str, ',') != NULL || strchr(str, '-') != NULL);
+	free(str);
+
 	if (!cached) {
-		cached_result = 0;
+		cached_result = active;
 		cached = true;
 	}
 	return cached_result;

Re: [PATCH v2 2/3] perf tool: fix detecting smt at machines with more than 32 cpus

[Arnaldo Carvalho de Melo]

Em Wed, Apr 29, 2020 at 07:22:43PM +0300, Konstantin Khlebnikov escreveu:
> Cpu bitmap is split into 32 bit words. For system with more than 32 cores
> threads are always in different words thus first word never has two bits:
> cpu0: "0000,00000100,00000001", cpu 79: "8000,00000080,00000000".
> 
> Instead of parsing bitmap read "core_cpus_list" or "thread_siblings_list"
> and simply check presence of ',' or '-' in it.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Fixes: de5077c4e38f ("perf tools: Add utility function to detect SMT status")
> ---
>  tools/perf/util/smt.c |   37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
> index 8481842e9edb..dc37b5abd1c3 100644
> --- a/tools/perf/util/smt.c
> +++ b/tools/perf/util/smt.c
> @@ -1,6 +1,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> +#include <string.h>
>  #include <linux/bitops.h>
>  #include "api/fs/fs.h"
>  #include "smt.h"
> @@ -9,39 +10,35 @@ int smt_on(void)
>  {
>  	static bool cached;
>  	static int cached_result;
> +	int active;
>  	int cpu;
>  	int ncpu;
> +	char *str = NULL;
> +	size_t strlen;

Try not to use as the name of a variable the well known name of a
standard C library function, there are cases where some of those names
are used as #define directives and then all hell break loose...

Also doing first the change that makes the use of that new file would
allow me to have processed that patch first, which is way simpler than
this one, i.e. try to leave the more involved changes to the end of the
patchkit, that helps cherry-picking the less complex/smaller parts of
your patchkit.

I've applied the first one, thanks!

- Arnaldo
  
>  	if (cached)
>  		return cached_result;
>  
>  	ncpu = sysconf(_SC_NPROCESSORS_CONF);
>  	for (cpu = 0; cpu < ncpu; cpu++) {
> -		unsigned long long siblings;
> -		char *str;
> -		size_t strlen;
>  		char fn[256];
>  
> -		snprintf(fn, sizeof fn,
> -			"devices/system/cpu/cpu%d/topology/core_cpus", cpu);
> -		if (sysfs__read_str(fn, &str, &strlen) < 0) {
> -			snprintf(fn, sizeof fn,
> -				"devices/system/cpu/cpu%d/topology/thread_siblings",
> -				cpu);
> -			if (sysfs__read_str(fn, &str, &strlen) < 0)
> -				continue;
> -		}
> -		/* Entry is hex, but does not have 0x, so need custom parser */
> -		siblings = strtoull(str, NULL, 16);
> -		free(str);
> -		if (hweight64(siblings) > 1) {
> -			cached_result = 1;
> -			cached = true;
> +		snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
> +			 cpu, "core_cpus_list");
> +		if (sysfs__read_str(fn, &str, &strlen) > 0)
> +			break;
> +
> +		snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
> +			 cpu, "thread_siblings_list");
> +		if (sysfs__read_str(fn, &str, &strlen) > 0)
>  			break;
> -		}
>  	}
> +
> +	active = str && (strchr(str, ',') != NULL || strchr(str, '-') != NULL);
> +	free(str);
> +
>  	if (!cached) {
> -		cached_result = 0;
> +		cached_result = active;
>  		cached = true;
>  	}
>  	return cached_result;
> 

-- 

- Arnaldo

Re: [PATCH v2 2/3] perf tool: fix detecting smt at machines with more than 32 cpus

[Konstantin Khlebnikov]

On Wed, Apr 29, 2020 at 9:16 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Wed, Apr 29, 2020 at 07:22:43PM +0300, Konstantin Khlebnikov escreveu:
> > Cpu bitmap is split into 32 bit words. For system with more than 32 cores
> > threads are always in different words thus first word never has two bits:
> > cpu0: "0000,00000100,00000001", cpu 79: "8000,00000080,00000000".
> >
> > Instead of parsing bitmap read "core_cpus_list" or "thread_siblings_list"
> > and simply check presence of ',' or '-' in it.
> >
> > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> > Fixes: de5077c4e38f ("perf tools: Add utility function to detect SMT status")
> > ---
> >  tools/perf/util/smt.c |   37 +++++++++++++++++--------------------
> >  1 file changed, 17 insertions(+), 20 deletions(-)
> >
> > diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
> > index 8481842e9edb..dc37b5abd1c3 100644
> > --- a/tools/perf/util/smt.c
> > +++ b/tools/perf/util/smt.c
> > @@ -1,6 +1,7 @@
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <unistd.h>
> > +#include <string.h>
> >  #include <linux/bitops.h>
> >  #include "api/fs/fs.h"
> >  #include "smt.h"
> > @@ -9,39 +10,35 @@ int smt_on(void)
> >  {
> >       static bool cached;
> >       static int cached_result;
> > +     int active;
> >       int cpu;
> >       int ncpu;
> > +     char *str = NULL;
> > +     size_t strlen;
>
> Try not to use as the name of a variable the well known name of a
> standard C library function, there are cases where some of those names
> are used as #define directives and then all hell break loose...

You mean "strlen"? Yeah, that's weird name for variable
but it was here before me thus I haven't noticed.

>
> Also doing first the change that makes the use of that new file would
> allow me to have processed that patch first, which is way simpler than
> this one, i.e. try to leave the more involved changes to the end of the
> patchkit, that helps cherry-picking the less complex/smaller parts of
> your patchkit.

Hmm. Common sense tells to put cleanups and bugfixes before new features.

>
> I've applied the first one, thanks!
>
> - Arnaldo
>
> >       if (cached)
> >               return cached_result;
> >
> >       ncpu = sysconf(_SC_NPROCESSORS_CONF);
> >       for (cpu = 0; cpu < ncpu; cpu++) {
> > -             unsigned long long siblings;
> > -             char *str;
> > -             size_t strlen;
> >               char fn[256];
> >
> > -             snprintf(fn, sizeof fn,
> > -                     "devices/system/cpu/cpu%d/topology/core_cpus", cpu);
> > -             if (sysfs__read_str(fn, &str, &strlen) < 0) {
> > -                     snprintf(fn, sizeof fn,
> > -                             "devices/system/cpu/cpu%d/topology/thread_siblings",
> > -                             cpu);
> > -                     if (sysfs__read_str(fn, &str, &strlen) < 0)
> > -                             continue;
> > -             }
> > -             /* Entry is hex, but does not have 0x, so need custom parser */
> > -             siblings = strtoull(str, NULL, 16);
> > -             free(str);
> > -             if (hweight64(siblings) > 1) {
> > -                     cached_result = 1;
> > -                     cached = true;
> > +             snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
> > +                      cpu, "core_cpus_list");
> > +             if (sysfs__read_str(fn, &str, &strlen) > 0)
> > +                     break;
> > +
> > +             snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
> > +                      cpu, "thread_siblings_list");
> > +             if (sysfs__read_str(fn, &str, &strlen) > 0)
> >                       break;
> > -             }
> >       }
> > +
> > +     active = str && (strchr(str, ',') != NULL || strchr(str, '-') != NULL);
> > +     free(str);
> > +
> >       if (!cached) {
> > -             cached_result = 0;
> > +             cached_result = active;
> >               cached = true;
> >       }
> >       return cached_result;
> >
>
> --
>
> - Arnaldo

Re: [PATCH v2 2/3] perf tool: fix detecting smt at machines with more than 32 cpus

[Arnaldo Carvalho de Melo]

Em Wed, Apr 29, 2020 at 09:38:52PM +0300, Konstantin Khlebnikov escreveu:
> On Wed, Apr 29, 2020 at 9:16 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Wed, Apr 29, 2020 at 07:22:43PM +0300, Konstantin Khlebnikov escreveu:
> > > Cpu bitmap is split into 32 bit words. For system with more than 32 cores
> > > threads are always in different words thus first word never has two bits:
> > > cpu0: "0000,00000100,00000001", cpu 79: "8000,00000080,00000000".
> > >
> > > Instead of parsing bitmap read "core_cpus_list" or "thread_siblings_list"
> > > and simply check presence of ',' or '-' in it.
> > >
> > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> > > Fixes: de5077c4e38f ("perf tools: Add utility function to detect SMT status")
> > > ---
> > >  tools/perf/util/smt.c |   37 +++++++++++++++++--------------------
> > >  1 file changed, 17 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
> > > index 8481842e9edb..dc37b5abd1c3 100644
> > > --- a/tools/perf/util/smt.c
> > > +++ b/tools/perf/util/smt.c
> > > @@ -1,6 +1,7 @@
> > >  #include <stdio.h>
> > >  #include <stdlib.h>
> > >  #include <unistd.h>
> > > +#include <string.h>
> > >  #include <linux/bitops.h>
> > >  #include "api/fs/fs.h"
> > >  #include "smt.h"
> > > @@ -9,39 +10,35 @@ int smt_on(void)
> > >  {
> > >       static bool cached;
> > >       static int cached_result;
> > > +     int active;
> > >       int cpu;
> > >       int ncpu;
> > > +     char *str = NULL;
> > > +     size_t strlen;
> >
> > Try not to use as the name of a variable the well known name of a
> > standard C library function, there are cases where some of those names
> > are used as #define directives and then all hell break loose...
> 
> You mean "strlen"? Yeah, that's weird name for variable
> but it was here before me thus I haven't noticed.

Sorry, I saw it in a + prefixed line so from a quick look I thought you
were introducing it, my bad.
 
> >
> > Also doing first the change that makes the use of that new file would
> > allow me to have processed that patch first, which is way simpler than
> > this one, i.e. try to leave the more involved changes to the end of the
> > patchkit, that helps cherry-picking the less complex/smaller parts of
> > your patchkit.
> 
> Hmm. Common sense tells to put cleanups and bugfixes before new features.

Well, in this case on up-to-date machines that code is not even used,
its a fallback.

If you don't have the time I'll eventually adjust the patch to what I
have now in my perf/core branch, since I've reordered them,

Thanks,

- Arnaldo
 
> > I've applied the first one, thanks!
> >
> > - Arnaldo
> >
> > >       if (cached)
> > >               return cached_result;
> > >
> > >       ncpu = sysconf(_SC_NPROCESSORS_CONF);
> > >       for (cpu = 0; cpu < ncpu; cpu++) {
> > > -             unsigned long long siblings;
> > > -             char *str;
> > > -             size_t strlen;
> > >               char fn[256];
> > >
> > > -             snprintf(fn, sizeof fn,
> > > -                     "devices/system/cpu/cpu%d/topology/core_cpus", cpu);
> > > -             if (sysfs__read_str(fn, &str, &strlen) < 0) {
> > > -                     snprintf(fn, sizeof fn,
> > > -                             "devices/system/cpu/cpu%d/topology/thread_siblings",
> > > -                             cpu);
> > > -                     if (sysfs__read_str(fn, &str, &strlen) < 0)
> > > -                             continue;
> > > -             }
> > > -             /* Entry is hex, but does not have 0x, so need custom parser */
> > > -             siblings = strtoull(str, NULL, 16);
> > > -             free(str);
> > > -             if (hweight64(siblings) > 1) {
> > > -                     cached_result = 1;
> > > -                     cached = true;
> > > +             snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
> > > +                      cpu, "core_cpus_list");
> > > +             if (sysfs__read_str(fn, &str, &strlen) > 0)
> > > +                     break;
> > > +
> > > +             snprintf(fn, sizeof(fn), "devices/system/cpu/cpu%d/topology/%s",
> > > +                      cpu, "thread_siblings_list");
> > > +             if (sysfs__read_str(fn, &str, &strlen) > 0)
> > >                       break;
> > > -             }
> > >       }
> > > +
> > > +     active = str && (strchr(str, ',') != NULL || strchr(str, '-') != NULL);
> > > +     free(str);
> > > +
> > >       if (!cached) {
> > > -             cached_result = 0;
> > > +             cached_result = active;
> > >               cached = true;
> > >       }
> > >       return cached_result;
> > >
> >
> > --
> >
> > - Arnaldo

-- 

- Arnaldo

[PATCH v2 3/3] perf tool: simplify checking active smt

[Konstantin Khlebnikov]

SMT now could be disabled via "/sys/devices/system/cpu/smt/control".
Status shown in "/sys/devices/system/cpu/smt/active" simply as "0" / "1".

If this knob isn't here then fallback to checking topology as before.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 tools/perf/util/smt.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
index dc37b5abd1c3..c398528d1006 100644
--- a/tools/perf/util/smt.c
+++ b/tools/perf/util/smt.c
@@ -19,6 +19,9 @@ int smt_on(void)
 	if (cached)
 		return cached_result;
 
+	if (sysfs__read_int("devices/system/cpu/smt/active", &active) > 0)
+		goto done;
+
 	ncpu = sysconf(_SC_NPROCESSORS_CONF);
 	for (cpu = 0; cpu < ncpu; cpu++) {
 		char fn[256];
@@ -37,6 +40,7 @@ int smt_on(void)
 	active = str && (strchr(str, ',') != NULL || strchr(str, '-') != NULL);
 	free(str);
 
+done:
 	if (!cached) {
 		cached_result = active;
 		cached = true;

Re: [PATCH v2 3/3] perf tool: simplify checking active smt

[Arnaldo Carvalho de Melo]

Em Wed, Apr 29, 2020 at 07:23:41PM +0300, Konstantin Khlebnikov escreveu:
> SMT now could be disabled via "/sys/devices/system/cpu/smt/control".
> Status shown in "/sys/devices/system/cpu/smt/active" simply as "0" / "1".
> 
> If this knob isn't here then fallback to checking topology as before.

I've manually applied this one, thanks, please check my perf/core branch
later before resending 2/3, thanks.

- Arnaldo
 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
>  tools/perf/util/smt.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
> index dc37b5abd1c3..c398528d1006 100644
> --- a/tools/perf/util/smt.c
> +++ b/tools/perf/util/smt.c
> @@ -19,6 +19,9 @@ int smt_on(void)
>  	if (cached)
>  		return cached_result;
>  
> +	if (sysfs__read_int("devices/system/cpu/smt/active", &active) > 0)
> +		goto done;
> +
>  	ncpu = sysconf(_SC_NPROCESSORS_CONF);
>  	for (cpu = 0; cpu < ncpu; cpu++) {
>  		char fn[256];
> @@ -37,6 +40,7 @@ int smt_on(void)
>  	active = str && (strchr(str, ',') != NULL || strchr(str, '-') != NULL);
>  	free(str);
>  
> +done:
>  	if (!cached) {
>  		cached_result = active;
>  		cached = true;
> 

-- 

- Arnaldo

[tip: perf/core] perf tools: Simplify checking if SMT is active.

[tip-bot2 for Konstantin Khlebnikov]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     bb629484d924118e3b1d8652177040115adcba01
Gitweb:        https://git.kernel.org/tip/bb629484d924118e3b1d8652177040115adcba01
Author:        Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
AuthorDate:    Wed, 29 Apr 2020 19:23:41 +03:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 05 May 2020 16:35:29 -03:00

perf tools: Simplify checking if SMT is active.

SMT now could be disabled via "/sys/devices/system/cpu/smt/control".

Status is shown in "/sys/devices/system/cpu/smt/active" simply as "0" / "1".

If this knob isn't here then fallback to checking topology as before.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/158817741394.748034.9273604089138009552.stgit@buzz
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/smt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
index 8481842..20bacd5 100644
--- a/tools/perf/util/smt.c
+++ b/tools/perf/util/smt.c
@@ -15,6 +15,9 @@ int smt_on(void)
 	if (cached)
 		return cached_result;
 
+	if (sysfs__read_int("devices/system/cpu/smt/active", &cached_result) > 0)
+		goto done;
+
 	ncpu = sysconf(_SC_NPROCESSORS_CONF);
 	for (cpu = 0; cpu < ncpu; cpu++) {
 		unsigned long long siblings;
@@ -42,6 +45,7 @@ int smt_on(void)
 	}
 	if (!cached) {
 		cached_result = 0;
+done:
 		cached = true;
 	}
 	return cached_result;

Re: [PATCH v2 1/3] perf tool: fix reading new topology attribute "core_cpus"

[Arnaldo Carvalho de Melo]

Em Wed, Apr 29, 2020 at 07:19:47PM +0300, Konstantin Khlebnikov escreveu:
> Check access("devices/system/cpu/cpu%d/topology/core_cpus", F_OK) fails,
> unless current directory is "/sys". Simply try read this file first.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Fixes: 0ccdb8407a46 ("perf tools: Apply new CPU topology sysfs attributes")
> ---
>  tools/perf/util/smt.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, applied,

- Arnaldo
 
> diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
> index 3b791ef2cd50..8481842e9edb 100644
> --- a/tools/perf/util/smt.c
> +++ b/tools/perf/util/smt.c
> @@ -24,13 +24,13 @@ int smt_on(void)
>  
>  		snprintf(fn, sizeof fn,
>  			"devices/system/cpu/cpu%d/topology/core_cpus", cpu);
> -		if (access(fn, F_OK) == -1) {
> +		if (sysfs__read_str(fn, &str, &strlen) < 0) {
>  			snprintf(fn, sizeof fn,
>  				"devices/system/cpu/cpu%d/topology/thread_siblings",
>  				cpu);
> +			if (sysfs__read_str(fn, &str, &strlen) < 0)
> +				continue;
>  		}
> -		if (sysfs__read_str(fn, &str, &strlen) < 0)
> -			continue;
>  		/* Entry is hex, but does not have 0x, so need custom parser */
>  		siblings = strtoull(str, NULL, 16);
>  		free(str);
> 

-- 

- Arnaldo

[tip: perf/core] perf tools: Fix reading new topology attribute "core_cpus"

[tip-bot2 for Konstantin Khlebnikov]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     846de4371fdfddfa49481e3d04884539870dc127
Gitweb:        https://git.kernel.org/tip/846de4371fdfddfa49481e3d04884539870dc127
Author:        Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
AuthorDate:    Wed, 29 Apr 2020 19:19:47 +03:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 05 May 2020 16:35:29 -03:00

perf tools: Fix reading new topology attribute "core_cpus"

Check if access("devices/system/cpu/cpu%d/topology/core_cpus", F_OK)
fails, which will happen unless the current directory is "/sys".

Simply try to read this file first.

Fixes: 0ccdb8407a46 ("perf tools: Apply new CPU topology sysfs attributes")
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/158817718710.747528.11009278875028211991.stgit@buzz
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/smt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
index 3b791ef..8481842 100644
--- a/tools/perf/util/smt.c
+++ b/tools/perf/util/smt.c
@@ -24,13 +24,13 @@ int smt_on(void)
 
 		snprintf(fn, sizeof fn,
 			"devices/system/cpu/cpu%d/topology/core_cpus", cpu);
-		if (access(fn, F_OK) == -1) {
+		if (sysfs__read_str(fn, &str, &strlen) < 0) {
 			snprintf(fn, sizeof fn,
 				"devices/system/cpu/cpu%d/topology/thread_siblings",
 				cpu);
+			if (sysfs__read_str(fn, &str, &strlen) < 0)
+				continue;
 		}
-		if (sysfs__read_str(fn, &str, &strlen) < 0)
-			continue;
 		/* Entry is hex, but does not have 0x, so need custom parser */
 		siblings = strtoull(str, NULL, 16);
 		free(str);