All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arch <linux-arch@vger.kernel.org>,
	Shuah Khan <shuah@kernel.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	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>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Collingbourne <pcc@google.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v5 02/23] kernel: Define gettimeofday vdso common code
Date: Fri, 22 Feb 2019 14:49:26 +0100	[thread overview]
Message-ID: <CAK8P3a1ssUdsqk7YW-tjD-dihEAP-voKLOEGO-W=TL7Lb8E7vw@mail.gmail.com> (raw)
In-Reply-To: <20190222122430.21180-3-vincenzo.frascino@arm.com>

On Fri, Feb 22, 2019 at 1:25 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:

> +/*
> + * 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 don't think we need __vdso_timeval at all, just use
__kernel_old_timeval everywhere. There is no generic
64-bit timeval type in the kernel, and there won't be because
gettimeofday() is deprecated.

For __vdso_timespec, I see how you ended up with this
redefinition, and it makes the current version of your patches
easier, but I fear it will in turn make it harder to add the
__kernel_old_timeval based variant.

What I'd prefer to see here is to always use the fixed length
types everywhere in the code, such as

static notrace int __cvdso_clock_gettime64(clockid_t clock,
                                        struct __kernel_timespec *ts)
{
... /* normal clock_gettime using 64-bit times */
}

static notrace int __cvdso_clock_gettime32(clockid_t clock,
                                        struct old_timespec32 *ts32)
{
     struct __kernel_timespec ts;
     int ret = __cvdso_clock_gettime64(clock, &ts);

     ts32->tv_sec = ts->tv_sec;
     ts32->tv_nsec = ts->tv_nsec;

     return ret;
}

and then have two different versions for 32-bit and 64-bit
architectures, calling one or the other function of the
generic code.

        Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arch <linux-arch@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Russell King <linux@armlinux.org.uk>,
	Ralf Baechle <ralf@linux-mips.org>,
	Paul Burton <paul.burton@mips.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Mark Salyzyn <salyzyn@android.com>,
	Peter Collingbourne <pcc@google.com>,
	Shuah Khan <shuah@kernel.org>,
	Dmitry Safonov <0x7f454c46@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: Re: [PATCH v5 02/23] kernel: Define gettimeofday vdso common code
Date: Fri, 22 Feb 2019 14:49:26 +0100	[thread overview]
Message-ID: <CAK8P3a1ssUdsqk7YW-tjD-dihEAP-voKLOEGO-W=TL7Lb8E7vw@mail.gmail.com> (raw)
Message-ID: <20190222134926.z_u4ydX3MqwR_EXkWTuG7FQFgJpAehhaGPB72vfz0gM@z> (raw)
In-Reply-To: <20190222122430.21180-3-vincenzo.frascino@arm.com>

On Fri, Feb 22, 2019 at 1:25 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:

> +/*
> + * 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 don't think we need __vdso_timeval at all, just use
__kernel_old_timeval everywhere. There is no generic
64-bit timeval type in the kernel, and there won't be because
gettimeofday() is deprecated.

For __vdso_timespec, I see how you ended up with this
redefinition, and it makes the current version of your patches
easier, but I fear it will in turn make it harder to add the
__kernel_old_timeval based variant.

What I'd prefer to see here is to always use the fixed length
types everywhere in the code, such as

static notrace int __cvdso_clock_gettime64(clockid_t clock,
                                        struct __kernel_timespec *ts)
{
... /* normal clock_gettime using 64-bit times */
}

static notrace int __cvdso_clock_gettime32(clockid_t clock,
                                        struct old_timespec32 *ts32)
{
     struct __kernel_timespec ts;
     int ret = __cvdso_clock_gettime64(clock, &ts);

     ts32->tv_sec = ts->tv_sec;
     ts32->tv_nsec = ts->tv_nsec;

     return ret;
}

and then have two different versions for 32-bit and 64-bit
architectures, calling one or the other function of the
generic code.

        Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arch <linux-arch@vger.kernel.org>,
	Shuah Khan <shuah@kernel.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	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>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Collingbourne <pcc@google.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v5 02/23] kernel: Define gettimeofday vdso common code
Date: Fri, 22 Feb 2019 14:49:26 +0100	[thread overview]
Message-ID: <CAK8P3a1ssUdsqk7YW-tjD-dihEAP-voKLOEGO-W=TL7Lb8E7vw@mail.gmail.com> (raw)
In-Reply-To: <20190222122430.21180-3-vincenzo.frascino@arm.com>

On Fri, Feb 22, 2019 at 1:25 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:

> +/*
> + * 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 don't think we need __vdso_timeval at all, just use
__kernel_old_timeval everywhere. There is no generic
64-bit timeval type in the kernel, and there won't be because
gettimeofday() is deprecated.

For __vdso_timespec, I see how you ended up with this
redefinition, and it makes the current version of your patches
easier, but I fear it will in turn make it harder to add the
__kernel_old_timeval based variant.

What I'd prefer to see here is to always use the fixed length
types everywhere in the code, such as

static notrace int __cvdso_clock_gettime64(clockid_t clock,
                                        struct __kernel_timespec *ts)
{
... /* normal clock_gettime using 64-bit times */
}

static notrace int __cvdso_clock_gettime32(clockid_t clock,
                                        struct old_timespec32 *ts32)
{
     struct __kernel_timespec ts;
     int ret = __cvdso_clock_gettime64(clock, &ts);

     ts32->tv_sec = ts->tv_sec;
     ts32->tv_nsec = ts->tv_nsec;

     return ret;
}

and then have two different versions for 32-bit and 64-bit
architectures, calling one or the other function of the
generic code.

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2019-02-22 13:49 UTC|newest]

Thread overview: 178+ 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 ` Vincenzo Frascino
2019-02-22 12:24 ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 01/23] kernel: Standardize vdso_datapage Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:58   ` Mark Rutland
2019-02-22 12:58     ` Mark Rutland
2019-02-22 12:58     ` Mark Rutland
2019-02-23 16:51   ` Thomas Gleixner
2019-02-23 16:51     ` Thomas Gleixner
2019-02-23 16:51     ` Thomas Gleixner
2019-02-27 14:23     ` Vincenzo Frascino
2019-02-27 14:23       ` Vincenzo Frascino
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 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 13:34   ` Mark Rutland
2019-02-22 13:34     ` Mark Rutland
2019-02-22 13:34     ` Mark Rutland
2019-02-22 13:49   ` Arnd Bergmann [this message]
2019-02-22 13:49     ` Arnd Bergmann
2019-02-22 13:49     ` Arnd Bergmann
2019-02-22 14:36     ` Arnd Bergmann
2019-02-22 14:36       ` Arnd Bergmann
2019-02-22 14:36       ` Arnd Bergmann
2019-02-27 14:52     ` Vincenzo Frascino
2019-02-27 14:52       ` Vincenzo Frascino
2019-02-27 14:52       ` Vincenzo Frascino
2019-02-28  9:29       ` Arnd Bergmann
2019-02-28  9:29         ` Arnd Bergmann
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           ` Arnd Bergmann
2019-02-28 11:58           ` Arnd Bergmann
2019-02-28 11:58           ` [PATCH 2/2] vdso: add clock_gettime64 Arnd Bergmann
2019-02-28 11:58             ` Arnd Bergmann
2019-02-28 11:58             ` Arnd Bergmann
2019-02-28 13:42           ` [PATCH 1/2] vdso: use fixed-size time types Thomas Gleixner
2019-02-28 13:42             ` Thomas Gleixner
2019-02-28 13:42             ` Thomas Gleixner
2019-02-28 13:45           ` Vincenzo Frascino
2019-02-28 13:45             ` Vincenzo Frascino
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-23 10:34     ` Thomas Gleixner
2019-02-23 10:34     ` Thomas Gleixner
2019-02-25 14:09     ` Vincenzo Frascino
2019-02-25 14:09       ` Vincenzo Frascino
2019-02-25 14:09       ` Vincenzo Frascino
2019-02-23 17:31   ` Thomas Gleixner
2019-02-23 17:31     ` Thomas Gleixner
2019-02-23 17:31     ` Thomas Gleixner
2019-02-27 13:47     ` Vincenzo Frascino
2019-02-27 13:47       ` Vincenzo Frascino
2019-02-27 13:47       ` Vincenzo Frascino
2019-02-27 15:49       ` Thomas Gleixner
2019-02-27 15:49         ` Thomas Gleixner
2019-02-27 15:49         ` Thomas Gleixner
2019-02-27 16:06         ` Vincenzo Frascino
2019-02-27 16:06           ` Vincenzo Frascino
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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 04/23] arm64: Substitute gettimeofday with C implementation Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 13:42   ` Mark Rutland
2019-02-22 13:42     ` Mark Rutland
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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 06/23] arm64: compat: Split kuser32 Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` 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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 08/23] arm64: compat: Add KUSER_HELPERS config option Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 14:04   ` Mark Rutland
2019-02-22 14:04     ` Mark Rutland
2019-02-22 14:04     ` Mark Rutland
2019-02-22 14:09     ` Russell King - ARM Linux admin
2019-02-22 14:09       ` Russell King - ARM Linux admin
2019-02-22 14:09       ` Russell King - ARM Linux admin
2019-02-26 12:10     ` Vincenzo Frascino
2019-02-26 12:10       ` Vincenzo Frascino
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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 10/23] arm64: compat: Expose signal related structures Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 11/23] arm64: compat: Generate asm offsets for signals Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 14:22   ` Mark Rutland
2019-02-22 14:22     ` Mark Rutland
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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 13/23] arm64: compat: Add vDSO Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 14:32   ` Mark Rutland
2019-02-22 14:32     ` Mark Rutland
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 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 14:37   ` Mark Rutland
2019-02-22 14:37     ` Mark Rutland
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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 16/23] arm64: elf: vDSO code page discovery Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` 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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 18/23] arm64: Add vDSO compat support Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 19/23] arm64: Enable compat vDSO support Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 14:39   ` Mark Rutland
2019-02-22 14:39     ` Mark Rutland
2019-02-22 14:39     ` Mark Rutland
2019-02-22 14:41     ` Mark Rutland
2019-02-22 14:41       ` 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   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 21/23] mips: " Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 22/23] x86: " Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-23 10:45   ` Thomas Gleixner
2019-02-23 10:45     ` Thomas Gleixner
2019-02-23 10:45     ` Thomas Gleixner
2019-02-27 16:18     ` Vincenzo Frascino
2019-02-27 16:18       ` Vincenzo Frascino
2019-02-27 16:18       ` Vincenzo Frascino
2019-02-22 12:24 ` [PATCH v5 23/23] kselftest: Extend vDSO selftest Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-22 12:24   ` Vincenzo Frascino
2019-02-23 17:39 ` [PATCH v5 00/23] Unify vDSOs across more architectures Thomas Gleixner
2019-02-23 17:39   ` Thomas Gleixner
2019-02-23 17:39   ` Thomas Gleixner
2019-02-28 11:40 ` Arnd Bergmann
2019-02-28 11:40   ` Arnd Bergmann
2019-02-28 12:09   ` Vincenzo Frascino
2019-02-28 12:09     ` Vincenzo Frascino
2019-02-28 12:38     ` Arnd Bergmann
2019-02-28 12:38       ` Arnd Bergmann
2019-02-28 12:38       ` Arnd Bergmann
2019-02-28 12:42       ` Vincenzo Frascino
2019-02-28 12:42         ` Vincenzo Frascino
2019-02-28 12:42         ` Vincenzo Frascino
2019-02-28 13:54         ` Arnd Bergmann
2019-02-28 13:54           ` Arnd Bergmann
2019-02-28 13:54           ` Arnd Bergmann
2019-02-28 15:51           ` Vincenzo Frascino
2019-02-28 15:51             ` Vincenzo Frascino
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='CAK8P3a1ssUdsqk7YW-tjD-dihEAP-voKLOEGO-W=TL7Lb8E7vw@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=0x7f454c46@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.