Patchwork gettimeofday / vdso / aarch64 question

login
register
mail settings
Submitter Steve Ellcey
Date May 9, 2018, 11:25 p.m.
Message ID <1525908319.28825.220.camel@cavium.com>
Download mbox | patch
Permalink /patch/27194/
State New
Headers show

Comments

Steve Ellcey - May 9, 2018, 11:25 p.m.
I was wondering if someone could look at my proposed getimeofday.c
implementation for aarch64 and help me figure out what I am doing
wrong.  I am trying to use the VDSO interface to speed it up and
it looks like that should be supported starting with Linux 2.6.39
(according to sysdeps/unix/sysv/linux/aarch64/init-first.c) but
when I include this gettimeofday.c in sysdeps/unix/sysv/linux/aarch64
I get a link failure with undefined references to __GI___gettimeofday.

I think something is wrong with my use of the various symbol macros,
probably with libc_ifunc_hidden or __hidden_ver1, but I can't seem to
figure out what the right incantation is.

Steve Ellcey
sellcey@cavium.com
Andreas Schwab - May 10, 2018, 7:42 a.m.
On Mai 09 2018, Steve Ellcey <sellcey@cavium.com> wrote:

> +libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday,
> +                    vdso_gettimeofday ?: (void *) __gettimeofday_syscall)
> +# undef libc_hidden_def
> +# define libc_hidden_def(name)                               \
> +  __hidden_ver1 (__gettimeofday_syscall, __GI___gettimeofday,  \
> +               __gettimeofday_syscall);
> +weak_alias (__gettimeofday, gettimeofday)
> +libc_hidden_weak (gettimeofday)

You are (re)defining libc_hidden_def, but don't use it.

Andreas.
Steve Ellcey - May 10, 2018, 4:22 p.m.
On Thu, 2018-05-10 at 09:42 +0200, Andreas Schwab wrote:
> On Mai 09 2018, Steve Ellcey <sellcey@cavium.com> wrote:
> 
> > 
> > +libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday,
> > +                    vdso_gettimeofday ?: (void *)
> > __gettimeofday_syscall)
> > +# undef libc_hidden_def
> > +# define libc_hidden_def(name)                               \
> > +  __hidden_ver1 (__gettimeofday_syscall, __GI___gettimeofday,  \
> > +               __gettimeofday_syscall);
> > +weak_alias (__gettimeofday, gettimeofday)
> > +libc_hidden_weak (gettimeofday)
> You are (re)defining libc_hidden_def, but don't use it.
> 
> Andreas.

Argh.  Staring at it too long to see it.  I have it building now,
I will submit a patch once the testsuite finishes.

Steve Ellcey
Adhemerval Zanella Netto - May 10, 2018, 6:05 p.m.
On 10/05/2018 13:22, Steve Ellcey wrote:
> On Thu, 2018-05-10 at 09:42 +0200, Andreas Schwab wrote:
>> On Mai 09 2018, Steve Ellcey <sellcey@cavium.com> wrote:
>>
>>>
>>> +libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday,
>>> +                    vdso_gettimeofday ?: (void *)
>>> __gettimeofday_syscall)
>>> +# undef libc_hidden_def
>>> +# define libc_hidden_def(name)                               \
>>> +  __hidden_ver1 (__gettimeofday_syscall, __GI___gettimeofday,  \
>>> +               __gettimeofday_syscall);
>>> +weak_alias (__gettimeofday, gettimeofday)
>>> +libc_hidden_weak (gettimeofday)
>> You are (re)defining libc_hidden_def, but don't use it.
>>
>> Andreas.
> 
> Argh.  Staring at it too long to see it.  I have it building now,
> I will submit a patch once the testsuite finishes.
> 
> Steve Ellcey
> 

Keep in mind that different that x86 and powerpc implementations, where
the vDSO symbol does not fail; the arm64 vDSO implements a syscall
fallback in case of underlying hardware requires an out-of-line counter
access (arch_timer_enable_workaround).

Using a ifunc accessors to call vDSO directly will result in a slight
different semantic since generic implementation (kernel/time/time.c)
might return EFAULT in some cases (which won't be handled by ifunc
implementation).  This should not be an issue since POSIX [1] defines
no error code should reserved for the symbol, but it might trigger
some test in LTP.

I recommend you document it on commit log with the rationale to use
ifunc instead of usual INLINE_VSYSCALL.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/

Patch

diff --git a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
index e69de29..7631322 100644
--- a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
@@ -0,0 +1,66 @@ 
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Get the current time of day and timezone information,
+   putting it into *tv and *tz.  If tz is null, *tz is not filled.
+   Returns 0 on success, -1 on errors.  */
+
+#ifdef SHARED
+
+# define __gettimeofday __redirect___gettimeofday
+# include <sys/time.h>
+# undef __gettimeofday
+# include <dl-vdso.h>
+static int
+__gettimeofday_syscall (struct timeval *tv, struct timezone *tz)
+{
+  /* Should this be INLINE_SYSCALL or INLINE_VSYSCALL? */
+  return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
+}
+/* PREPARE_VERSION will need an __LP64__ ifdef when ILP32 support
+   goes in.  See _libc_vdso_platform_setup in
+   sysdeps/unix/sysv/linux/aarch64/init-first.c.  */
+
+# undef INIT_ARCH
+# define INIT_ARCH() \
+	   PREPARE_VERSION (linux_version, "LINUX_2.6.39", 123718537); \
+	   void *vdso_gettimeofday = \
+	     _dl_vdso_vsym ("__kernel_gettimeofday", &linux_version);
+
+libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday,
+                    vdso_gettimeofday ?: (void *) __gettimeofday_syscall)
+# undef libc_hidden_def
+# define libc_hidden_def(name)                               \
+  __hidden_ver1 (__gettimeofday_syscall, __GI___gettimeofday,  \
+               __gettimeofday_syscall);
+weak_alias (__gettimeofday, gettimeofday)
+libc_hidden_weak (gettimeofday)
+
+#else
+
+# include <sys/time.h>
+# include <sysdep.h>
+int
+__gettimeofday (struct timeval *tv, struct timezone *tz)
+{
+  return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
+}
+libc_hidden_def (__gettimeofday)
+weak_alias (__gettimeofday, gettimeofday)
+libc_hidden_weak (gettimeofday)
+#endif