2015-05-19 13:20:53

by Petr Holasek

[permalink] [raw]
Subject: [PATCH] perf tools: rework of handling lack of sched_getcpu in old glibc

Commit e1e455f4f4d3 ("perf tools: Work around lack of sched_getcpu in
glibc < 2.6.") introduced weak function sched_getcpu() which should
be overridden by sched_getcpu() provided by glibc >= 2.6. Unfortunately,
the weak function was linked to sched_getcpu() used in perf bench numa
and made the benchmark heavily relying on this function unusable.

This patch alternates weak function magic for preprocessor defines.

Signed-off-by: Petr Holasek <[email protected]>
---
tools/perf/util/cloexec.c | 13 +++++++++++--
tools/perf/util/cloexec.h | 6 ------
2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/cloexec.c b/tools/perf/util/cloexec.c
index 85b5238..23b8aa9 100644
--- a/tools/perf/util/cloexec.c
+++ b/tools/perf/util/cloexec.c
@@ -7,11 +7,20 @@

static unsigned long flag = PERF_FLAG_FD_CLOEXEC;

-int __weak sched_getcpu(void)
+#ifdef __GLIBC_PREREQ
+#if !__GLIBC_PREREQ(2, 6)
+static int do_sched_getcpu(void)
{
errno = ENOSYS;
return -1;
}
+#else
+static int do_sched_getcpu(void)
+{
+ return sched_getcpu();
+}
+#endif
+#endif

static int perf_flag_probe(void)
{
@@ -27,7 +36,7 @@ static int perf_flag_probe(void)
pid_t pid = -1;
char sbuf[STRERR_BUFSIZE];

- cpu = sched_getcpu();
+ cpu = do_sched_getcpu();
if (cpu < 0)
cpu = 0;

diff --git a/tools/perf/util/cloexec.h b/tools/perf/util/cloexec.h
index 68888c2..94a5a7d 100644
--- a/tools/perf/util/cloexec.h
+++ b/tools/perf/util/cloexec.h
@@ -3,10 +3,4 @@

unsigned long perf_event_open_cloexec_flag(void);

-#ifdef __GLIBC_PREREQ
-#if !__GLIBC_PREREQ(2, 6)
-extern int sched_getcpu(void) __THROW;
-#endif
-#endif
-
#endif /* __PERF_CLOEXEC_H */
--
2.1.0


2015-05-19 13:49:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: rework of handling lack of sched_getcpu in old glibc

Em Tue, May 19, 2015 at 03:20:47PM +0200, Petr Holasek escreveu:
> Commit e1e455f4f4d3 ("perf tools: Work around lack of sched_getcpu in
> glibc < 2.6.") introduced weak function sched_getcpu() which should
> be overridden by sched_getcpu() provided by glibc >= 2.6. Unfortunately,
> the weak function was linked to sched_getcpu() used in perf bench numa
> and made the benchmark heavily relying on this function unusable.
>
> This patch alternates weak function magic for preprocessor defines.

With this you break the build again on RHEL5, right? That "fix" was a
quickie, the good thing would be to have this sched_getcpu() thing
somewhere else, far from its user, i.e. cloexec has no business doing
this if there are other places (perf bench numa) that need this, right?

- Arnaldo


> Signed-off-by: Petr Holasek <[email protected]>
> ---
> tools/perf/util/cloexec.c | 13 +++++++++++--
> tools/perf/util/cloexec.h | 6 ------
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/cloexec.c b/tools/perf/util/cloexec.c
> index 85b5238..23b8aa9 100644
> --- a/tools/perf/util/cloexec.c
> +++ b/tools/perf/util/cloexec.c
> @@ -7,11 +7,20 @@
>
> static unsigned long flag = PERF_FLAG_FD_CLOEXEC;
>
> -int __weak sched_getcpu(void)
> +#ifdef __GLIBC_PREREQ
> +#if !__GLIBC_PREREQ(2, 6)
> +static int do_sched_getcpu(void)
> {
> errno = ENOSYS;
> return -1;
> }
> +#else
> +static int do_sched_getcpu(void)
> +{
> + return sched_getcpu();
> +}
> +#endif
> +#endif
>
> static int perf_flag_probe(void)
> {
> @@ -27,7 +36,7 @@ static int perf_flag_probe(void)
> pid_t pid = -1;
> char sbuf[STRERR_BUFSIZE];
>
> - cpu = sched_getcpu();
> + cpu = do_sched_getcpu();
> if (cpu < 0)
> cpu = 0;
>
> diff --git a/tools/perf/util/cloexec.h b/tools/perf/util/cloexec.h
> index 68888c2..94a5a7d 100644
> --- a/tools/perf/util/cloexec.h
> +++ b/tools/perf/util/cloexec.h
> @@ -3,10 +3,4 @@
>
> unsigned long perf_event_open_cloexec_flag(void);
>
> -#ifdef __GLIBC_PREREQ
> -#if !__GLIBC_PREREQ(2, 6)
> -extern int sched_getcpu(void) __THROW;
> -#endif
> -#endif
> -
> #endif /* __PERF_CLOEXEC_H */
> --
> 2.1.0

2015-05-19 15:16:32

by Petr Holasek

[permalink] [raw]
Subject: Re: [PATCH] perf tools: rework of handling lack of sched_getcpu in old glibc

On Tue, 19 May 2015, Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Tue, May 19, 2015 at 03:20:47PM +0200, Petr Holasek escreveu:
> > Commit e1e455f4f4d3 ("perf tools: Work around lack of sched_getcpu in
> > glibc < 2.6.") introduced weak function sched_getcpu() which should
> > be overridden by sched_getcpu() provided by glibc >= 2.6. Unfortunately,
> > the weak function was linked to sched_getcpu() used in perf bench numa
> > and made the benchmark heavily relying on this function unusable.
> >
> > This patch alternates weak function magic for preprocessor defines.
>
> With this you break the build again on RHEL5, right? That "fix" was a
> quickie, the good thing would be to have this sched_getcpu() thing
> somewhere else, far from its user, i.e. cloexec has no business doing
> this if there are other places (perf bench numa) that need this, right?
>
> - Arnaldo
>

This fix is also quickie, since perf bench numa was unusable from 4.0-rc1
and I needed to fix it somehow. But I agree, the right place for the fix
should be *far* from its users.

>
> > Signed-off-by: Petr Holasek <[email protected]>
> > ---
> > tools/perf/util/cloexec.c | 13 +++++++++++--
> > tools/perf/util/cloexec.h | 6 ------
> > 2 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/util/cloexec.c b/tools/perf/util/cloexec.c
> > index 85b5238..23b8aa9 100644
> > --- a/tools/perf/util/cloexec.c
> > +++ b/tools/perf/util/cloexec.c
> > @@ -7,11 +7,20 @@
> >
> > static unsigned long flag = PERF_FLAG_FD_CLOEXEC;
> >
> > -int __weak sched_getcpu(void)
> > +#ifdef __GLIBC_PREREQ
> > +#if !__GLIBC_PREREQ(2, 6)
> > +static int do_sched_getcpu(void)
> > {
> > errno = ENOSYS;
> > return -1;
> > }
> > +#else
> > +static int do_sched_getcpu(void)
> > +{
> > + return sched_getcpu();
> > +}
> > +#endif
> > +#endif
> >
> > static int perf_flag_probe(void)
> > {
> > @@ -27,7 +36,7 @@ static int perf_flag_probe(void)
> > pid_t pid = -1;
> > char sbuf[STRERR_BUFSIZE];
> >
> > - cpu = sched_getcpu();
> > + cpu = do_sched_getcpu();
> > if (cpu < 0)
> > cpu = 0;
> >
> > diff --git a/tools/perf/util/cloexec.h b/tools/perf/util/cloexec.h
> > index 68888c2..94a5a7d 100644
> > --- a/tools/perf/util/cloexec.h
> > +++ b/tools/perf/util/cloexec.h
> > @@ -3,10 +3,4 @@
> >
> > unsigned long perf_event_open_cloexec_flag(void);
> >
> > -#ifdef __GLIBC_PREREQ
> > -#if !__GLIBC_PREREQ(2, 6)
> > -extern int sched_getcpu(void) __THROW;
> > -#endif
> > -#endif
> > -
> > #endif /* __PERF_CLOEXEC_H */
> > --
> > 2.1.0

--
Petr Holasek
[email protected]