Messages in this thread | | | Date | Thu, 16 Apr 2020 11:45:27 +0100 | From | Mark Rutland <> | Subject | Re: [PATCH 3/6] arm64/vdso: Add time napespace page |
| |
Hi Andrei,
On Wed, Apr 15, 2020 at 10:26:15PM -0700, Andrei Vagin wrote: > diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h > index 07468428fd29..351c145d3808 100644 > --- a/arch/arm64/include/asm/vdso.h > +++ b/arch/arm64/include/asm/vdso.h > @@ -12,6 +12,12 @@ > */ > #define VDSO_LBASE 0x0 > > +#ifdef CONFIG_TIME_NS > +#define __VVAR_PAGES 2 > +#else > +#define __VVAR_PAGES 1 > +#endif > + > #ifndef __ASSEMBLY__
> +#ifdef CONFIG_TIME_NS > +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void) > +{ > + const struct vdso_data *ret; > + > + ret = _timens_data; > + OPTIMIZER_HIDE_VAR(ret); > + > + return ret; > +} > +#endif
Sorry for the confusion here, but please either:
* Add a preparatory patch making __arch_get_vdso_data() use OPTIMIZER_HIDE_VAR(), and use OPTIMIZER_HIDE_VAR() here.
* Use the same assembly as __arch_get_vdso_data() currently does.
... and either way add a comment here:
/* See __arch_get_vdso_data() */
... so taht the rationale is obvious.
[...]
> +enum vvar_pages { > + VVAR_DATA_PAGE_OFFSET = 0, > +#ifdef CONFIG_TIME_NS > + VVAR_TIMENS_PAGE_OFFSET = 1, > +#endif /* CONFIG_TIME_NS */ > + VVAR_NR_PAGES = __VVAR_PAGES, > +};
Pet peeve, but we don't need the initializers here, as enums start from zero. The last element shouldn't have a trailing comma as we don't expect to add elements after it in future.
Rather than assigning to VVAR_NR_PAGES, it'd be better to use a BUILD_BUG_ON() to verify that it is the number we expect:
enum vvar_pages { VVAR_DATA_PAGE, #ifdef CONFIG_TIME_NS VVAR_TIMENS_PAGE, #endif VVAR_NR_PAGES };
BUILD_BUG_ON(VVAR_NR_PAGES != __VVAR_PAGES);
Thanks, Mark.
| |