linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] perf tests: Check for ARM [vectors] page
@ 2018-12-21  3:43 Florian Fainelli
  2018-12-21  3:43 ` [PATCH v3 1/2] perf tools: Make find_vdso_map() more modular Florian Fainelli
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Florian Fainelli @ 2018-12-21  3:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: cphealy, Florian Fainelli, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner,
	Ravi Bangoria, Thomas Richter, rmk+kernel, l.stach

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%)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/2] perf tools: Make find_vdso_map() more modular
  2018-12-21  3:43 [PATCH v3 0/2] perf tests: Check for ARM [vectors] page Florian Fainelli
@ 2018-12-21  3:43 ` Florian Fainelli
  2019-01-09  7:11   ` [tip:perf/urgent] " tip-bot for Florian Fainelli
  2018-12-21  3:43 ` [PATCH v3 2/2] perf tests: Add a test for the ARM 32-bit [vectors] page Florian Fainelli
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2018-12-21  3:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: cphealy, Florian Fainelli, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner,
	Ravi Bangoria, Thomas Richter, rmk+kernel, l.stach

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.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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%)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d95655489f7e..04e70a664adb 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -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
 
diff --git a/tools/perf/perf-read-vdso.c b/tools/perf/perf-read-vdso.c
index 8c0ca0cc428f..aaa5210ea84a 100644
--- a/tools/perf/perf-read-vdso.c
+++ b/tools/perf/perf-read-vdso.c
@@ -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;
diff --git a/tools/perf/util/find-vdso-map.c b/tools/perf/util/find-map.c
similarity index 71%
rename from tools/perf/util/find-vdso-map.c
rename to tools/perf/util/find-map.c
index d7823e3508fc..7b2300588ece 100644
--- a/tools/perf/util/find-vdso-map.c
+++ b/tools/perf/util/find-map.c
@@ -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;
 	}
 
diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 741af209b19d..3702cba11d7d 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/2] perf tests: Add a test for the ARM 32-bit [vectors] page
  2018-12-21  3:43 [PATCH v3 0/2] perf tests: Check for ARM [vectors] page Florian Fainelli
  2018-12-21  3:43 ` [PATCH v3 1/2] perf tools: Make find_vdso_map() more modular Florian Fainelli
@ 2018-12-21  3:43 ` Florian Fainelli
  2019-01-09  7:11   ` [tip:perf/urgent] " tip-bot for Florian Fainelli
  2018-12-27 10:55 ` [PATCH v3 0/2] perf tests: Check for ARM " Namhyung Kim
  2019-01-01 17:39 ` Jiri Olsa
  3 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2018-12-21  3:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: cphealy, Florian Fainelli, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner,
	Ravi Bangoria, Thomas Richter, rmk+kernel, l.stach

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.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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

diff --git a/tools/perf/arch/arm/tests/Build b/tools/perf/arch/arm/tests/Build
index 883c57ff0c08..d9ae2733f9cc 100644
--- a/tools/perf/arch/arm/tests/Build
+++ b/tools/perf/arch/arm/tests/Build
@@ -1,4 +1,5 @@
 libperf-y += regs_load.o
 libperf-y += dwarf-unwind.o
+libperf-y += vectors-page.o
 
 libperf-y += arch-tests.o
diff --git a/tools/perf/arch/arm/tests/arch-tests.c b/tools/perf/arch/arm/tests/arch-tests.c
index 5b1543c98022..6848101a855f 100644
--- a/tools/perf/arch/arm/tests/arch-tests.c
+++ b/tools/perf/arch/arm/tests/arch-tests.c
@@ -10,6 +10,10 @@ struct test arch_tests[] = {
 		.func = test__dwarf_unwind,
 	},
 #endif
+	{
+		.desc = "Vectors page",
+		.func = test__vectors_page,
+	},
 	{
 		.func = NULL,
 	},
diff --git a/tools/perf/arch/arm/tests/vectors-page.c b/tools/perf/arch/arm/tests/vectors-page.c
new file mode 100644
index 000000000000..7ffdd79971c8
--- /dev/null
+++ b/tools/perf/arch/arm/tests/vectors-page.c
@@ -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;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index b82f55fcc294..399f18ca71a3 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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 */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/2] perf tests: Check for ARM [vectors] page
  2018-12-21  3:43 [PATCH v3 0/2] perf tests: Check for ARM [vectors] page Florian Fainelli
  2018-12-21  3:43 ` [PATCH v3 1/2] perf tools: Make find_vdso_map() more modular Florian Fainelli
  2018-12-21  3:43 ` [PATCH v3 2/2] perf tests: Add a test for the ARM 32-bit [vectors] page Florian Fainelli
@ 2018-12-27 10:55 ` Namhyung Kim
  2018-12-28  1:35   ` Florian Fainelli
  2019-01-01 17:39 ` Jiri Olsa
  3 siblings, 1 reply; 10+ messages in thread
From: Namhyung Kim @ 2018-12-27 10:55 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, cphealy, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner, Ravi Bangoria,
	Thomas Richter, rmk+kernel, l.stach, kernel-team

Hello,

On Thu, Dec 20, 2018 at 07:43:35PM -0800, Florian Fainelli wrote:
> Hi all,
> 
> I just painfully learned that perf would segfault when
> CONFIG_KUSER_HELPERS is disabled because it unconditionally makes use of

Could you please elaborate?

Thanks,
Namhyung


> 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%)
> 
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/2] perf tests: Check for ARM [vectors] page
  2018-12-27 10:55 ` [PATCH v3 0/2] perf tests: Check for ARM " Namhyung Kim
@ 2018-12-28  1:35   ` Florian Fainelli
  2019-01-11  2:11     ` Namhyung Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2018-12-28  1:35 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, cphealy, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner, Ravi Bangoria,
	Thomas Richter, rmk+kernel, l.stach, kernel-team

Le 12/27/18 à 2:55 AM, Namhyung Kim a écrit :
> Hello,
> 
> On Thu, Dec 20, 2018 at 07:43:35PM -0800, Florian Fainelli wrote:
>> Hi all,
>>
>> I just painfully learned that perf would segfault when
>> CONFIG_KUSER_HELPERS is disabled because it unconditionally makes use of
> 
> Could 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!

> 
> Thanks,
> Namhyung
> 
> 
>> 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%)
>>
>> -- 
>> 2.17.1
>>


-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/2] perf tests: Check for ARM [vectors] page
  2018-12-21  3:43 [PATCH v3 0/2] perf tests: Check for ARM [vectors] page Florian Fainelli
                   ` (2 preceding siblings ...)
  2018-12-27 10:55 ` [PATCH v3 0/2] perf tests: Check for ARM " Namhyung Kim
@ 2019-01-01 17:39 ` Jiri Olsa
  2019-01-08 13:23   ` Arnaldo Carvalho de Melo
  3 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2019-01-01 17:39 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, cphealy, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Namhyung Kim,
	Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner, Ravi Bangoria,
	Thomas Richter, rmk+kernel, l.stach

On Thu, Dec 20, 2018 at 07:43:35PM -0800, Florian Fainelli wrote:
> 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

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> 
>  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%)
> 
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/2] perf tests: Check for ARM [vectors] page
  2019-01-01 17:39 ` Jiri Olsa
@ 2019-01-08 13:23   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-01-08 13:23 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Florian Fainelli, linux-kernel, cphealy, Peter Zijlstra,
	Ingo Molnar, Alexander Shishkin, Namhyung Kim, Kim Phillips,
	Greg Kroah-Hartman, Thomas Gleixner, Ravi Bangoria,
	Thomas Richter, rmk+kernel, l.stach

Em Tue, Jan 01, 2019 at 06:39:26PM +0100, Jiri Olsa escreveu:
> On Thu, Dec 20, 2018 at 07:43:35PM -0800, Florian Fainelli wrote:
> > 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
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip:perf/urgent] perf tools: Make find_vdso_map() more modular
  2018-12-21  3:43 ` [PATCH v3 1/2] perf tools: Make find_vdso_map() more modular Florian Fainelli
@ 2019-01-09  7:11   ` tip-bot for Florian Fainelli
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Florian Fainelli @ 2019-01-09  7:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tmricht, ravi.bangoria, acme, f.fainelli, jolsa, tglx,
	kim.phillips, gregkh, l.stach, alexander.shishkin, rmk+kernel,
	cphealy, peterz, linux-kernel, hpa, mingo, namhyung

Commit-ID:  011532379b7c2de6757e129037bdfc8d704bce23
Gitweb:     https://git.kernel.org/tip/011532379b7c2de6757e129037bdfc8d704bce23
Author:     Florian Fainelli <f.fainelli@gmail.com>
AuthorDate: Thu, 20 Dec 2018 19:43:36 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kim Phillips <kim.phillips@arm.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Link: http://lkml.kernel.org/r/20181221034337.26663-2-f.fainelli@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 2921f829a0f4..0ee6795d82cc 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -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
 
diff --git a/tools/perf/perf-read-vdso.c b/tools/perf/perf-read-vdso.c
index 8c0ca0cc428f..aaa5210ea84a 100644
--- a/tools/perf/perf-read-vdso.c
+++ b/tools/perf/perf-read-vdso.c
@@ -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;
diff --git a/tools/perf/util/find-vdso-map.c b/tools/perf/util/find-map.c
similarity index 71%
rename from tools/perf/util/find-vdso-map.c
rename to tools/perf/util/find-map.c
index d7823e3508fc..7b2300588ece 100644
--- a/tools/perf/util/find-vdso-map.c
+++ b/tools/perf/util/find-map.c
@@ -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;
 	}
 
diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 741af209b19d..3702cba11d7d 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -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;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:perf/urgent] perf tests: Add a test for the ARM 32-bit [vectors] page
  2018-12-21  3:43 ` [PATCH v3 2/2] perf tests: Add a test for the ARM 32-bit [vectors] page Florian Fainelli
@ 2019-01-09  7:11   ` tip-bot for Florian Fainelli
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Florian Fainelli @ 2019-01-09  7:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, tmricht, f.fainelli, ravi.bangoria, linux-kernel,
	kim.phillips, namhyung, rmk+kernel, l.stach, jolsa,
	alexander.shishkin, mingo, hpa, tglx, gregkh, cphealy, acme

Commit-ID:  21327c7843e9169d5e2e527713e60e6c9842a56c
Gitweb:     https://git.kernel.org/tip/21327c7843e9169d5e2e527713e60e6c9842a56c
Author:     Florian Fainelli <f.fainelli@gmail.com>
AuthorDate: Thu, 20 Dec 2018 19:43:37 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kim Phillips <kim.phillips@arm.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Link: http://lkml.kernel.org/r/20181221034337.26663-3-f.fainelli@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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(+)

diff --git a/tools/perf/arch/arm/tests/Build b/tools/perf/arch/arm/tests/Build
index 883c57ff0c08..d9ae2733f9cc 100644
--- a/tools/perf/arch/arm/tests/Build
+++ b/tools/perf/arch/arm/tests/Build
@@ -1,4 +1,5 @@
 libperf-y += regs_load.o
 libperf-y += dwarf-unwind.o
+libperf-y += vectors-page.o
 
 libperf-y += arch-tests.o
diff --git a/tools/perf/arch/arm/tests/arch-tests.c b/tools/perf/arch/arm/tests/arch-tests.c
index 5b1543c98022..6848101a855f 100644
--- a/tools/perf/arch/arm/tests/arch-tests.c
+++ b/tools/perf/arch/arm/tests/arch-tests.c
@@ -10,6 +10,10 @@ struct test arch_tests[] = {
 		.func = test__dwarf_unwind,
 	},
 #endif
+	{
+		.desc = "Vectors page",
+		.func = test__vectors_page,
+	},
 	{
 		.func = NULL,
 	},
diff --git a/tools/perf/arch/arm/tests/vectors-page.c b/tools/perf/arch/arm/tests/vectors-page.c
new file mode 100644
index 000000000000..7ffdd79971c8
--- /dev/null
+++ b/tools/perf/arch/arm/tests/vectors-page.c
@@ -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;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index b82f55fcc294..399f18ca71a3 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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 */

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/2] perf tests: Check for ARM [vectors] page
  2018-12-28  1:35   ` Florian Fainelli
@ 2019-01-11  2:11     ` Namhyung Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Namhyung Kim @ 2019-01-11  2:11 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, cphealy, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Kim Phillips, Greg Kroah-Hartman, Thomas Gleixner, Ravi Bangoria,
	Thomas Richter, rmk+kernel, l.stach, kernel-team

Hello,

Sorry for being so late.

On Thu, Dec 27, 2018 at 05:35:17PM -0800, Florian Fainelli wrote:
> Le 12/27/18 à 2:55 AM, Namhyung Kim a écrit :
> > Hello,
> > 
> > On Thu, Dec 20, 2018 at 07:43:35PM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> I just painfully learned that perf would segfault when
> >> CONFIG_KUSER_HELPERS is disabled because it unconditionally makes use of
> > 
> > Could 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 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-01-11  2:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-21  3:43 [PATCH v3 0/2] perf tests: Check for ARM [vectors] page Florian Fainelli
2018-12-21  3:43 ` [PATCH v3 1/2] perf tools: Make find_vdso_map() more modular Florian Fainelli
2019-01-09  7:11   ` [tip:perf/urgent] " tip-bot for Florian Fainelli
2018-12-21  3:43 ` [PATCH v3 2/2] perf tests: Add a test for the ARM 32-bit [vectors] page Florian Fainelli
2019-01-09  7:11   ` [tip:perf/urgent] " tip-bot for Florian Fainelli
2018-12-27 10:55 ` [PATCH v3 0/2] perf tests: Check for ARM " Namhyung Kim
2018-12-28  1:35   ` Florian Fainelli
2019-01-11  2:11     ` Namhyung Kim
2019-01-01 17:39 ` Jiri Olsa
2019-01-08 13:23   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).