[PATCH v4] ARM: vDSO gettimeofday using generic timer architecture
David Riley
davidriley at google.com
Fri Mar 21 13:05:38 EDT 2014
I was thinking the same thing about having the data section before the
code section would make things more robust. I'll try out the
incremental patch.
Here are the contents of my vdso.so.dbg:
000003c0 t $a
000004b4 t $a
00000700 t $a
00000780 t $a
0000081c t $a
0000082c t $a
0000083c t $a
00000870 t $a
00000b48 t $a
00000b58 t $a
00000b6c t $a
000004ac t $d
000002ec r $d
00000364 r $d
000006f8 t $d
0000077c t $d
00000818 t $d
00000828 t $d
00000838 t $d
00000010 N $d
000001f8 N $d
0000004c N $d
0000006c N $d
0000086c t $d
0000002c N $d
00000250 r $d
00000b54 t $d
00000b64 t $d
00000b74 t $d
00000880 t $t
00000b30 t $t
00000b68 t $t
00000886 t .divsi3_skip_div0_test
00000000 A LINUX_3.15
00000264 a _DYNAMIC
00000b78 a _GLOBAL_OFFSET_TABLE_
00000b69 t ____aeabi_idiv0_from_thumb
00000b58 t ____aeabi_idiv_veneer
00000b48 t ____aeabi_llsr_veneer
00000881 t __aeabi_idiv
0000081c t __aeabi_idiv0
00000b15 t __aeabi_idivmod
0000082c t __aeabi_ldiv0
00000b31 t __aeabi_llsr
0000083c t __aeabi_unwind_cpp_pr0
0000084c t __aeabi_unwind_cpp_pr1
0000085c t __aeabi_unwind_cpp_pr2
0000080c t __div0
00000881 t __divsi3
00000870 t __get_datapage
00000700 T __kernel_clock_getres
000004b4 T __kernel_clock_gettime
00000780 T __kernel_gettimeofday
00000b31 t __lshrdi3
00001000 a _vdso_data
000003c0 t do_realtime
00000000 a shift
On Fri, Mar 21, 2014 at 9:16 AM, Nathan Lynch <Nathan_Lynch at mentor.com> wrote:
> On 03/21/2014 09:58 AM, Steve Capper wrote:
>> On 18 March 2014 00:49, David Riley <davidriley at chromium.org> wrote:
>>> Hi Nathan,
>>>
>>> I gave this a try against a 3.10 system and ran into issues when
>>> vdso.so grew to two pages because of of a large .GCC.command.line
>>> section. __get_datapage was returning an address that was at the
>>> beginning of the second code page instead of the data page since it
>>> didn't account for the additional section. I got it working by adding
>>> .GCC.command.line to the DISCARD group in the linker script, but in
>>> general it feels as if this code is a bit fragile since it depends on
>>> knowing exactly which sections exist in the .so for the _vdso_data
>>> symbol to be correct.
>>>
>>> Cheers,
>>> David
>>>
>>
>> Hi David,
>> Could you please send us the following output:
>> $ nm -C ./arch/arm/kernel/vdso/vdso.so.dbg
>>
>> For the vdso without the extra DISCARD?
>> (As I'm interested in the alignment of __vdso_data)
>
> Something like this, I imagine: (I just added -frecord-gcc-switches to
> ccflags-y):
>
> 00000770 t __aeabi_idiv0
> 00000774 t __aeabi_ldiv0
> 00000778 t __aeabi_unwind_cpp_pr0
> 0000077c t __aeabi_unwind_cpp_pr1
> 00000780 t __aeabi_unwind_cpp_pr2
> 0000076c t __div0
> 000002e8 t do_realtime
> 00000240 a _DYNAMIC
> 00000788 t __get_datapage
> 00000798 a _GLOBAL_OFFSET_TABLE_
> 0000066c T __kernel_clock_getres
> 000003f8 T __kernel_clock_gettime
> 000006e4 T __kernel_gettimeofday
> 00000000 A LINUX_3.15
> 00001000 d _vdso_data
>
> And yes, vdso.so.dbg and vdso.so come out larger than 4K. So
> _vdso_data is wrong (we'd want it to be 0x2000).
>
> The issue, I think, is the linker is free to deposit "orphan" sections
> -- those which aren't explicitly treated in the linker script -- such
> as .GCC.command.line wherever it sees fit, and the counter in the
> linker script doesn't account for those. So it seems to me that
> mapping the data page after the text is a losing game, and it's more
> robust to map it at the beginning of the VMA.
>
> Incremental patch against v4 below, tested okay on OMAP5:
>
> arch/arm/include/asm/elf.h | 11 +++++++----
> arch/arm/include/asm/vdso_datapage.h | 7 +++++++
> arch/arm/kernel/asm-offsets.c | 5 +++++
> arch/arm/kernel/vdso.c | 14 ++++++--------
> arch/arm/kernel/vdso/datapage.S | 3 ++-
> arch/arm/kernel/vdso/vdso.lds.S | 6 +++---
> 6 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
> index fc96b78a8102..45d2ddff662a 100644
> --- a/arch/arm/include/asm/elf.h
> +++ b/arch/arm/include/asm/elf.h
> @@ -3,6 +3,7 @@
>
> #include <asm/auxvec.h>
> #include <asm/hwcap.h>
> +#include <asm/vdso_datapage.h>
>
> /*
> * ELF register definitions..
> @@ -131,10 +132,12 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm);
>
> #ifdef CONFIG_MMU
> #ifdef CONFIG_VDSO
> -#define ARCH_DLINFO \
> -do { \
> - NEW_AUX_ENT(AT_SYSINFO_EHDR, \
> - (elf_addr_t)current->mm->context.vdso); \
> +#define ARCH_DLINFO \
> +do { \
> + /* Account for the data page at the beginning of the [vdso] VMA. */ \
> + NEW_AUX_ENT(AT_SYSINFO_EHDR, \
> + (elf_addr_t)current->mm->context.vdso + \
> + sizeof(union vdso_data_store)); \
> } while (0)
> #endif
> #define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1
> diff --git a/arch/arm/include/asm/vdso_datapage.h b/arch/arm/include/asm/vdso_datapage.h
> index 9011ec75a24b..f08bdb73d3f4 100644
> --- a/arch/arm/include/asm/vdso_datapage.h
> +++ b/arch/arm/include/asm/vdso_datapage.h
> @@ -22,6 +22,8 @@
>
> #ifndef __ASSEMBLY__
>
> +#include <asm/page.h>
> +
> /* Try to be cache-friendly on systems that don't implement the
> * generic timer: fit the unconditionally updated fields in the first
> * 32 bytes.
> @@ -46,6 +48,11 @@ struct vdso_data {
> u32 tz_dsttime;
> };
>
> +union vdso_data_store {
> + struct vdso_data data;
> + u8 page[PAGE_SIZE];
> +};
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __KERNEL__ */
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index ded041711beb..dda3363ef0bf 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -24,6 +24,7 @@
> #include <asm/memory.h>
> #include <asm/procinfo.h>
> #include <asm/suspend.h>
> +#include <asm/vdso_datapage.h>
> #include <asm/hardware/cache-l2x0.h>
> #include <linux/kbuild.h>
>
> @@ -199,5 +200,9 @@ int main(void)
> #endif
> DEFINE(KVM_VTTBR, offsetof(struct kvm, arch.vttbr));
> #endif
> + BLANK();
> +#ifdef CONFIG_VDSO
> + DEFINE(VDSO_DATA_SIZE, sizeof(union vdso_data_store));
> +#endif
> return 0;
> }
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index 91cbc5f14fb5..1a2bc699c1ac 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -33,11 +33,8 @@ static unsigned long vdso_pages;
> static unsigned long vdso_mapping_len;
> static struct page **vdso_pagelist;
>
> -static union {
> - struct vdso_data data;
> - u8 page[PAGE_SIZE];
> -} vdso_data_store __page_aligned_data;
> -struct vdso_data *vdso_data = &vdso_data_store.data;
> +static union vdso_data_store vdso_data_store __page_aligned_data;
> +static struct vdso_data *vdso_data = &vdso_data_store.data;
>
> /*
> * The vDSO data page.
> @@ -62,12 +59,13 @@ static int __init vdso_init(void)
> if (vdso_pagelist == NULL)
> return -ENOMEM;
>
> + /* Grab the vDSO data page. */
> + vdso_pagelist[0] = virt_to_page(vdso_data);
> +
> /* Grab the vDSO code pages. */
> for (i = 0; i < vdso_pages; i++)
> - vdso_pagelist[i] = virt_to_page(&vdso_start + i * PAGE_SIZE);
> + vdso_pagelist[i + 1] = virt_to_page(&vdso_start + i * PAGE_SIZE);
>
> - /* Grab the vDSO data page. */
> - vdso_pagelist[i] = virt_to_page(vdso_data);
>
> /* Precompute the mapping size */
> vdso_mapping_len = (vdso_pages + 1) << PAGE_SHIFT;
> diff --git a/arch/arm/kernel/vdso/datapage.S b/arch/arm/kernel/vdso/datapage.S
> index 91b19b815888..fbf36d75da06 100644
> --- a/arch/arm/kernel/vdso/datapage.S
> +++ b/arch/arm/kernel/vdso/datapage.S
> @@ -1,8 +1,9 @@
> #include <linux/linkage.h>
> +#include <asm/asm-offsets.h>
>
> .align 2
> .L_vdso_data_ptr:
> - .long _vdso_data - .
> + .long _start - . - VDSO_DATA_SIZE
>
> ENTRY(__get_datapage)
> .cfi_startproc
> diff --git a/arch/arm/kernel/vdso/vdso.lds.S b/arch/arm/kernel/vdso/vdso.lds.S
> index 24dd7b84e366..799d259f86e0 100644
> --- a/arch/arm/kernel/vdso/vdso.lds.S
> +++ b/arch/arm/kernel/vdso/vdso.lds.S
> @@ -30,6 +30,8 @@ OUTPUT_ARCH(arm)
>
> SECTIONS
> {
> + PROVIDE(_start = .);
> +
> . = VDSO_LBASE + SIZEOF_HEADERS;
>
> .hash : { *(.hash) } :text
> @@ -55,14 +57,12 @@ SECTIONS
> .got : { *(.got) }
> .rel.plt : { *(.rel.plt) }
>
> - . = ALIGN(PAGE_SIZE);
> - PROVIDE(_vdso_data = .);
> -
> /DISCARD/ : {
> *(.note.GNU-stack)
> *(.data .data.* .gnu.linkonce.d.* .sdata*)
> *(.bss .sbss .dynbss .dynsbss)
> }
> +
> }
>
> /*
>
More information about the linux-arm-kernel
mailing list