From: Mark Rutland <mark.rutland@arm.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arch@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
Catalin Marinas <catalin.marinas@arm.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Will Deacon <will.deacon@arm.com>,
Russell King <linux@armlinux.org.uk>,
Ralf Baechle <ralf@linux-mips.org>,
Mark Salyzyn <salyzyn@android.com>,
Paul Burton <paul.burton@mips.com>,
Dmitry Safonov <0x7f454c46@gmail.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Collingbourne <pcc@google.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v5 02/23] kernel: Define gettimeofday vdso common code
Date: Fri, 22 Feb 2019 13:34:23 +0000 [thread overview]
Message-ID: <20190222133423.GG42419@lakrids.cambridge.arm.com> (raw)
In-Reply-To: <20190222122430.21180-3-vincenzo.frascino@arm.com>
On Fri, Feb 22, 2019 at 12:24:09PM +0000, Vincenzo Frascino wrote:
> In the last few years we assisted to an explosion of vdso
> implementations that mostly share similar code.
>
> Try to unify the gettimeofday vdso implementation introducing
> lib/vdso. The code contained in this library can ideally be
> reused by all the architectures avoiding, where possible, code
> duplication.
>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
> include/vdso/datapage.h | 1 +
> include/vdso/helpers.h | 52 ++++++++++++
> include/vdso/types.h | 39 +++++++++
> lib/Kconfig | 5 ++
> lib/vdso/Kconfig | 37 +++++++++
> lib/vdso/Makefile | 22 +++++
> lib/vdso/gettimeofday.c | 175 ++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 331 insertions(+)
> create mode 100644 include/vdso/helpers.h
> create mode 100644 include/vdso/types.h
> create mode 100644 lib/vdso/Kconfig
> create mode 100644 lib/vdso/Makefile
> create mode 100644 lib/vdso/gettimeofday.c
>
> diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
> index da346ad02b03..ff332fcba73c 100644
> --- a/include/vdso/datapage.h
> +++ b/include/vdso/datapage.h
> @@ -9,6 +9,7 @@
> #include <linux/bits.h>
> #include <linux/types.h>
> #include <linux/time.h>
> +#include <vdso/types.h>
>
> #define VDSO_BASES (CLOCK_TAI + 1)
> #define VDSO_HRES (BIT(CLOCK_REALTIME) | \
> diff --git a/include/vdso/helpers.h b/include/vdso/helpers.h
> new file mode 100644
> index 000000000000..511dea979f6b
> --- /dev/null
> +++ b/include/vdso/helpers.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __VDSO_HELPERS_H
> +#define __VDSO_HELPERS_H
> +
> +#ifdef __KERNEL__
Nit: __KERNEL__ guards can go.
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <vdso/datapage.h>
> +
> +static __always_inline notrace u32 vdso_read_begin(const struct vdso_data *vd)
Rather than explicitly annotating all functions with notrace, can't we
disable instrumentation for all C files used for the vDSO using compiler
flags?
That would be more robust, and make the code less noisy to read.
> +{
> + u32 seq;
> +
> +repeat:
> + seq = READ_ONCE(vd->seq);
> + if (seq & 1) {
> + cpu_relax();
> + goto repeat;
> + }
> +
> + smp_rmb();
> + return seq;
> +}
You could simplify the repeat loop as:
while ((seq = READ_ONCE(vd->seq)) & 1)
cpu_relax();
> +
> +static __always_inline notrace u32 vdso_read_retry(const struct vdso_data *vd,
> + u32 start)
> +{
> + u32 seq;
> +
> + smp_rmb();
> + seq = READ_ONCE(vd->seq);
> + return seq != start;
> +}
> +
> +static __always_inline notrace void vdso_write_begin(struct vdso_data *vd)
> +{
> + ++vd->seq;
> + smp_wmb();
> +}
> +
> +static __always_inline notrace void vdso_write_end(struct vdso_data *vd)
> +{
> + smp_wmb();
> + ++vd->seq;
> +}
I realise this is what existing vDSO update code does, but I do think
that these should be using WRITE_ONCE() to perform the update, to ensure
that the write is not torn.
e.g. these should be:
static __always_inline notrace void vdso_write_begin(struct vdso_data *vd)
{
WRITE_ONCE(vd->seq, vd->seq + 1);
smp_wmb();
}
static __always_inline notrace void vdso_write_end(struct vdso_data *vd)
{
smp_wmb();
WRITE_ONCE(vd->seq, vd->seq + 1);
}
Otherwise, the compiler can validly tear updates to vd->seq, and there's
the possibility that a reader sees an inconsistent value for vd->seq,
and consequently uses inconsistent data read from the vdso data page.
[...]
> +#include <linux/types.h>
> +#include <linux/time.h>
Nit: please order headers alphabetically>
> +
> +/*
> + * The definitions below are required to overcome the limitations
> + * of time_t on 32 bit architectures, which overflows in 2038.
> + * The new code should use the replacements based on time64_t and
> + * timespec64.
> + *
> + * The abstraction below will be updated once the migration to
> + * time64_t is complete.
> + */
> +#ifdef CONFIG_GENERIC_VDSO_32
> +#define __vdso_timespec old_timespec32
> +#define __vdso_timeval old_timeval32
> +#else
> +#ifdef ENABLE_COMPAT_VDSO
> +#define __vdso_timespec old_timespec32
> +#define __vdso_timeval old_timeval32
> +#else
> +#define __vdso_timespec __kernel_timespec
> +#define __vdso_timeval __kernel_old_timeval
> +#endif /* CONFIG_COMPAT_VDSO */
> +#endif /* CONFIG_GENERIC_VDSO_32 */
I'm not sure what the comment is trying to say.
For a 64-bit kernel, does this affec the native vDSO, or just the compat
vDSO?
[...]
> +config HAVE_GENERIC_VDSO
> + bool
> + default n
IIRC, 'n' is the implicit default.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-02-22 13:34 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-22 12:24 [PATCH v5 00/23] Unify vDSOs across more architectures Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 01/23] kernel: Standardize vdso_datapage Vincenzo Frascino
2019-02-22 12:58 ` Mark Rutland
2019-02-23 16:51 ` Thomas Gleixner
2019-02-27 14:23 ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 02/23] kernel: Define gettimeofday vdso common code Vincenzo Frascino
2019-02-22 13:34 ` Mark Rutland [this message]
2019-02-22 13:49 ` Arnd Bergmann
2019-02-22 14:36 ` Arnd Bergmann
2019-02-27 14:52 ` Vincenzo Frascino
2019-02-28 9:29 ` Arnd Bergmann
2019-02-28 11:58 ` [PATCH 1/2] vdso: use fixed-size time types Arnd Bergmann
2019-02-28 11:58 ` [PATCH 2/2] vdso: add clock_gettime64 Arnd Bergmann
2019-02-28 13:42 ` [PATCH 1/2] vdso: use fixed-size time types Thomas Gleixner
2019-02-28 13:45 ` Vincenzo Frascino
2019-02-23 10:34 ` [PATCH v5 02/23] kernel: Define gettimeofday vdso common code Thomas Gleixner
2019-02-25 14:09 ` Vincenzo Frascino
2019-02-23 17:31 ` Thomas Gleixner
2019-02-27 13:47 ` Vincenzo Frascino
2019-02-27 15:49 ` Thomas Gleixner
2019-02-27 16:06 ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 03/23] arm64: Build vDSO with -ffixed-x18 Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 04/23] arm64: Substitute gettimeofday with C implementation Vincenzo Frascino
2019-02-22 13:42 ` Mark Rutland
2019-02-22 12:24 ` [PATCH v5 05/23] arm64: compat: Alloc separate pages for vectors and sigpage Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 06/23] arm64: compat: Split kuser32 Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 07/23] arm64: compat: Refactor aarch32_alloc_vdso_pages() Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 08/23] arm64: compat: Add KUSER_HELPERS config option Vincenzo Frascino
2019-02-22 14:04 ` Mark Rutland
2019-02-22 14:09 ` Russell King - ARM Linux admin
2019-02-26 12:10 ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 09/23] arm64: compat: Add missing syscall numbers Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 10/23] arm64: compat: Expose signal related structures Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 11/23] arm64: compat: Generate asm offsets for signals Vincenzo Frascino
2019-02-22 14:22 ` Mark Rutland
2019-02-22 12:24 ` [PATCH v5 12/23] lib: vdso: Add compat support Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 13/23] arm64: compat: Add vDSO Vincenzo Frascino
2019-02-22 14:32 ` Mark Rutland
2019-02-22 12:24 ` [PATCH v5 14/23] arm64: Refactor vDSO code Vincenzo Frascino
2019-02-22 14:37 ` Mark Rutland
2019-02-22 12:24 ` [PATCH v5 15/23] arm64: compat: vDSO setup for compat layer Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 16/23] arm64: elf: vDSO code page discovery Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 17/23] arm64: compat: Get sigreturn trampolines from vDSO Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 18/23] arm64: Add vDSO compat support Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 19/23] arm64: Enable compat vDSO support Vincenzo Frascino
2019-02-22 14:39 ` Mark Rutland
2019-02-22 14:41 ` Mark Rutland
2019-02-22 12:24 ` [PATCH v5 20/23] arm: Add support for generic vDSO Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 21/23] mips: " Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 22/23] x86: " Vincenzo Frascino
2019-02-23 10:45 ` Thomas Gleixner
2019-02-27 16:18 ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 23/23] kselftest: Extend vDSO selftest Vincenzo Frascino
2019-02-23 17:39 ` [PATCH v5 00/23] Unify vDSOs across more architectures Thomas Gleixner
2019-02-28 11:40 ` Arnd Bergmann
2019-02-28 12:09 ` Vincenzo Frascino
2019-02-28 12:38 ` Arnd Bergmann
2019-02-28 12:42 ` Vincenzo Frascino
2019-02-28 13:54 ` Arnd Bergmann
2019-02-28 15:51 ` Vincenzo Frascino
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=20190222133423.GG42419@lakrids.cambridge.arm.com \
--to=mark.rutland@arm.com \
--cc=0x7f454c46@gmail.com \
--cc=arnd@arndb.de \
--cc=catalin.marinas@arm.com \
--cc=daniel.lezcano@linaro.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux@armlinux.org.uk \
--cc=linux@rasmusvillemoes.dk \
--cc=paul.burton@mips.com \
--cc=pcc@google.com \
--cc=ralf@linux-mips.org \
--cc=salyzyn@android.com \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@arm.com \
--cc=will.deacon@arm.com \
/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;
as well as URLs for NNTP newsgroup(s).