Re: [PATCH 3/5] arm64/vdso: Add time napespace page

From: Vincenzo Frascino
Date: Thu Feb 20 2020 - 07:03:32 EST

Next message: Srinivas Kandagatla: "Re: [PATCH v4 2/4] gpiolib: use kref in gpio_desc"
Previous message: Jin, Yao: "Re: [PATCH v1 0/2] perf report: Support annotation of code without symbols"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Andrei,

On 04/02/2020 17:59, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages. Provide
> __arch_get_timens_vdso_data() helper for VDSO code to get the
> code-relative position of VVARs on that special page.
>
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
>
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
>
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
>
> Signed-off-by: Andrei Vagin <avagin@xxxxxxxxx>
> ---
> arch/arm64/Kconfig | 1 +
> .../arm64/include/asm/vdso/compat_gettimeofday.h | 11 +++++++++++
> arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++++++++
> arch/arm64/kernel/vdso.c | 16 +++++++++++++---
> arch/arm64/kernel/vdso/vdso.lds.S | 3 ++-
> arch/arm64/kernel/vdso32/vdso.lds.S | 3 ++-
> include/vdso/datapage.h | 1 +
> 7 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index e688dfad0b72..a671c2e36e5f 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -109,6 +109,7 @@ config ARM64
> select GENERIC_STRNLEN_USER
> select GENERIC_TIME_VSYSCALL
> select GENERIC_GETTIMEOFDAY
> + select GENERIC_VDSO_TIME_NS
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_PCI

Could you please add the selection in a separate patch at the end of this series?

> diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> index 537b1e695365..30a674f598c7 100644
> --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> @@ -161,6 +161,17 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
> return ret;
> }
>
> +#ifdef CONFIG_TIME_NS
> +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> + const struct vdso_data *ret;
> +
> + asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
> +
> + return ret;
> +}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
> index b08f476b72b4..aa38e80dfbc4 100644
> --- a/arch/arm64/include/asm/vdso/gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/gettimeofday.h
> @@ -98,6 +98,14 @@ const struct vdso_data *__arch_get_vdso_data(void)
> return _vdso_data;
> }
>
> +#ifdef CONFIG_TIME_NS
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> + return _timens_data;
> +}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 5ef808ddf08c..bc93e26ae485 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -46,6 +46,10 @@ enum arch_vdso_type {
> #define VDSO_TYPES (ARM64_VDSO + 1)
> #endif /* CONFIG_COMPAT_VDSO */
>
> +#define VVAR_DATA_PAGE_OFFSET 0
> +#define VVAR_TIMENS_PAGE_OFFSET 1
> +#define VVAR_NR_PAGES 2
> +

Maybe we could change this in an enumeration. Something like:

enum vvar_pages {
VVAR_DATA_PAGE_OFFSET = 0,
#ifdef CONFIG_TIME_NS
VVAR_TIMENS_PAGE_OFFSET = 1,
#endif /* CONFIG_TIME_NS */
VVAR_NR_PAGES = __VDSO_PAGES,
(look at the change suggested below for vdso.h)
};

> struct __vdso_abi {
> const char *name;
> const char *vdso_code_start;
> @@ -81,6 +85,12 @@ static union {
> } vdso_data_store __page_aligned_data;
> struct vdso_data *vdso_data = vdso_data_store.data;
>
> +
> +struct vdso_data *arch_get_vdso_data(void *vvar_page)
> +{
> + return (struct vdso_data *)(vvar_page);
> +}
> +
> static int __vdso_remap(enum arch_vdso_type arch_index,
> const struct vm_special_mapping *sm,
> struct vm_area_struct *new_vma)
> @@ -182,7 +192,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
>
> vdso_text_len = vdso_lookup[arch_index].vdso_pages << PAGE_SHIFT;
> /* Be sure to map the data page */
> - vdso_mapping_len = vdso_text_len + PAGE_SIZE;
> + vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
>
> vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
> if (IS_ERR_VALUE(vdso_base)) {
> @@ -190,13 +200,13 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
> goto up_fail;
> }
>
> - ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
> + ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
> VM_READ|VM_MAYREAD|VM_PFNMAP,
> vdso_lookup[arch_index].dm);
> if (IS_ERR(ret))
> goto up_fail;
>
> - vdso_base += PAGE_SIZE;
> + vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
> mm->context.vdso = (void *)vdso_base;
> ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
> VM_READ|VM_EXEC|
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index 7ad2d3a0cd48..a90b7d14e990 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -17,7 +17,8 @@ OUTPUT_ARCH(aarch64)
>
> SECTIONS
> {
> - PROVIDE(_vdso_data = . - PAGE_SIZE);
> + PROVIDE(_vdso_data = . - 2 * PAGE_SIZE);

Maybe we could have some conditional definition on CONFIG_TIME_NS in vdso.h:

#ifdef CONFIG_TIME_NS
#define __VVAR_PAGES 2
#else
#define __VVAR_PAGES 1
#endif

and then here:

PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);

just to not forget what "2" stands for. What do you think?

> + PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
> . = VDSO_LBASE + SIZEOF_HEADERS;
>
> .hash : { *(.hash) } :text
> diff --git a/arch/arm64/kernel/vdso32/vdso.lds.S b/arch/arm64/kernel/vdso32/vdso.lds.S
> index a3944927eaeb..3e432b536e53 100644
> --- a/arch/arm64/kernel/vdso32/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso32/vdso.lds.S
> @@ -17,7 +17,8 @@ OUTPUT_ARCH(arm)
>
> SECTIONS
> {
> - PROVIDE_HIDDEN(_vdso_data = . - PAGE_SIZE);
> + PROVIDE_HIDDEN(_vdso_data = . - 2 * PAGE_SIZE);

Ditto.

> + PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE);
> . = VDSO_LBASE + SIZEOF_HEADERS;
>
> .hash : { *(.hash) } :text
> diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
> index c5f347cc5e55..57eec6caca69 100644
> --- a/include/vdso/datapage.h
> +++ b/include/vdso/datapage.h
> @@ -100,6 +100,7 @@ struct vdso_data {
> * relocation, and this is what we need.
> */
> extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
> +extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
>
> #endif /* !__ASSEMBLY__ */
>
>

--
Regards,
Vincenzo

Attachment: pEpkey.asc
Description: application/pgp-keys

Next message: Srinivas Kandagatla: "Re: [PATCH v4 2/4] gpiolib: use kref in gpio_desc"
Previous message: Jin, Yao: "Re: [PATCH v1 0/2] perf report: Support annotation of code without symbols"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]