Linux Perf Users
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] perf test: Make leafloop workload immune to compiler options
Date: Fri, 29 May 2026 09:25:01 -0300	[thread overview]
Message-ID: <ahmFnY9zT5hRGbAs@x1> (raw)
In-Reply-To: <CAP-5=fXtzHw2c_Er0WPpfFo1cfng46VXKvShb+kbdnSoxL1rmw@mail.gmail.com>

On Mon, May 11, 2026 at 08:38:48AM -0700, Ian Rogers wrote:
> On Mon, May 11, 2026 at 2:19 AM James Clark <james.clark@linaro.org> wrote:
> >
> > Since the leafloop test program was moved into the main Perf binary as a
> > workload, it inherited the same compiler options as Perf. In this case
> > the -fstack-protector option broke the assumption that simple leaf
> > frames don't have a stack frame on Arm. This causes
> > test_arm_callgraph_fp.sh to pass even if the stack isn't augmented with
> > the link register, making the test useless.
> >
> > Fix it by rewriting the leaf function in assembly seeing as it's so
> > simple. Adding -fno-stack-protector would also work, but wouldn't be
> > robust against other future compiler option additions.
> >
> > The local variables and 'a' variable were never needed so remove them to
> > simplify.
> >
> > Assisted-by: GitHub-Copilot:GPT-5.5
> > Signed-off-by: James Clark <james.clark@linaro.org>
> > ---
> > Changes in v2:
> > - Push and pop asm sections - (Sashiko)
> > - Add .size directive - (Sashiko)
> > - Add asm label for done and test with LTO enabled - (Sashiko)
> > - Link to v1: https://lore.kernel.org/r/20260508-james-perf-leafloop-stack-v1-1-637c260b2da8@linaro.org
> > ---
> >  tools/perf/tests/workloads/leafloop.c | 40 +++++++++++++++++++++++++++--------
> >  1 file changed, 31 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> > index f7561767e32c..c20c75f7ba49 100644
> > --- a/tools/perf/tests/workloads/leafloop.c
> > +++ b/tools/perf/tests/workloads/leafloop.c
> > @@ -6,26 +6,48 @@
> >  #include "../tests.h"
> >
> >  /* We want to check these symbols in perf script */
> > -noinline void leaf(volatile int b);
> > -noinline void parent(volatile int b);
> > +noinline void leaf(void);
> > +noinline void parent(void);
> >
> > -static volatile int a;
> > -static volatile sig_atomic_t done;
> > +static volatile sig_atomic_t done asm("leafloop_done");
> >
> >  static void sighandler(int sig __maybe_unused)
> >  {
> >         done = 1;
> >  }
> >
> > -noinline void leaf(volatile int b)
> > +#if defined(__aarch64__)
> > +/*
> > + * Write leaf() in assembly so it stays as a minimal leaf function with no
> > + * stack frame and won't get silently broken in the future by any Perf wide
> > + * compilation options like -fstack-protector-all.
> > + */
> > +asm(
> > +       ".pushsection .text,\"ax\",%progbits\n"
> > +       ".global leaf\n"
> > +       ".type leaf, %function\n"
> > +       "leaf:\n"
> > +       "       adrp    x1, leafloop_done\n"
> > +       "       ldr     w2, [x1, #:lo12:leafloop_done]\n"
> > +       "       cbz     w2, leaf\n"
> > +       "       ret\n"
> > +       ".size leaf, .-leaf\n"
> > +       ".popsection\n"
> > +);
> 
> On reading this I thought, why can't we just use an asm block in a
> function, but I get it, you want specific function entry/exit to test
> the leaf unwinding feature.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo

      parent reply	other threads:[~2026-05-29 12:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-11  9:19 [PATCH v2] perf test: Make leafloop workload immune to compiler options James Clark
2026-05-11 15:38 ` Ian Rogers
2026-05-11 16:17   ` James Clark
2026-05-29 12:25   ` Arnaldo Carvalho de Melo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ahmFnY9zT5hRGbAs@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox