Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC PATCH v2 04/10] lib: vdso: get pointer to vdso data from the arch
Date:   Tue, 24 Dec 2019 20:15:11 +0800
Message-Id: <D2614EC4-5B80-4846-994D-22730ACD44A1@amacapital.net>
References: <de5273aa-69dc-8e37-c917-44708257d2ba@c-s.fr>
Cc:     Andy Lutomirski <luto@kernel.org>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Arnd Bergmann <arnd@arndb.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        LKML <linux-kernel@vger.kernel.org>,
        linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        "open list:MIPS" <linux-mips@vger.kernel.org>,
        X86 ML <x86@kernel.org>
In-Reply-To: <de5273aa-69dc-8e37-c917-44708257d2ba@c-s.fr>
To:     christophe leroy <christophe.leroy@c-s.fr>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Dec 24, 2019, at 7:53 PM, christophe leroy <christophe.leroy@c-s.fr> wr=
ote:
>=20
> =EF=BB=BF
>=20
>> Le 24/12/2019 =C3=A0 03:27, Andy Lutomirski a =C3=A9crit :
>>> On Mon, Dec 23, 2019 at 6:31 AM Christophe Leroy
>>> <christophe.leroy@c-s.fr> wrote:
>>>=20
>>> On powerpc, __arch_get_vdso_data() clobbers the link register,
>>> requiring the caller to set a stack frame in order to save it.
>>>=20
>>> As the parent function already has to set a stack frame and save
>>> the link register to call the C vdso function, retriving the
>>> vdso data pointer there is lighter.
>> I'm confused.  Can't you inline __arch_get_vdso_data()?  Or is the
>> issue that you can't retrieve the program counter on power without
>> clobbering the link register?
>=20
> Yes it can be inlined (I did it in V1 https://patchwork.ozlabs.org/patch/1=
180571/), but you can't do it without clobbering the link register, because t=
he only way to get the program counter is to do to as if you were calling an=
other function but you call to the address which just follows where you are,=
 so that it sets LR which the simulated return address which corresponds to t=
he address following the branch.
>=20
> static __always_inline
> const struct vdso_data *__arch_get_vdso_data(void)
> {
>    void *ptr;
>=20
>    asm volatile(
>        "    bcl    20, 31, .+4;\n"
>        "    mflr    %0;\n"
>        "    addi    %0, %0, __kernel_datapage_offset - (.-4);\n"
>        : "=3Db"(ptr) : : "lr");
>=20
>    return ptr + *(unsigned long *)ptr;
> }
>=20
>> I would imagine that this patch generates worse code on any
>> architecture with PC-relative addressing modes (which includes at
>> least x86_64, and I would guess includes most modern architectures).
>=20
> Why ? Powerpc is also using PC-relative addressing for all calls but indir=
ect calls.

I mean PC-relative access for data.  The data page is at a constant, known o=
ffset from the vDSO text.

I haven=E2=80=99t checked how much x86_64 benefits from this, but at least t=
he non-array fields ought to be accessible with a PC-relative access.

It should be possible to refactor a little bit so that the compiler can stil=
l see what=E2=80=99s going on.  Maybe your patch actually does this. I=E2=80=
=99d want to look at the assembly.  This also might not matter much on x86_6=
4 in particular, since x86_64 can convert a PC-relative address to an absolu=
te address with a single instruction with no clobbers.

Does power have PC-relative data access?  If so, I wonder if the code can be=
 arranged so that even the array accesses don=E2=80=99t require computing an=
 absolute address at any point.