[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fUpnW1DE68pMW0q3vMT+n6d5SeNkwXd45XLaf01-eP47A@mail.gmail.com>
Date: Mon, 23 Jun 2025 08:12:47 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v2 1/2] perf test workload noploop: Name the noploop process
On Fri, Jun 20, 2025 at 12:36 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Hi Ian,
>
> On Wed, Jun 18, 2025 at 05:20:33PM -0700, Ian Rogers wrote:
> > Name the noploop process "perf-noploop" so that tests can easily check
> > for its existence.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/tests/workloads/noploop.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/tests/workloads/noploop.c b/tools/perf/tests/workloads/noploop.c
> > index 940ea5910a84..8b954d466083 100644
> > --- a/tools/perf/tests/workloads/noploop.c
> > +++ b/tools/perf/tests/workloads/noploop.c
> > @@ -2,6 +2,8 @@
> > #include <stdlib.h>
> > #include <signal.h>
> > #include <unistd.h>
> > +#include <linux/prctl.h>
> > +#include <sys/prctl.h>
>
> I'm afraid it'd introduce a build failure on musl. Please see
>
> https://lore.kernel.org/linux-perf-users/20250611092542.F4ooE2FL@linutronix.de/
>
> I think <sys/prctl.h> would be enough.
Hi Namhyung,
we could do that but in the glibc man page it says:
https://man7.org/linux/man-pages/man2/prctl.2.html
```
#include <linux/prctl.h> /* Definition of PR_* constants */
#include <sys/prctl.h>
```
So wouldn't the PR_SET_NAME definition potentially be missing then? We
could fill in the missing definition with `#ifndef PR_SET_NAME` it
seems sub-optimal - I see this is what was done in the linked to
changes. Perhaps we need something like a tools/perf/util/prctl.h, and
a feature test, then in that we have something like:
```
/* A header file to work around musl libc breakages caused conflicting
definitions in linux/prctl.h . */
#ifndef HAVE_SENSIBLE_PRCTL
# include <linux/prctl.h>
# include <sys/prctl.h>
#else // !HAVE_SENSIBLE_PRCTL
# include <sys/prctl.h>
# ifndef PR_SET_NAME
# define PR_SET_NAME 15
# endif // PR_SET_NAME
# ifndef PR_FUTEX_HASH
# define PR_FUTEX_HASH 78
# define PR_FUTEX_HASH_SET_SLOTS 1
# define FH_FLAG_IMMUTABLE (1ULL << 0)
# define PR_FUTEX_HASH_GET_SLOTS 2
# define PR_FUTEX_HASH_GET_IMMUTABLE 3
# endif // PR_FUTEX_HASH
#endif // HAVE_SENSIBLE_PRCTL
```
The feature test would just be to include the two headers and see if
the build breaks.
It'd be nice to think musl was slowly getting fixed. I notice we're
carrying two comments in the code about working around musl, but I
suspect we do this kind of thing in a lot of places (just knowing the
issues that come up from yourself and Arnaldo when trying to integrate
patches). It'd be nice if perf's code could keep close to what the man
page, or whatever, is saying the standard should be, rather than doing
some kind of adhoc linux/prctl.h rewrite (or if we are doing that,
make the code more explicit that it's what we're doing).
It'd be nice to think that the perf code could be clean wrt analyses
like "include what you use" [1], but strategically removing #includes
to try to work around musl issues seems like the opposite of this.
Thanks,
Ian
[1] https://github.com/include-what-you-use/include-what-you-use
> Thanks,
> Namhyung
>
>
> > #include <linux/compiler.h>
> > #include "../tests.h"
> >
> > @@ -16,6 +18,7 @@ static int noploop(int argc, const char **argv)
> > {
> > int sec = 1;
> >
> > + prctl(PR_SET_NAME, "perf-noploop");
> > if (argc > 0)
> > sec = atoi(argv[0]);
> >
> > --
> > 2.50.0.rc2.701.gf1e915cc24-goog
> >
Powered by blists - more mailing lists