linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] perf test: Fix dwarf unwind for optimized builds.
@ 2021-09-22 17:38 Ian Rogers
  2021-09-22 22:00 ` Jiri Olsa
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2021-09-22 17:38 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-kernel, clang-built-linux, Ard Biesheuvel, Miguel Ojeda
  Cc: Stephane Eranian, Ian Rogers

To ensure the stack frames are on the stack tail calls optimizations
need to be inhibited. If your compiler supports an attribute use it,
otherwise use an asm volatile barrier.

The barrier fix was suggested here:
https://lore.kernel.org/lkml/20201028081123.GT2628@hirez.programming.kicks-ass.net/
Tested with an optimized clang build and by forcing the asm barrier
route with an optimized clang build.

A GCC bug tracking a proper disable_tail_calls is:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97831

Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
       attribute")

v2. is a rebase. The original fix patch generated quite a lot of
    discussion over the right place for the fix:
https://lore.kernel.org/lkml/20201114000803.909530-1-irogers@google.com/
    The patch reflects my preference of it being near the use, so that
    future code cleanups don't break this somewhat special usage.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index a288035eb362..c756284b3b13 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -20,6 +20,23 @@
 /* For bsearch. We try to unwind functions in shared object. */
 #include <stdlib.h>
 
+/*
+ * The test will assert frames are on the stack but tail call optimizations lose
+ * the frame of the caller. Clang can disable this optimization on a called
+ * function but GCC currently (11/2020) lacks this attribute. The barrier is
+ * used to inhibit tail calls in these cases.
+ */
+#ifdef __has_attribute
+#if __has_attribute(disable_tail_calls)
+#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
+#define NO_TAIL_CALL_BARRIER
+#endif
+#endif
+#ifndef NO_TAIL_CALL_ATTRIBUTE
+#define NO_TAIL_CALL_ATTRIBUTE
+#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
+#endif
+
 static int mmap_handler(struct perf_tool *tool __maybe_unused,
 			union perf_event *event,
 			struct perf_sample *sample,
@@ -91,7 +108,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
 	return strcmp((const char *) symbol, funcs[idx]);
 }
 
-noinline int test_dwarf_unwind__thread(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thread)
 {
 	struct perf_sample sample;
 	unsigned long cnt = 0;
@@ -122,7 +139,7 @@ noinline int test_dwarf_unwind__thread(struct thread *thread)
 
 static int global_unwind_retval = -INT_MAX;
 
-noinline int test_dwarf_unwind__compare(void *p1, void *p2)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__compare(void *p1, void *p2)
 {
 	/* Any possible value should be 'thread' */
 	struct thread *thread = *(struct thread **)p1;
@@ -141,7 +158,7 @@ noinline int test_dwarf_unwind__compare(void *p1, void *p2)
 	return p1 - p2;
 }
 
-noinline int test_dwarf_unwind__krava_3(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_3(struct thread *thread)
 {
 	struct thread *array[2] = {thread, thread};
 	void *fp = &bsearch;
@@ -160,14 +177,22 @@ noinline int test_dwarf_unwind__krava_3(struct thread *thread)
 	return global_unwind_retval;
 }
 
-noinline int test_dwarf_unwind__krava_2(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_2(struct thread *thread)
 {
-	return test_dwarf_unwind__krava_3(thread);
+	int ret;
+
+	ret =  test_dwarf_unwind__krava_3(thread);
+	NO_TAIL_CALL_BARRIER;
+	return ret;
 }
 
-noinline int test_dwarf_unwind__krava_1(struct thread *thread)
+NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_1(struct thread *thread)
 {
-	return test_dwarf_unwind__krava_2(thread);
+	int ret;
+
+	ret =  test_dwarf_unwind__krava_2(thread);
+	NO_TAIL_CALL_BARRIER;
+	return ret;
 }
 
 int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unused)
-- 
2.33.0.464.g1972c5931b-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf test: Fix dwarf unwind for optimized builds.
  2021-09-22 17:38 [PATCH v2] perf test: Fix dwarf unwind for optimized builds Ian Rogers
@ 2021-09-22 22:00 ` Jiri Olsa
  2021-09-23 13:32   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2021-09-22 22:00 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-kernel,
	clang-built-linux, Ard Biesheuvel, Miguel Ojeda,
	Stephane Eranian

On Wed, Sep 22, 2021 at 10:38:12AM -0700, Ian Rogers wrote:
> To ensure the stack frames are on the stack tail calls optimizations
> need to be inhibited. If your compiler supports an attribute use it,
> otherwise use an asm volatile barrier.
> 
> The barrier fix was suggested here:
> https://lore.kernel.org/lkml/20201028081123.GT2628@hirez.programming.kicks-ass.net/
> Tested with an optimized clang build and by forcing the asm barrier
> route with an optimized clang build.
> 
> A GCC bug tracking a proper disable_tail_calls is:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97831
> 
> Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
>        attribute")
> 
> v2. is a rebase. The original fix patch generated quite a lot of
>     discussion over the right place for the fix:
> https://lore.kernel.org/lkml/20201114000803.909530-1-irogers@google.com/
>     The patch reflects my preference of it being near the use, so that
>     future code cleanups don't break this somewhat special usage.

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> index a288035eb362..c756284b3b13 100644
> --- a/tools/perf/tests/dwarf-unwind.c
> +++ b/tools/perf/tests/dwarf-unwind.c
> @@ -20,6 +20,23 @@
>  /* For bsearch. We try to unwind functions in shared object. */
>  #include <stdlib.h>
>  
> +/*
> + * The test will assert frames are on the stack but tail call optimizations lose
> + * the frame of the caller. Clang can disable this optimization on a called
> + * function but GCC currently (11/2020) lacks this attribute. The barrier is
> + * used to inhibit tail calls in these cases.
> + */
> +#ifdef __has_attribute
> +#if __has_attribute(disable_tail_calls)
> +#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
> +#define NO_TAIL_CALL_BARRIER
> +#endif
> +#endif
> +#ifndef NO_TAIL_CALL_ATTRIBUTE
> +#define NO_TAIL_CALL_ATTRIBUTE
> +#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
> +#endif
> +
>  static int mmap_handler(struct perf_tool *tool __maybe_unused,
>  			union perf_event *event,
>  			struct perf_sample *sample,
> @@ -91,7 +108,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
>  	return strcmp((const char *) symbol, funcs[idx]);
>  }
>  
> -noinline int test_dwarf_unwind__thread(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thread)
>  {
>  	struct perf_sample sample;
>  	unsigned long cnt = 0;
> @@ -122,7 +139,7 @@ noinline int test_dwarf_unwind__thread(struct thread *thread)
>  
>  static int global_unwind_retval = -INT_MAX;
>  
> -noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__compare(void *p1, void *p2)
>  {
>  	/* Any possible value should be 'thread' */
>  	struct thread *thread = *(struct thread **)p1;
> @@ -141,7 +158,7 @@ noinline int test_dwarf_unwind__compare(void *p1, void *p2)
>  	return p1 - p2;
>  }
>  
> -noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_3(struct thread *thread)
>  {
>  	struct thread *array[2] = {thread, thread};
>  	void *fp = &bsearch;
> @@ -160,14 +177,22 @@ noinline int test_dwarf_unwind__krava_3(struct thread *thread)
>  	return global_unwind_retval;
>  }
>  
> -noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_2(struct thread *thread)
>  {
> -	return test_dwarf_unwind__krava_3(thread);
> +	int ret;
> +
> +	ret =  test_dwarf_unwind__krava_3(thread);
> +	NO_TAIL_CALL_BARRIER;
> +	return ret;
>  }
>  
> -noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_1(struct thread *thread)
>  {
> -	return test_dwarf_unwind__krava_2(thread);
> +	int ret;
> +
> +	ret =  test_dwarf_unwind__krava_2(thread);
> +	NO_TAIL_CALL_BARRIER;
> +	return ret;
>  }
>  
>  int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unused)
> -- 
> 2.33.0.464.g1972c5931b-goog
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf test: Fix dwarf unwind for optimized builds.
  2021-09-22 22:00 ` Jiri Olsa
@ 2021-09-23 13:32   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-09-23 13:32 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Namhyung Kim, linux-kernel,
	clang-built-linux, Ard Biesheuvel, Miguel Ojeda,
	Stephane Eranian

Em Thu, Sep 23, 2021 at 12:00:11AM +0200, Jiri Olsa escreveu:
> On Wed, Sep 22, 2021 at 10:38:12AM -0700, Ian Rogers wrote:
> > To ensure the stack frames are on the stack tail calls optimizations
> > need to be inhibited. If your compiler supports an attribute use it,
> > otherwise use an asm volatile barrier.
> > 
> > The barrier fix was suggested here:
> > https://lore.kernel.org/lkml/20201028081123.GT2628@hirez.programming.kicks-ass.net/
> > Tested with an optimized clang build and by forcing the asm barrier
> > route with an optimized clang build.
> > 
> > A GCC bug tracking a proper disable_tail_calls is:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97831
> > 
> > Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
> >        attribute")
> > 
> > v2. is a rebase. The original fix patch generated quite a lot of
> >     discussion over the right place for the fix:
> > https://lore.kernel.org/lkml/20201114000803.909530-1-irogers@google.com/
> >     The patch reflects my preference of it being near the use, so that
> >     future code cleanups don't break this somewhat special usage.
> 
> Acked-by: Jiri Olsa <jolsa@redhat.com>

Thanks, applied.

- Arnaldo

 
> thanks,
> jirka
> 
> > 
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
> >  1 file changed, 32 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> > index a288035eb362..c756284b3b13 100644
> > --- a/tools/perf/tests/dwarf-unwind.c
> > +++ b/tools/perf/tests/dwarf-unwind.c
> > @@ -20,6 +20,23 @@
> >  /* For bsearch. We try to unwind functions in shared object. */
> >  #include <stdlib.h>
> >  
> > +/*
> > + * The test will assert frames are on the stack but tail call optimizations lose
> > + * the frame of the caller. Clang can disable this optimization on a called
> > + * function but GCC currently (11/2020) lacks this attribute. The barrier is
> > + * used to inhibit tail calls in these cases.
> > + */
> > +#ifdef __has_attribute
> > +#if __has_attribute(disable_tail_calls)
> > +#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
> > +#define NO_TAIL_CALL_BARRIER
> > +#endif
> > +#endif
> > +#ifndef NO_TAIL_CALL_ATTRIBUTE
> > +#define NO_TAIL_CALL_ATTRIBUTE
> > +#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
> > +#endif
> > +
> >  static int mmap_handler(struct perf_tool *tool __maybe_unused,
> >  			union perf_event *event,
> >  			struct perf_sample *sample,
> > @@ -91,7 +108,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> >  	return strcmp((const char *) symbol, funcs[idx]);
> >  }
> >  
> > -noinline int test_dwarf_unwind__thread(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thread)
> >  {
> >  	struct perf_sample sample;
> >  	unsigned long cnt = 0;
> > @@ -122,7 +139,7 @@ noinline int test_dwarf_unwind__thread(struct thread *thread)
> >  
> >  static int global_unwind_retval = -INT_MAX;
> >  
> > -noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> >  {
> >  	/* Any possible value should be 'thread' */
> >  	struct thread *thread = *(struct thread **)p1;
> > @@ -141,7 +158,7 @@ noinline int test_dwarf_unwind__compare(void *p1, void *p2)
> >  	return p1 - p2;
> >  }
> >  
> > -noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> >  {
> >  	struct thread *array[2] = {thread, thread};
> >  	void *fp = &bsearch;
> > @@ -160,14 +177,22 @@ noinline int test_dwarf_unwind__krava_3(struct thread *thread)
> >  	return global_unwind_retval;
> >  }
> >  
> > -noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_2(struct thread *thread)
> >  {
> > -	return test_dwarf_unwind__krava_3(thread);
> > +	int ret;
> > +
> > +	ret =  test_dwarf_unwind__krava_3(thread);
> > +	NO_TAIL_CALL_BARRIER;
> > +	return ret;
> >  }
> >  
> > -noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> > +NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__krava_1(struct thread *thread)
> >  {
> > -	return test_dwarf_unwind__krava_2(thread);
> > +	int ret;
> > +
> > +	ret =  test_dwarf_unwind__krava_2(thread);
> > +	NO_TAIL_CALL_BARRIER;
> > +	return ret;
> >  }
> >  
> >  int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unused)
> > -- 
> > 2.33.0.464.g1972c5931b-goog
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-23 13:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 17:38 [PATCH v2] perf test: Fix dwarf unwind for optimized builds Ian Rogers
2021-09-22 22:00 ` Jiri Olsa
2021-09-23 13:32   ` 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).