perf tests: Check for ARM [vectors] page
Subsystems (2)
v3 cover perf tests: Check for ARM [vectors] page
Hi all,
I just painfully learned that perf would segfault when CONFIG_KUSER_HELPERS is disabled because it unconditionally makes use of it. This patch series adds an ARM test for that by leveraging the existing find_vdso_map() function and making it more generic and capable of location any map within /proc/self/maps.
Changes in v3:
- remove find_vdso_map() call find_map() with VDSO__MAP_NAME
Changes in v2:
- use strlen() instead of sizeof() -1 since we made the page name a parameter
- use TEST_OK/TEST_FAIL in lieu of 0/-1
- added an error message indicating CONFIG_KUSER_HELPERS might be disabled
Florian Fainelli (2):
perf tools: Make find_vdso_map() more modular perf tests: Add a test for the ARM 32-bit [vectors] page
tools/perf/Makefile.perf | 4 ++--
tools/perf/arch/arm/tests/Build | 1 +
tools/perf/arch/arm/tests/arch-tests.c | 4 ++++
tools/perf/arch/arm/tests/vectors-page.c | 24 +++++++++++++++++++
tools/perf/perf-read-vdso.c | 6 ++---
tools/perf/tests/tests.h | 5 ++++
.../perf/util/{find-vdso-map.c => find-map.c} | 7 +++---
tools/perf/util/vdso.c | 6 ++---
8 files changed, 45 insertions(+), 12 deletions(-)
create mode 100644 tools/perf/arch/arm/tests/vectors-page.c
rename tools/perf/util/{find-vdso-map.c => find-map.c} (71%)Signature · 1 line
2.17.1
PATCH v3 1/2 perf tools: Make find_vdso_map() more modular
In preparation for checking that the vectors page on the ARM architecture, refactor the find_vdso_map() function to accept finding an arbitrary string and create a dedicated helper function for that under util/find-map.c and update the filename to find-map.c and all references to it: perf-read-vdso.c and util/vdso.c.
---
tools/perf/Makefile.perf | 4 ++--
tools/perf/perf-read-vdso.c | 6 +++---
tools/perf/util/{find-vdso-map.c => find-map.c} | 7 +++----
tools/perf/util/vdso.c | 6 +++---
4 files changed, 11 insertions(+), 12 deletions(-)
rename tools/perf/util/{find-vdso-map.c => find-map.c} (71%)tools/perf/Makefile.perf +2 -2
@@ -623,12 +623,12 @@ $(OUTPUT)perf-%: %.o $(PERFLIBS) $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $(filter %.o,$^) $(LIBS) ifndef NO_PERF_READ_VDSO32 -$(OUTPUT)perf-read-vdso32: perf-read-vdso.c util/find-vdso-map.c +$(OUTPUT)perf-read-vdso32: perf-read-vdso.c util/find-map.c $(QUIET_CC)$(CC) -m32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c endif ifndef NO_PERF_READ_VDSOX32 -$(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-vdso-map.c +$(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c endif
tools/perf/perf-read-vdso.c +3 -3
@@ -5,17 +5,17 @@#define VDSO__MAP_NAME "[vdso]"/* -* Include definition of find_vdso_map() also used in util/vdso.c for +* Include definition of find_map() also used in util/vdso.c for * building perf. */ -#include "util/find-vdso-map.c" +#include "util/find-map.c" int main (void ) {void *start, *end;size_t size, written; -if (find_vdso_map (&start, &end)) +if (find_map (&start, &end,VDSO__MAP_NAME ))return 1 ; size = end - start;
tools/perf/util/find-map.c renamed from tools/perf/util/find-vdso-map.c +3 -4
@@ -1,5 +1,5 @@// SPDX-License-Identifier: GPL-2.0 -static int find_vdso_map (void **start,void **end) +static int find_map (void **start,void **end,const char *name) {FILE *maps;char line[128 ];
@@ -7,7 +7,7 @@ static int find_vdso_map(void **start, void **end) maps =fopen ("/proc/self/maps" ,"r" );if (!maps) { -fprintf (stderr,"vdso: cannot open maps\n" ); +fprintf (stderr,"cannot open maps\n" );return -1 ; }
@@ -21,8 +21,7 @@ static int find_vdso_map(void **start, void **end)if (m <0 )continue ; -if (!strncmp (&line[m],VDSO__MAP_NAME , -sizeof (VDSO__MAP_NAME ) -1 )) +if (!strncmp (&line[m], name,strlen (name))) found =1 ; }
tools/perf/util/vdso.c +3 -3
@@ -18,10 +18,10 @@#include "debug.h" /* -* Include definition of find_vdso_map() also used in perf-read-vdso.c for +* Include definition of find_map() also used in perf-read-vdso.c for * building perf-read-vdso32 and perf-read-vdsox32. */ -#include "find-vdso-map.c" +#include "find-map.c" #define VDSO__TEMP_FILE_NAME "/tmp/perf-vdso.so-XXXXXX"
@@ -76,7 +76,7 @@ static char *get_file(struct vdso_file *vdso_file)if (vdso_file->found)return vdso_file->temp_file_name; -if (vdso_file->error ||find_vdso_map (&start, &end)) +if (vdso_file->error ||find_map (&start, &end,VDSO__MAP_NAME ))return NULL ; size = end - start;
Signature · 1 line
2.17.1
tip-bot for Florian Fainelli
merge bot[tip:perf/urgent] perf tools: Make find_vdso_map() more modular
perf tools: Make find_vdso_map() more modular
Commit-ID: 011532379b7c2de6757e129037bdfc8d704bce23 Gitweb: https://git.kernel.org/tip/011532379b7c2de6757e129037bdfc8d704bce23 Author: Florian Fainelli <[email protected]> AuthorDate: Thu, 20 Dec 2018 19:43:36 -0800 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 8 Jan 2019 13:28:13 -0300
perf tools: Make find_vdso_map() more modular
In preparation for checking that the vectors page on the ARM architecture, refactor the find_vdso_map() function to accept finding an arbitrary string and create a dedicated helper function for that under util/find-map.c and update the filename to find-map.c and all references to it: perf-read-vdso.c and util/vdso.c.
---
tools/perf/Makefile.perf | 4 ++--
tools/perf/perf-read-vdso.c | 6 +++---
tools/perf/util/{find-vdso-map.c => find-map.c} | 7 +++----
tools/perf/util/vdso.c | 6 +++---
4 files changed, 11 insertions(+), 12 deletions(-)tools/perf/Makefile.perf +2 -2
@@ -662,12 +662,12 @@ $(OUTPUT)perf-%: %.o $(PERFLIBS) $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $(filter %.o,$^) $(LIBS) ifndef NO_PERF_READ_VDSO32 -$(OUTPUT)perf-read-vdso32: perf-read-vdso.c util/find-vdso-map.c +$(OUTPUT)perf-read-vdso32: perf-read-vdso.c util/find-map.c $(QUIET_CC)$(CC) -m32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c endif ifndef NO_PERF_READ_VDSOX32 -$(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-vdso-map.c +$(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c endif
tools/perf/perf-read-vdso.c +3 -3
@@ -5,17 +5,17 @@#define VDSO__MAP_NAME "[vdso]"/* -* Include definition of find_vdso_map() also used in util/vdso.c for +* Include definition of find_map() also used in util/vdso.c for * building perf. */ -#include "util/find-vdso-map.c" +#include "util/find-map.c" int main (void ) {void *start, *end;size_t size, written; -if (find_vdso_map (&start, &end)) +if (find_map (&start, &end,VDSO__MAP_NAME ))return 1 ; size = end - start;
tools/perf/util/find-map.c renamed from tools/perf/util/find-vdso-map.c +3 -4
@@ -1,5 +1,5 @@// SPDX-License-Identifier: GPL-2.0 -static int find_vdso_map (void **start,void **end) +static int find_map (void **start,void **end,const char *name) {FILE *maps;char line[128 ];
@@ -7,7 +7,7 @@ static int find_vdso_map(void **start, void **end) maps =fopen ("/proc/self/maps" ,"r" );if (!maps) { -fprintf (stderr,"vdso: cannot open maps\n" ); +fprintf (stderr,"cannot open maps\n" );return -1 ; }
@@ -21,8 +21,7 @@ static int find_vdso_map(void **start, void **end)if (m <0 )continue ; -if (!strncmp (&line[m],VDSO__MAP_NAME , -sizeof (VDSO__MAP_NAME ) -1 )) +if (!strncmp (&line[m], name,strlen (name))) found =1 ; }
tools/perf/util/vdso.c +3 -3
@@ -18,10 +18,10 @@#include "debug.h" /* -* Include definition of find_vdso_map() also used in perf-read-vdso.c for +* Include definition of find_map() also used in perf-read-vdso.c for * building perf-read-vdso32 and perf-read-vdsox32. */ -#include "find-vdso-map.c" +#include "find-map.c" #define VDSO__TEMP_FILE_NAME "/tmp/perf-vdso.so-XXXXXX"
@@ -76,7 +76,7 @@ static char *get_file(struct vdso_file *vdso_file)if (vdso_file->found)return vdso_file->temp_file_name; -if (vdso_file->error ||find_vdso_map (&start, &end)) +if (vdso_file->error ||find_map (&start, &end,VDSO__MAP_NAME ))return NULL ; size = end - start;
PATCH v3 2/2 perf tests: Add a test for the ARM 32-bit [vectors] page
In reply to Florian Fainelliperf on ARM requires CONFIG_KUSER_HELPERS to be turned on to allow some independance with respect to the ARM CPU being used. Add a test which tries to locate the [vectors] page, created when CONFIG_KUSER_HELPERS is turned on to help asses the system's health.
--- tools/perf/arch/arm/tests/Build | 1 + tools/perf/arch/arm/tests/arch-tests.c | 4 ++++ tools/perf/arch/arm/tests/vectors-page.c | 24 ++++++++++++++++++++++++ tools/perf/tests/tests.h | 5 +++++ 4 files changed, 34 insertions(+) create mode 100644 tools/perf/arch/arm/tests/vectors-page.c
tools/perf/arch/arm/tests/Build +1 -0
@@ -1,4 +1,5 @@ libperf-y += regs_load.o libperf-y += dwarf-unwind.o +libperf-y += vectors-page.o libperf-y += arch-tests.o
tools/perf/arch/arm/tests/arch-tests.c +4 -0
@@ -10,6 +10,10 @@ struct test arch_tests[] = { .func = test__dwarf_unwind, },#endif + { + .desc = "Vectors page", + .func = test__vectors_page, + }, { .func = NULL, },
tools/perf/arch/arm/tests/vectors-page.c new file +24 -0
@@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <stdio.h> +#include <string.h> +#include <linux/compiler.h> + +#include "debug.h" +#include "tests/tests.h" +#include "util/find-map.c" + +#define VECTORS__MAP_NAME "[vectors]" + +int test__vectors_page (struct test *test__maybe_unused , +int subtest__maybe_unused ) +{ +void *start, *end; + +if (find_map (&start, &end,VECTORS__MAP_NAME )) { +pr_err ("%s not found, is CONFIG_KUSER_HELPERS enabled?\n" , +VECTORS__MAP_NAME ); +return TEST_FAIL ; + } + +return TEST_OK ; +}
tools/perf/tests/tests.h +5 -0
@@ -119,4 +119,9 @@ int test__arch_unwind_sample(struct perf_sample *sample, struct thread *thread);#endif #endif + +#if defined(__arm__) +int test__vectors_page (struct test *test,int subtest); +#endif +#endif /* TESTS_H */
Signature · 1 line
2.17.1
tip-bot for Florian Fainelli
merge bot[tip:perf/urgent] perf tests: Add a test for the ARM 32-bit [vectors] page
perf tests: Add a test for the ARM 32-bit [vectors] page
Commit-ID: 21327c7843e9169d5e2e527713e60e6c9842a56c Gitweb: https://git.kernel.org/tip/21327c7843e9169d5e2e527713e60e6c9842a56c Author: Florian Fainelli <[email protected]> AuthorDate: Thu, 20 Dec 2018 19:43:37 -0800 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 8 Jan 2019 13:28:13 -0300
perf tests: Add a test for the ARM 32-bit [vectors] page
perf on ARM requires CONFIG_KUSER_HELPERS to be turned on to allow some independance with respect to the ARM CPU being used. Add a test which tries to locate the [vectors] page, created when CONFIG_KUSER_HELPERS is turned on to help asses the system's health.
--- tools/perf/arch/arm/tests/Build | 1 + tools/perf/arch/arm/tests/arch-tests.c | 4 ++++ tools/perf/arch/arm/tests/vectors-page.c | 24 ++++++++++++++++++++++++ tools/perf/tests/tests.h | 5 +++++ 4 files changed, 34 insertions(+)
tools/perf/arch/arm/tests/Build +1 -0
@@ -1,4 +1,5 @@ libperf-y += regs_load.o libperf-y += dwarf-unwind.o +libperf-y += vectors-page.o libperf-y += arch-tests.o
tools/perf/arch/arm/tests/arch-tests.c +4 -0
@@ -10,6 +10,10 @@ struct test arch_tests[] = { .func = test__dwarf_unwind, },#endif + { + .desc = "Vectors page", + .func = test__vectors_page, + }, { .func = NULL, },
tools/perf/arch/arm/tests/vectors-page.c new file +24 -0
@@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <stdio.h> +#include <string.h> +#include <linux/compiler.h> + +#include "debug.h" +#include "tests/tests.h" +#include "util/find-map.c" + +#define VECTORS__MAP_NAME "[vectors]" + +int test__vectors_page (struct test *test__maybe_unused , +int subtest__maybe_unused ) +{ +void *start, *end; + +if (find_map (&start, &end,VECTORS__MAP_NAME )) { +pr_err ("%s not found, is CONFIG_KUSER_HELPERS enabled?\n" , +VECTORS__MAP_NAME ); +return TEST_FAIL ; + } + +return TEST_OK ; +}
tools/perf/tests/tests.h +5 -0
@@ -119,4 +119,9 @@ int test__arch_unwind_sample(struct perf_sample *sample, struct thread *thread);#endif #endif + +#if defined(__arm__) +int test__vectors_page (struct test *test,int subtest); +#endif +#endif /* TESTS_H */
Namhyung Kim
maintainerHello,
Quoted 4 lines · Florian Fainelli
Hi all,
I just painfully learned that perf would segfault when
CONFIG_KUSER_HELPERSis disabled because it unconditionally makes use of
Could you please elaborate?
Thanks,
Namhyung
Quoted 35 lines
it. This patch series adds an ARM test for that by leveraging the existing
find_vdso_map()function and making it more generic and capable of location any map within/proc/self/maps.Changes in v3:
- remove
find_vdso_map()callfind_map()withVDSO__MAP_NAMEChanges in v2:
- use
strlen()instead ofsizeof()-1 since we made the page name a parameter
- use TEST_OK/TEST_FAIL in lieu of 0/-1
- added an error message indicatingCONFIG_KUSER_HELPERSmight be disabledFlorian Fainelli (2):
perf tools: Makefind_vdso_map()more modular perf tests: Add a test for the ARM 32-bit [vectors] pagetools/perf/Makefile.perf | 4 ++-- tools/perf/arch/arm/tests/Build | 1 + tools/perf/arch/arm/tests/arch-tests.c | 4 ++++ tools/perf/arch/arm/tests/vectors-page.c | 24 +++++++++++++++++++ tools/perf/perf-read-vdso.c | 6 ++--- tools/perf/tests/tests.h | 5 ++++ .../perf/util/{find-vdso-map.c => find-map.c} | 7 +++--- tools/perf/util/vdso.c | 6 ++--- 8 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 tools/perf/arch/arm/tests/vectors-page.c rename tools/perf/util/{find-vdso-map.c => find-map.c} (71%)Signature · 1 line
2.17.1
Le 12/27/18 à 2:55 AM, Namhyung Kim a écrit :
Quoted 9 lines
Hello,
Quoted 4 lines · Florian Fainelli
Hi all,
I just painfully learned that perf would segfault when
CONFIG_KUSER_HELPERSis disabled because it unconditionally makes use ofCould you please elaborate?
Sure, I was debugging why perf was segfaulting on my systems and saw that the faulting address was within 0xffff_0000 (high vectors); and because CONFIG_KUSER_HELPERS was not enabled, nothing was mapped at that address so this was a legitimate crash. This was on a variety of ARMv7A systems, Cortex-A9, Cortex-A5 etc.
Later on, I found that in tools/arch/arm/include/asm/barrier.h the barriers are unconditionally defined to make use of the [vectors] page that the ARM kernel only sets up when CONFIG_KUSER_HELPERS is enabled and this is the reason for the crash.
Testing for the page itself is pretty harmless if you think we should make something more robust around checking for HAVE_AUXTRACE_SUPPORT (which appears to be the specific location making use of barriers), let me know.
Thanks!
Quoted 40 lines
Thanks,
NamhyungQuoted 35 lines
it. This patch series adds an ARM test for that by leveraging the existing
find_vdso_map()function and making it more generic and capable of location any map within/proc/self/maps.Changes in v3:
- remove
find_vdso_map()callfind_map()withVDSO__MAP_NAMEChanges in v2:
- use
strlen()instead ofsizeof()-1 since we made the page name a parameter
- use TEST_OK/TEST_FAIL in lieu of 0/-1
- added an error message indicatingCONFIG_KUSER_HELPERSmight be disabledFlorian Fainelli (2):
perf tools: Makefind_vdso_map()more modular perf tests: Add a test for the ARM 32-bit [vectors] pagetools/perf/Makefile.perf | 4 ++-- tools/perf/arch/arm/tests/Build | 1 + tools/perf/arch/arm/tests/arch-tests.c | 4 ++++ tools/perf/arch/arm/tests/vectors-page.c | 24 +++++++++++++++++++ tools/perf/perf-read-vdso.c | 6 ++--- tools/perf/tests/tests.h | 5 ++++ .../perf/util/{find-vdso-map.c => find-map.c} | 7 +++--- tools/perf/util/vdso.c | 6 ++--- 8 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 tools/perf/arch/arm/tests/vectors-page.c rename tools/perf/util/{find-vdso-map.c => find-map.c} (71%)Signature · 1 line
2.17.1
Signature · 1 line
Florian
Namhyung Kim
maintainerHello,
Sorry for being so late.
Quoted 22 lines · Florian Fainelli
Le 12/27/18 à 2:55 AM, Namhyung Kim a écrit :
Quoted 9 lines
Hello,
Quoted 4 lines · Florian Fainelli
Hi all,
I just painfully learned that perf would segfault when
CONFIG_KUSER_HELPERSis disabled because it unconditionally makes use ofCould you please elaborate?
Sure, I was debugging why perf was segfaulting on my systems and saw that the faulting address was within 0xffff_0000 (high vectors); and because
CONFIG_KUSER_HELPERSwas not enabled, nothing was mapped at that address so this was a legitimate crash. This was on a variety of ARMv7A systems, Cortex-A9, Cortex-A5 etc.Later on, I found that in
tools/arch/arm/include/asm/barrier.hthe barriers are unconditionally defined to make use of the [vectors] page that the ARM kernel only sets up whenCONFIG_KUSER_HELPERSis enabled and this is the reason for the crash.
Quoted 4 lines
Testing for the page itself is pretty harmless if you think we should make something more robust around checking for
HAVE_AUXTRACE_SUPPORT(which appears to be the specific location making use of barriers), let me know.
Thanks for the explanation.
Is there anything we can do instead if CONFIG_KUSER_HELPERS is not defined?
I think it'd be better making the barriers into functions (probably with "static inline") and configurable depending on a result of runtime checking of the availability (like you did).
The init routine of the auxtrace (or other future users of barriers) should call an arch-specific function to check the availability then.
Thanks,
Namhyung
Quoted 20 lines · Florian Fainelli
Hi all,
I just painfully learned that perf would segfault when
CONFIG_KUSER_HELPERSis disabled because it unconditionally makes use of it. This patch series adds an ARM test for that by leveraging the existingfind_vdso_map()function and making it more generic and capable of location any map within/proc/self/maps.Changes in v3:
- remove
find_vdso_map()callfind_map()withVDSO__MAP_NAMEChanges in v2:
- use
strlen()instead ofsizeof()-1 since we made the page name a parameter
- use TEST_OK/TEST_FAIL in lieu of 0/-1
- added an error message indicatingCONFIG_KUSER_HELPERSmight be disabled
Quoted 3 lines
Florian Fainelli (2):
perf tools: Makefind_vdso_map()more modular perf tests: Add a test for the ARM 32-bit [vectors] page
thanks,
jirka
Quoted 16 lines
tools/perf/Makefile.perf | 4 ++-- tools/perf/arch/arm/tests/Build | 1 + tools/perf/arch/arm/tests/arch-tests.c | 4 ++++ tools/perf/arch/arm/tests/vectors-page.c | 24 +++++++++++++++++++ tools/perf/perf-read-vdso.c | 6 ++--- tools/perf/tests/tests.h | 5 ++++ .../perf/util/{find-vdso-map.c => find-map.c} | 7 +++--- tools/perf/util/vdso.c | 6 ++--- 8 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 tools/perf/arch/arm/tests/vectors-page.c rename tools/perf/util/{find-vdso-map.c => find-map.c} (71%)Signature · 1 line
2.17.1
Em Tue, Jan 01, 2019 at 06:39:26PM +0100, Jiri Olsa escreveu:
Quoted 19 lines · Florian Fainelli
Quoted 18 lines
I just painfully learned that perf would segfault when
CONFIG_KUSER_HELPERSis disabled because it unconditionally makes use of it. This patch series adds an ARM test for that by leveraging the existingfind_vdso_map()function and making it more generic and capable of location any map within/proc/self/maps.Changes in v3:
- remove
find_vdso_map()callfind_map()withVDSO__MAP_NAMEChanges in v2:
- use
strlen()instead ofsizeof()-1 since we made the page name a parameter
- use TEST_OK/TEST_FAIL in lieu of 0/-1
- added an error message indicatingCONFIG_KUSER_HELPERSmight be disabled
Quoted 5 lines
Quoted 3 lines
Florian Fainelli (2):
perf tools: Makefind_vdso_map()more modular perf tests: Add a test for the ARM 32-bit [vectors] pageAcked-by Jiri Olsa <[email protected]>
Thanks, applied.
- Arnaldo