| Message ID | 1573045254-39833-1-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 71f699078b154fcb1c9162fd0208ada9ce532ffc |
| Headers | show |
| Series | [RFC] perf tools: Fix cross compile for ARM64 | expand |
Em Wed, Nov 06, 2019 at 09:00:54PM +0800, John Garry escreveu: > Currently when cross compiling perf tool for ARM64 on my x86 machine I get > this error: > arch/arm64/util/sym-handling.c:9:10: fatal error: gelf.h: No such file or directory > #include <gelf.h> > > For the build, libelf is reported off: > Auto-detecting system features: > ... > ... libelf: [ OFF ] Thanks, applied. - Arnaldo > Indeed, test-libelf is not built successfully: > more ./build/feature/test-libelf.make.output > test-libelf.c:2:10: fatal error: libelf.h: No such file or directory > #include <libelf.h> > ^~~~~~~~~~ > compilation terminated. > > I have no such problems natively compiling on ARM64, and I did not > previously have this issue for cross compiling. Fix by relocating > the gelf.h include. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > > I marked this as RFC as I am suspicious that I have seen no other > reports, and whether fixing up the libelf.h include issue is the proper > approach. > > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c > index 5df788985130..8dfa3e5229f1 100644 > --- a/tools/perf/arch/arm64/util/sym-handling.c > +++ b/tools/perf/arch/arm64/util/sym-handling.c > @@ -6,9 +6,10 @@ > > #include "symbol.h" // for the elf__needs_adjust_symbols() prototype > #include <stdbool.h> > -#include <gelf.h> > > #ifdef HAVE_LIBELF_SUPPORT > +#include <gelf.h> > + > bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) > { > return ehdr.e_type == ET_EXEC || > -- > 2.17.1
On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: > Hi all, > > I find to my surprise that "perf top" does not work for arm64: > > root@ubuntu:/home/john/linux# tools/perf/perf top > Couldn't read the cpuid for this machine: No such file or directory there was recent change that check on cpuid and quits: 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine Arnaldo, maybe this should be just a warning/info, because it seems to be related to annotations only..? get_cpuid is defined only for s390/x86/powerpc, so I guess it won't work on the rest as well jirka > > That's v5.5-rc1 release. > > It seems that we are just missing an arm64 version of get_cpuid() - with the > patch below, I now get as hoped: > > PerfTop: 32857 irqs/sec kernel:85.0% exact: 0.0% lost: 0/0 drop: 0/0 > [4000Hz cycles], (all, 64 CPUs) > ------------------------------------------------------------------------------- > > 8.99% [kernel] [k] arm_smmu_cmdq_issue_cmdlist > 5.80% [kernel] [k] __softirqentry_text_start > 4.49% [kernel] [k] _raw_spin_unlock_irqrestore > 3.48% [kernel] [k] el0_svc_common.constprop.2 > 3.37% [kernel] [k] _raw_write_lock_irqsave > 3.28% [kernel] [k] __local_bh_enable_ip > 3.05% [kernel] [k] __blk_complete_request > 2.07% [kernel] [k] queued_spin_lock_slowpath > 1.93% [vdso] [.] 0x0000000000000484 > > > Was this just missed? Or is there a good reason to omit? > > Thanks, > John > > --->8--- > > Subject: [PATCH] perf: Add perf top support for arm64 > > Copied from get_cpuid_str() essentially... > > Signed-off-by: John Garry <john.garry@huawei.com> > > diff --git a/tools/perf/arch/arm64/util/header.c > b/tools/perf/arch/arm64/util/header.c > index a32e4b72a98f..ecd1f86e29cc 100644 > --- a/tools/perf/arch/arm64/util/header.c > +++ b/tools/perf/arch/arm64/util/header.c > @@ -1,10 +1,12 @@ > #include <stdio.h> > #include <stdlib.h> > #include <perf/cpumap.h> > +#include <util/cpumap.h> > #include <internal/cpumap.h> > #include <api/fs/fs.h> > #include "debug.h" > #include "header.h" > +#include <errno.h> > > #define MIDR "/regs/identification/midr_el1" > #define MIDR_SIZE 19 > @@ -12,6 +14,59 @@ > #define MIDR_VARIANT_SHIFT 20 > #define MIDR_VARIANT_MASK (0xf << MIDR_VARIANT_SHIFT) > > +int > +get_cpuid(char *buffer, size_t sz) > +{ > + char *buf = NULL; > + char path[PATH_MAX]; > + const char *sysfs = sysfs__mountpoint(); > + int cpu; > + u64 midr = 0; > + FILE *file; > + > + if (!sysfs) > + return EINVAL; > + > + buf = malloc(MIDR_SIZE); > + if (!buf) > + return EINVAL; > + > + /* read midr from list of cpus mapped to this pmu */ > + for (cpu = 0; cpu < cpu__max_present_cpu(); cpu++) { > + scnprintf(path, sz, "%s/devices/system/cpu/cpu%d"MIDR, > + sysfs, cpu); > + > + file = fopen(path, "r"); > + if (!file) { > + pr_debug("fopen failed for file %s\n", path); > + continue; > + } > + > + if (!fgets(buf, MIDR_SIZE, file)) { > + fclose(file); > + continue; > + } > + fclose(file); > + > + /* Ignore/clear Variant[23:20] and > + * Revision[3:0] of MIDR > + */ > + midr = strtoul(buf, NULL, 16); > + midr &= (~(MIDR_VARIANT_MASK | MIDR_REVISION_MASK)); > + scnprintf(buffer, MIDR_SIZE, "0x%016lx", midr); > + /* got midr break loop */ > + break; > + } > + > + if (!midr) { > + pr_err("failed to get cpuid string\n"); > + free(buf); > + return EINVAL; > + } > + return 0; > +} > + >
On 10/12/2019 16:36, Jiri Olsa wrote: > On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: >> Hi all, >> >> I find to my surprise that "perf top" does not work for arm64: >> >> root@ubuntu:/home/john/linux# tools/perf/perf top >> Couldn't read the cpuid for this machine: No such file or directory > Hi Jirka, > there was recent change that check on cpuid and quits: > 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine > ok, this is new code. I obviously didn't check the git history... But, apart from this, there are many other places where get_cpuid() is called. I wonder what else we're missing out on, and whether we should still add it. Thanks, John > Arnaldo, > maybe this should be just a warning/info, because it seems to be related > to annotations only..? > > get_cpuid is defined only for s390/x86/powerpc, so I guess it won't work > on the rest as well > > jirka > >> >> That's v5.5-rc1 release. >> >> It seems that we are just missing an arm64 version of get_cpuid() - with the >> patch below, I now get as hoped: >> >> PerfTop: 32857 irqs/sec kernel:85.0% exact: 0.0% lost: 0/0 drop: 0/0 >> [4000Hz cycles], (all, 64 CPUs) >> ------------------------------------------------------------------------------- >> >> 8.99% [kernel] [k] arm_smmu_cmdq_issue_cmdlist >> 5.80% [kernel] [k] __softirqentry_text_start >> 4.49% [kernel] [k] _raw_spin_unlock_irqrestore >> 3.48% [kernel] [k] el0_svc_common.constprop.2 >> 3.37% [kernel] [k] _raw_write_lock_irqsave >> 3.28% [kernel] [k] __local_bh_enable_ip >> 3.05% [kernel] [k] __blk_complete_request >> 2.07% [kernel] [k] queued_spin_lock_slowpath >> 1.93% [vdso] [.] 0x0000000000000484 >> >> >> Was this just missed? Or is there a good reason to omit? >> >> Thanks, >> John >> >> --->8--- >> >> Subject: [PATCH] perf: Add perf top support for arm64 >> >> Copied from get_cpuid_str() essentially... >> >> Signed-off-by: John Garry <john.garry@huawei.com> >> >> diff --git a/tools/perf/arch/arm64/util/header.c >> b/tools/perf/arch/arm64/util/header.c >> index a32e4b72a98f..ecd1f86e29cc 100644 >> --- a/tools/perf/arch/arm64/util/header.c >> +++ b/tools/perf/arch/arm64/util/header.c >> @@ -1,10 +1,12 @@ >> #include <stdio.h> >> #include <stdlib.h> >> #include <perf/cpumap.h> >> +#include <util/cpumap.h> >> #include <internal/cpumap.h> >> #include <api/fs/fs.h> >> #include "debug.h" >> #include "header.h" >> +#include <errno.h> >> >> #define MIDR "/regs/identification/midr_el1" >> #define MIDR_SIZE 19 >> @@ -12,6 +14,59 @@ >> #define MIDR_VARIANT_SHIFT 20 >> #define MIDR_VARIANT_MASK (0xf << MIDR_VARIANT_SHIFT) >> >> +int >> +get_cpuid(char *buffer, size_t sz) >> +{ >> + char *buf = NULL; >> + char path[PATH_MAX]; >> + const char *sysfs = sysfs__mountpoint(); >> + int cpu; >> + u64 midr = 0; >> + FILE *file; >> + >> + if (!sysfs) >> + return EINVAL; >> + >> + buf = malloc(MIDR_SIZE); >> + if (!buf) >> + return EINVAL; >> + >> + /* read midr from list of cpus mapped to this pmu */ >> + for (cpu = 0; cpu < cpu__max_present_cpu(); cpu++) { >> + scnprintf(path, sz, "%s/devices/system/cpu/cpu%d"MIDR, >> + sysfs, cpu); >> + >> + file = fopen(path, "r"); >> + if (!file) { >> + pr_debug("fopen failed for file %s\n", path); >> + continue; >> + } >> + >> + if (!fgets(buf, MIDR_SIZE, file)) { >> + fclose(file); >> + continue; >> + } >> + fclose(file); >> + >> + /* Ignore/clear Variant[23:20] and >> + * Revision[3:0] of MIDR >> + */ >> + midr = strtoul(buf, NULL, 16); >> + midr &= (~(MIDR_VARIANT_MASK | MIDR_REVISION_MASK)); >> + scnprintf(buffer, MIDR_SIZE, "0x%016lx", midr); >> + /* got midr break loop */ >> + break; >> + } >> + >> + if (!midr) { >> + pr_err("failed to get cpuid string\n"); >> + free(buf); >> + return EINVAL; >> + } >> + return 0; >> +} >> + >> > > . >
On Tue, Dec 10, 2019 at 04:52:52PM +0000, John Garry wrote: > On 10/12/2019 16:36, Jiri Olsa wrote: > > On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: > > > Hi all, > > > > > > I find to my surprise that "perf top" does not work for arm64: > > > > > > root@ubuntu:/home/john/linux# tools/perf/perf top > > > Couldn't read the cpuid for this machine: No such file or directory > > > > Hi Jirka, > > > there was recent change that check on cpuid and quits: > > 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine > > > > ok, this is new code. I obviously didn't check the git history... > > But, apart from this, there are many other places where get_cpuid() is > called. I wonder what else we're missing out on, and whether we should still > add it. right, I was just wondering how come vendor events are working for you, but realized we have get_cpuid_str being called in there ;-) I think we should add it as you have it prepared already, could you post it with bigger changelog that would explain where it's being used for arm? jirka
On 10/12/2019 17:08, Jiri Olsa wrote: > On Tue, Dec 10, 2019 at 04:52:52PM +0000, John Garry wrote: >> On 10/12/2019 16:36, Jiri Olsa wrote: >>> On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: >>>> Hi all, >>>> >>>> I find to my surprise that "perf top" does not work for arm64: >>>> >>>> root@ubuntu:/home/john/linux# tools/perf/perf top >>>> Couldn't read the cpuid for this machine: No such file or directory >>> >> >> Hi Jirka, >> >>> there was recent change that check on cpuid and quits: >>> 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine >>> >> >> ok, this is new code. I obviously didn't check the git history... >> >> But, apart from this, there are many other places where get_cpuid() is >> called. I wonder what else we're missing out on, and whether we should still >> add it. > > right, I was just wondering how come vendor events are working for you, > but realized we have get_cpuid_str being called in there ;-) > > I think we should add it as you have it prepared already, > could you post it with bigger changelog that would explain > where it's being used for arm? ok, I can look to do that. But, as you know, we still need to fix perf top for other architectures affected. Thanks, John
Em Tue, Dec 10, 2019 at 05:36:55PM +0100, Jiri Olsa escreveu: > On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: > > Hi all, > > > > I find to my surprise that "perf top" does not work for arm64: > > > > root@ubuntu:/home/john/linux# tools/perf/perf top > > Couldn't read the cpuid for this machine: No such file or directory > > there was recent change that check on cpuid and quits: > 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine > > Arnaldo, > maybe this should be just a warning/info, because it seems to be related > to annotations only..? Right, my bad, I'll look into making this just a debug message and then check in the annotation code when this is really needed to show an error/popup window :-\ - Arnaldo > get_cpuid is defined only for s390/x86/powerpc, so I guess it won't work > on the rest as well > > jirka > > > > > That's v5.5-rc1 release. > > > > It seems that we are just missing an arm64 version of get_cpuid() - with the > > patch below, I now get as hoped: > > > > PerfTop: 32857 irqs/sec kernel:85.0% exact: 0.0% lost: 0/0 drop: 0/0 > > [4000Hz cycles], (all, 64 CPUs) > > ------------------------------------------------------------------------------- > > > > 8.99% [kernel] [k] arm_smmu_cmdq_issue_cmdlist > > 5.80% [kernel] [k] __softirqentry_text_start > > 4.49% [kernel] [k] _raw_spin_unlock_irqrestore > > 3.48% [kernel] [k] el0_svc_common.constprop.2 > > 3.37% [kernel] [k] _raw_write_lock_irqsave > > 3.28% [kernel] [k] __local_bh_enable_ip > > 3.05% [kernel] [k] __blk_complete_request > > 2.07% [kernel] [k] queued_spin_lock_slowpath > > 1.93% [vdso] [.] 0x0000000000000484 > > > > > > Was this just missed? Or is there a good reason to omit? > > > > Thanks, > > John > > > > --->8--- > > > > Subject: [PATCH] perf: Add perf top support for arm64 > > > > Copied from get_cpuid_str() essentially... > > > > Signed-off-by: John Garry <john.garry@huawei.com> > > > > diff --git a/tools/perf/arch/arm64/util/header.c > > b/tools/perf/arch/arm64/util/header.c > > index a32e4b72a98f..ecd1f86e29cc 100644 > > --- a/tools/perf/arch/arm64/util/header.c > > +++ b/tools/perf/arch/arm64/util/header.c > > @@ -1,10 +1,12 @@ > > #include <stdio.h> > > #include <stdlib.h> > > #include <perf/cpumap.h> > > +#include <util/cpumap.h> > > #include <internal/cpumap.h> > > #include <api/fs/fs.h> > > #include "debug.h" > > #include "header.h" > > +#include <errno.h> > > > > #define MIDR "/regs/identification/midr_el1" > > #define MIDR_SIZE 19 > > @@ -12,6 +14,59 @@ > > #define MIDR_VARIANT_SHIFT 20 > > #define MIDR_VARIANT_MASK (0xf << MIDR_VARIANT_SHIFT) > > > > +int > > +get_cpuid(char *buffer, size_t sz) > > +{ > > + char *buf = NULL; > > + char path[PATH_MAX]; > > + const char *sysfs = sysfs__mountpoint(); > > + int cpu; > > + u64 midr = 0; > > + FILE *file; > > + > > + if (!sysfs) > > + return EINVAL; > > + > > + buf = malloc(MIDR_SIZE); > > + if (!buf) > > + return EINVAL; > > + > > + /* read midr from list of cpus mapped to this pmu */ > > + for (cpu = 0; cpu < cpu__max_present_cpu(); cpu++) { > > + scnprintf(path, sz, "%s/devices/system/cpu/cpu%d"MIDR, > > + sysfs, cpu); > > + > > + file = fopen(path, "r"); > > + if (!file) { > > + pr_debug("fopen failed for file %s\n", path); > > + continue; > > + } > > + > > + if (!fgets(buf, MIDR_SIZE, file)) { > > + fclose(file); > > + continue; > > + } > > + fclose(file); > > + > > + /* Ignore/clear Variant[23:20] and > > + * Revision[3:0] of MIDR > > + */ > > + midr = strtoul(buf, NULL, 16); > > + midr &= (~(MIDR_VARIANT_MASK | MIDR_REVISION_MASK)); > > + scnprintf(buffer, MIDR_SIZE, "0x%016lx", midr); > > + /* got midr break loop */ > > + break; > > + } > > + > > + if (!midr) { > > + pr_err("failed to get cpuid string\n"); > > + free(buf); > > + return EINVAL; > > + } > > + return 0; > > +} > > + > >
Em Tue, Dec 10, 2019 at 05:17:56PM +0000, John Garry escreveu: > On 10/12/2019 17:08, Jiri Olsa wrote: > > On Tue, Dec 10, 2019 at 04:52:52PM +0000, John Garry wrote: > > > On 10/12/2019 16:36, Jiri Olsa wrote: > > > > On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: > > > > > Hi all, > > > > > > > > > > I find to my surprise that "perf top" does not work for arm64: > > > > > > > > > > root@ubuntu:/home/john/linux# tools/perf/perf top > > > > > Couldn't read the cpuid for this machine: No such file or directory > > > > > > > > > > Hi Jirka, > > > > > > > there was recent change that check on cpuid and quits: > > > > 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine > > > > > > > > > > ok, this is new code. I obviously didn't check the git history... > > > > > > But, apart from this, there are many other places where get_cpuid() is > > > called. I wonder what else we're missing out on, and whether we should still > > > add it. > > > > right, I was just wondering how come vendor events are working for you, > > but realized we have get_cpuid_str being called in there ;-) > > > > I think we should add it as you have it prepared already, > > could you post it with bigger changelog that would explain > > where it's being used for arm? > > ok, I can look to do that. > > But, as you know, we still need to fix perf top for other architectures > affected. Right, I need to make that just a pr_debug() message and then check in the annotation code when that is needed to see if it is set, if not, then show a popup error message and refuse to do whatever annotation feature requires that. Anyway, your patch should make sense and provide info that the ARM64 annotation may use now or in the future. - Arnaldo
> -----Original Message----- > From: linux-perf-users-owner@vger.kernel.org > <linux-perf-users-owner@vger.kernel.org> On Behalf Of Jiri Olsa > Sent: 2019年12月11日 1:09 > To: John Garry <john.garry@huawei.com> > Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>; > peterz@infradead.org; mingo@redhat.com; > alexander.shishkin@linux.intel.com; namhyung@kernel.org; > mark.rutland@arm.com; will@kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>; > linux-perf-users@vger.kernel.org > Subject: Re: perf top for arm64? > > On Tue, Dec 10, 2019 at 04:52:52PM +0000, John Garry wrote: > > On 10/12/2019 16:36, Jiri Olsa wrote: > > > On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: > > > > Hi all, > > > > > > > > I find to my surprise that "perf top" does not work for arm64: > > > > > > > > root@ubuntu:/home/john/linux# tools/perf/perf top Couldn't read > > > > the cpuid for this machine: No such file or directory > > > > > > > Hi Jirka, > > > > > there was recent change that check on cpuid and quits: > > > 608127f73779 perf top: Initialize perf_env->cpuid, needed by the > > > per arch annotation init routine > > > > > > > ok, this is new code. I obviously didn't check the git history... > > > > But, apart from this, there are many other places where get_cpuid() is > > called. I wonder what else we're missing out on, and whether we should > > still add it. > > right, I was just wondering how come vendor events are working for you, but > realized we have get_cpuid_str being called in there ;-) > > I think we should add it as you have it prepared already, could you post it with > bigger changelog that would explain where it's being used for arm? Hi Jirka, I reported metricgroup cannot work on ARM64 before, however, no one can come up with a solution, could you take a look how to fix it? Thanks a lot! You can refer to below link for more info: [1] https://www.spinics.net/lists/linux-perf-users/msg09190.html (NACK by Will Deason) [2] https://www.spinics.net/lists/linux-perf-users/msg09324.html Best Regards, Joakim Zhang > jirka
Hi John, On 2019/12/11 1:08, Jiri Olsa wrote: > On Tue, Dec 10, 2019 at 04:52:52PM +0000, John Garry wrote: >> On 10/12/2019 16:36, Jiri Olsa wrote: >>> On Tue, Dec 10, 2019 at 04:13:49PM +0000, John Garry wrote: >>>> Hi all, >>>> >>>> I find to my surprise that "perf top" does not work for arm64: >>>> >>>> root@ubuntu:/home/john/linux# tools/perf/perf top >>>> Couldn't read the cpuid for this machine: No such file or directory >>> >> >> Hi Jirka, >> >>> there was recent change that check on cpuid and quits: >>> 608127f73779 perf top: Initialize perf_env->cpuid, needed by the per arch annotation init routine >>> >> >> ok, this is new code. I obviously didn't check the git history... >> >> But, apart from this, there are many other places where get_cpuid() is >> called. I wonder what else we're missing out on, and whether we should still >> add it. > > right, I was just wondering how come vendor events are working for you, > but realized we have get_cpuid_str being called in there ;-) > > I think we should add it as you have it prepared already, > could you post it with bigger changelog that would explain > where it's being used for arm? I've also seen the similar problem when I was looking to add support for 'perf kvm stat' on arm64 [1] (which though got stuck due to some other reasons for a very long time :( It would be great if your patch can address this issue! [1] https://lore.kernel.org/patchwork/patch/1087531/ Thanks, Zenghui
diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c index 5df788985130..8dfa3e5229f1 100644 --- a/tools/perf/arch/arm64/util/sym-handling.c +++ b/tools/perf/arch/arm64/util/sym-handling.c @@ -6,9 +6,10 @@ #include "symbol.h" // for the elf__needs_adjust_symbols() prototype #include <stdbool.h> -#include <gelf.h> #ifdef HAVE_LIBELF_SUPPORT +#include <gelf.h> + bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) { return ehdr.e_type == ET_EXEC ||
Currently when cross compiling perf tool for ARM64 on my x86 machine I get this error: arch/arm64/util/sym-handling.c:9:10: fatal error: gelf.h: No such file or directory #include <gelf.h> For the build, libelf is reported off: Auto-detecting system features: ... ... libelf: [ OFF ] Indeed, test-libelf is not built successfully: more ./build/feature/test-libelf.make.output test-libelf.c:2:10: fatal error: libelf.h: No such file or directory #include <libelf.h> ^~~~~~~~~~ compilation terminated. I have no such problems natively compiling on ARM64, and I did not previously have this issue for cross compiling. Fix by relocating the gelf.h include. Signed-off-by: John Garry <john.garry@huawei.com> --- I marked this as RFC as I am suspicious that I have seen no other reports, and whether fixing up the libelf.h include issue is the proper approach.