Message-ID: <5969f434-4fa2-b984-6014-456f208608b1@android.com>
Date:   Tue, 2 Oct 2018 08:09:17 -0700
From:   Mark Salyzyn <salyzyn@...roid.com>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     John Stultz <john.stultz@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        Kees Cook <keescook@...omium.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Kevin Brodsky <kevin.brodsky@....com>,
        Will Deacon <will.deacon@....com>,
        lkml <linux-kernel@...r.kernel.org>,
        Jeremy Linton <Jeremy.Linton@....com>,
        Andy Lutomirski <luto@...capital.net>,
        James Morse <james.morse@....com>,
        Andrew Pinski <apinski@...ium.com>,
        Dmitry Safonov <dsafonov@...tuozzo.com>,
        Andy Gross <andy.gross@...aro.org>,
        Russell King <linux@...linux.org.uk>,
        Thomas Gleixner <tglx@...utronix.de>,
        Laura Abbott <labbott@...hat.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: RESEND and REBASE arm+arm64+aarch32 vdso rewrite

On 10/02/2018 03:00 AM, Catalin Marinas wrote:
> On Mon, Oct 01, 2018 at 01:44:52PM -0700, Mark Salyzyn wrote:
>> On 10/01/2018 11:49 AM, John Stultz wrote:
>>> On Mon, Oct 1, 2018 at 10:58 AM, Mark Salyzyn <salyzyn@...roid.com> wrote:
>>>> Last sent 23 Nov 2016.
>>>>
>>>> The following 23 patches are rebased and resent, and represent a
>>>> rewrite of the arm and arm64 vDSO into C, adding support for arch32
>>>> (32-bit user space hosted 64-bit kernels) and into a common library
>>>> that other (arm, or non-arm) architectures may utilize.
>>> So I feel like this has gone around a few times w/o much comment from
>>> the arm/arm64 maintainers. I'm not sure if there's a reason?
>> I am "forming an opinion"(tm) that ARM is not interested in any work on 32
>> bit arm architectures. They have no manpower that they are willing to devote
>> to this.
> Actually, we are interested in this work but, TBH, I find it a bit hard
> to read your series and have postponed looking into it in detail. Just
> look at the patch numbering/versioning for example:
>
>> [PATCH v5 01/12] arm: vdso: rename vdso_datapage variables
>> [PATCH v5 02/12] arm: vdso: add include file defining __get_datapage()
>> [PATCH v5 03/12] arm: vdso: inline assembler operations to compiler.h
>> [PATCH v5 04/12] arm: vdso: do calculations outside reader loops
>> [PATCH v6 05/12] arm: vdso: Add support for CLOCK_MONOTONIC_RAW
>> [PATCH v5 06/12] arm: vdso: add support for clock_getres
>> [PATCH v5 07/12] arm: vdso: disable profiling
>> [PATCH v5 08/12] arm: vdso: Add ARCH_CLOCK_FIXED_MASK
>> [PATCH v5 09/12] arm: vdso: move vgettimeofday.c to lib/vdso/
>> [PATCH v5 10/12] arm64: vdso: replace gettimeofday.S with global vgettimeofday.C
>> [PATCH v6 11/12] lib: vdso: Add support for CLOCK_BOOTTIME
>> [PATCH v5 12/12] lib: vdso: do not expose gettimeofday, if no arch supported timer
>> [PATCH] lib: vdso: add support for time
>> [PATCH v2 1/3] arm64: compat: Split the sigreturn trampolines and kuser helpers (C sources)
>> [PATCH v2 2/3] arm64: compat: Split the sigreturn trampolines and kuser helpers (assembler sources)
>> [PATCH v2 3/3] arm64: compat: Add CONFIG_KUSER_HELPERS
>> [PATCH] arm64: compat: Expose offset to registers in sigframes
>> [PATCH 1/6] arm64: compat: Use vDSO sigreturn trampolines if available
>> [PATCH 2/6] arm64: elf: Set AT_SYSINFO_EHDR in compat processes
>> [PATCH 3/6] arm64: Refactor vDSO init/setup
>> [PATCH v2 4/6] arm64: compat: Add a 32-bit vDSO
>> [PATCH 5/6] arm64: compat: 32-bit vDSO setup
>> [PATCH 6/6] arm64: Wire up and expose the new compat vDSO
> The above may look obvious to you as you've worked on it but not to
> maintainers who have to read lots of other patchsets.
Because the whole set was not taken, I split them into mostly orthogonal 
pieces for divide and conquer as requested. I feel so betrayed by the 
system ;-} :-)

There is an order, but you will find at least

[PATCH v2 1/3] arm64: compat: Split the sigreturn trampolines and kuser helpers (C sources)
[PATCH v2 2/3] arm64: compat: Split the sigreturn trampolines and kuser helpers (assembler sources)
[PATCH v2 3/3] arm64: compat: Add CONFIG_KUSER_HELPERS

can go independently at first and standalone providing a much needed rework and added security by allowing control over the troublesome kuser helpers.

>> Despite the gain of 0.4% for screen-on battery life, where Android has a mix
>> of 64 and 32 bit applications, thus still relevant _today_ on 64 bit
>> architectures (providing vDSO32 for 32-bit applications).
> As Russell said, if that's the only gain, you may need other selling
> points.
0.4% screen on means all other components on the phone including the 
backlight taking power, and _still_ had a measurable power impact adding 
arm64 vDSO32 (32 arm) applications that are a subset of the phone 
ecosystem. There are 64-bit phones that have only a 32-bit user space 
that no doubt will take plenty more from this.

Microbenchmarks for arm32 application on arm64 report ~3-10 fold 
improvement in performance (time() call being the ten fold improvement, 
a gain for both arm32 and arm64 applications)
> The main advantage I see is to avoid code duplication, hence a vdso
> library that could be shared by arm/arm64/arm64-compat _and_ future or
> existing architectures that need vdso support.

Thankfully added after being reviewed, but alas increased the complexity 
of the set to fulfill.
>> ARM has complained that they want them all at one time because individually
>> they represent more work. So the whole set is here ready to go.
> Having five separate series without a clear dependency between them was
> worse than the current numbering scheme ;).

For that I apologize, I allowed others to ask it to be split up and 
complied.
> Anyway, since I still think this series is important, some weeks ago I
> assigned Vincenzo Frascino in my team the task of de-cluttering this
> patchset and posting it to the list. So we may see a new series later
> this month (and any feedback welcome).

WooHoo (sorry for being so emotional)

-- Mark

Powered by blists - more mailing lists