[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160125232118.GD238@brightrain.aerifal.cx>
Date: Mon, 25 Jan 2016 18:21:18 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH v2] mips: add vdso support
On Tue, Jan 26, 2016 at 12:00:12AM +0100, Hauke Mehrtens wrote:
> I just saw that the mips stuff is implemented in a bad way, it returns
> -ENOSYS when it can not handle the vdso call. I already complained to
> the mips kernel developers. ;-) This code was not tested, I am still
> building.
Thanks for catching that! It would have been a bad regression.
> arch/mips/syscall_arch.h | 4 ++++
> src/time/clock_gettime.c | 12 +++++++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/syscall_arch.h b/arch/mips/syscall_arch.h
> index e74e0ad..39c0ea3 100644
> --- a/arch/mips/syscall_arch.h
> +++ b/arch/mips/syscall_arch.h
> @@ -161,3 +161,7 @@ static inline long __syscall6(long n, long a, long b, long c, long d, long e, lo
> if (n == SYS_fstatat) __stat_fix(c);
> return r2;
> }
> +
> +#define VDSO_USEFUL
> +#define VDSO_CGT_SYM "__vdso_clock_gettime"
> +#define VDSO_CGT_VER "LINUX_2.6"
> diff --git a/src/time/clock_gettime.c b/src/time/clock_gettime.c
> index 1572de0..dba99ff 100644
> --- a/src/time/clock_gettime.c
> +++ b/src/time/clock_gettime.c
> @@ -26,13 +26,23 @@ void *__vdsosym(const char *, const char *);
> int __clock_gettime(clockid_t clk, struct timespec *ts)
> {
> #ifdef VDSO_CGT_SYM
> + int ret;
> static int (*volatile cgt)(clockid_t, struct timespec *);
> if (!cgt) {
> void *f = __vdsosym(VDSO_CGT_VER, VDSO_CGT_SYM);
> if (!f) f = (void *)sc_clock_gettime;
> a_cas_p(&cgt, 0, f);
> }
> - return cgt(clk, ts);
> + ret = cgt(clk, ts);
> +
> + /*
> + * mips in linux kernel 4.4.0 returns -ENOSYS if it can not
> + * handle the syscall in vdso, the original syscall should be
> + * called by the libc in such a case.
> + */
> + if (ret == -ENOSYS)
> + return sc_clock_gettime(clk, ts);
> + return ret;
> #else
> return sc_clock_gettime(clk, ts);
> #endif
This could probably be written better as:
if (ret != -ENOSYS) return ret;
#endif
return sc_clock_gettime(clk, ts);
On the other hand if the kernel is going to fix the mips bug and use a
new symbol version for the fixed one, we could just wait and define
VDSO_CGT_VER appropriately. That's the less invasive fix (doesn't
affect other archs) but I'm not really partial.
There's some value to having the fallback anyway in case some buggy
kernel breaks the vdso function on an arch where it was previously
working. I almost wonder if we shouldn't just do:
if (ret == -EINVAL) return ret;
#endif
return sc_clock_gettime(clk, ts);
i.e. reject any error but EINVAL from the vdso and try the syscall,
since EINVAL is the only one that should be possible.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
