Discussions around time namespace are there for a long time. The first
attempt to implement it was in 2006 by Jeff Dike. From that time, the
topic appears on and off in various discussions.
There are two main use cases for time namespaces:
1. change date and time inside a container;
2. adjust clocks for a container restored from a checkpoint.
“It seems like this might be one of the last major obstacles keeping
migration from being used in production systems, given that not all
containers and connections can be migrated as long as a time dependency
is capable of messing it up.” (by github.com/dav-ell)
The kernel provides access to several clocks: CLOCK_REALTIME,
CLOCK_MONOTONIC, CLOCK_BOOTTIME. Last two clocks are monotonous, but the
start points for them are not defined and are different for each
system. When a container is migrated from one node to another, all
clocks have to be restored into consistent states; in other words, they
have to continue running from the same points where they have been
dumped.
The main idea of this patch set is adding per-namespace offsets for
system clocks. When a process in a non-root time namespace requests
time of a clock, a namespace offset is added to the current value of
this clock and the sum is returned.
All offsets are placed on a separate page, this allows us to map it as
part of VVAR into user processes and use offsets from VDSO calls.
Now offsets are implemented for CLOCK_MONOTONIC and CLOCK_BOOTTIME
clocks.
v4 Changes:
* CLOCKE_NEWTIME is unshare()-only flag now (CLON_PIDFD took previous value)
* Addressing Jann Horn's feedback - we don't allow CLONE_THREAD or
CLONE_VM together with CLONE_NEWTIME (thanks for spotting!)
* Addressing issues found by Thomas - removed unmaintainable CLOCK_TIMENS
and introduced another call back into k_clock to get ktime instead
of getting timespec and converting it (Patch 03)
* Renaming timens_offsets members to omit _offset postfix
(thanks Cyrill for the suggestion)
* Suggestions, renaming and making code more maintainable from Thomas's
feedback (thanks much!)
* Fixing out-of-bounds and other issues in procfs file (kudos Jann Horn)
* vdso_fault() can be called on a remote task by /proc/$pid/mem or
process_vm_readv() - addressed by adding a slow-path with searching
for owner's namespace (thanks for spotting this unobvious issue, Jann)
* Other nits by Jann Horn
v3: Major changes:
* Simplify two VDSO images by using static_branch() in vclock_gettime()
Removes unwanted conflicts with generic VDSO movement patches and
simplifies things by dropping too invasive linker magic.
As an alternative to static_branch() we tested an attempt to introduce
home-made dynamic patching called retcalls:
https://github.com/0x7f454c46/linux/commit/4cc0180f6d65
Considering some theoretical problems with toolchains, we decided to go
with long well-tested nop-patching in static_branch(). Though, it was
needed to provide backend for relative code.
* address Thomas' comments.
* add sanity checks for offsets:
- the current clock time in a namespace has to be in [0, KTIME_MAX / 2).
KTIME_MAX is divided by two here to be sure that the KTIME_MAX limit
is still unreachable.
Link: https://lkml.org/lkml/2018/9/19/950
Link: https://lkml.org/lkml/2019/2/5/867
v2: There are two major changes:
* Two versions of the VDSO library to avoid a performance penalty for
host tasks outside time namespace (as suggested by Andy and Thomas).
As it has been discussed on timens RFC, adding a new conditional branch
`if (inside_time_ns)` on VDSO for all processes is undesirable.
It will add a penalty for everybody as branch predictor may mispredict
the jump. Also there are instruction cache lines wasted on cmp/jmp.
Those effects of introducing time namespace are very much unwanted
having in mind how much work have been spent on micro-optimisation
VDSO code.
Addressing those problems, there are two versions of VDSO's .so:
for host tasks (without any penalty) and for processes inside of time
namespace with clk_to_ns() that subtracts offsets from host's time.
* Allow to set clock offsets for a namespace only before any processes
appear in it.
Now a time namespace looks similar to a pid namespace in a way how it is
created: unshare(CLONE_NEWTIME) system call creates a new time namespace,
but doesn't set it to the current process. Then all children of
the process will be born in the new time namespace, or a process can
use the setns() system call to join a namespace.
This scheme allows to create a new time namespaces, set clock offsets
and then populate the namespace with processes.
Our performance measurements show that the price of VDSO's clock_gettime()
in a child time namespace is about 8% with a hot CPU cache and about 90%
with a cold CPU cache. There is no performance regression for host
processes outside time namespace on those tests.
We wrote two small benchmarks. The first one gettime_perf.c calls
clock_gettime() in a loop for 3 seconds. It shows us performance with
a hot CPU cache (more clock_gettime() cycles - the better):
| before | CONFIG_TIME_NS=n | host | inside timens
--------|------------|------------------|-------------|-------------
cycles | 139887013 | 139453003 | 139899785 | 128792458
diff (%)| 100 | 99.7 | 100 | 92
The second one gettime_perf_cold.c calls rdtsc, clock_gettime(), rdtsc
and shows a difference of second and first rdtsc. We call this binary in
a loop 1000 times, get 1000 values and calculate MODE for them.
It should show us performance with a cold CPU cache
(lesser tsc per cycle - the better):
| before | CONFIG_TIME_NS=n | host | inside timens
--------|------------|------------------|-------------|-------------
tsc | 6748 | 6718 | 6862 | 12682
diff (%)| 100 | 99.6 | 101.7 | 188
The numbers gathered on Intel(R) Core(TM) i5-6300U CPU @ 2.40GHz.
Cc: Adrian Reber <redacted>
Cc: Andrei Vagin <redacted>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christian Brauner <redacted>
Cc: Cyrill Gorcunov <redacted>
Cc: Dmitry Safonov <redacted>
Cc: "Eric W. Biederman" <redacted>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: Jeff Dike <redacted>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Pavel Emelyanov <redacted>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Thomas Gleixner <redacted>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: containers@lists.linux-foundation.org
Cc: criu@openvz.org
Cc: linux-api@vger.kernel.org
Cc: x86@kernel.org
v3: https://lkml.kernel.org/r/20190425161416.26600-1-dima@arista.com
v2: https://lore.kernel.org/lkml/20190206001107.16488-1-dima@arista.com/
RFC: https://lkml.kernel.org/r/20180919205037.9574-1-dima@arista.com/
Andrei Vagin (17):
ns: Introduce Time Namespace
timens: Add timens_offsets
posix-clocks: add another call back to return clock time in ktime_t
timens: Introduce CLOCK_MONOTONIC offsets
timens: Introduce CLOCK_BOOTTIME offset
timerfd/timens: Take into account ns clock offsets
posix-timers/timens: Take into account clock offsets
timens/kernel: Take into account timens clock offsets in
clock_nanosleep
x86/vdso: Add offsets page in vvar
vdso: introduce timens_static_branch
timens/fs/proc: Introduce /proc/pid/timens_offsets
selftest/timens: Add a test for timerfd
selftest/timens: Add a test for clock_nanosleep()
selftest/timens: Add timer offsets test
x86/vdso: Align VDSO functions by CPU L1 cache line
selftests: Add a simple perf test for clock_gettime()
selftest/timens: Check that a right vdso is mapped after fork and exec
Dmitry Safonov (11):
timens: Shift /proc/uptime
x86/vdso2c: Correct err messages on file opening
x86/vdso2c: Convert iterator to unsigned
x86/vdso/Makefile: Add vobjs32
x86/vdso: Restrict splitting VVAR VMA
x86/vdso: Rename vdso_image {.data=>.text}
x86/vdso: Allocate timens vdso
x86/vdso: Switch image on setns()/unshare()/clone()
timens: Add align for timens_offsets
selftest/timens: Add Time Namespace test for supported clocks
selftest/timens: Add procfs selftest
MAINTAINERS | 3 +
arch/Kconfig | 5 +
arch/x86/Kconfig | 1 +
arch/x86/entry/vdso/Makefile | 16 +-
arch/x86/entry/vdso/vclock_gettime.c | 48 +++
arch/x86/entry/vdso/vdso-layout.lds.S | 10 +-
arch/x86/entry/vdso/vdso2c.c | 7 +-
arch/x86/entry/vdso/vdso2c.h | 24 +-
arch/x86/entry/vdso/vma.c | 189 ++++++++-
arch/x86/include/asm/jump_label.h | 14 +
arch/x86/include/asm/vdso.h | 14 +-
fs/proc/base.c | 95 +++++
fs/proc/namespaces.c | 4 +
fs/proc/uptime.c | 3 +
fs/timerfd.c | 3 +
include/linux/hrtimer.h | 2 +-
include/linux/jump_label.h | 5 +
include/linux/nsproxy.h | 2 +
include/linux/posix-timers.h | 3 +
include/linux/proc_ns.h | 2 +
include/linux/time_namespace.h | 115 ++++++
include/linux/timens_offsets.h | 18 +
include/linux/user_namespace.h | 1 +
include/uapi/linux/sched.h | 5 +
init/Kconfig | 8 +
kernel/Makefile | 1 +
kernel/fork.c | 29 +-
kernel/nsproxy.c | 41 +-
kernel/time/alarmtimer.c | 27 +-
kernel/time/hrtimer.c | 8 +-
kernel/time/posix-clock.c | 8 +-
kernel/time/posix-cpu-timers.c | 32 +-
kernel/time/posix-stubs.c | 15 +-
kernel/time/posix-timers.c | 87 +++--
kernel/time/posix-timers.h | 7 +-
kernel/time_namespace.c | 367 ++++++++++++++++++
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/timens/.gitignore | 8 +
tools/testing/selftests/timens/Makefile | 12 +
.../selftests/timens/clock_nanosleep.c | 100 +++++
tools/testing/selftests/timens/config | 1 +
tools/testing/selftests/timens/exec.c | 91 +++++
tools/testing/selftests/timens/gettime_perf.c | 74 ++++
.../selftests/timens/gettime_perf_cold.c | 63 +++
tools/testing/selftests/timens/log.h | 26 ++
tools/testing/selftests/timens/procfs.c | 142 +++++++
tools/testing/selftests/timens/timens.c | 188 +++++++++
tools/testing/selftests/timens/timens.h | 63 +++
tools/testing/selftests/timens/timer.c | 116 ++++++
tools/testing/selftests/timens/timerfd.c | 127 ++++++
50 files changed, 2132 insertions(+), 99 deletions(-)
create mode 100644 include/linux/time_namespace.h
create mode 100644 include/linux/timens_offsets.h
create mode 100644 kernel/time_namespace.c
create mode 100644 tools/testing/selftests/timens/.gitignore
create mode 100644 tools/testing/selftests/timens/Makefile
create mode 100644 tools/testing/selftests/timens/clock_nanosleep.c
create mode 100644 tools/testing/selftests/timens/config
create mode 100644 tools/testing/selftests/timens/exec.c
create mode 100644 tools/testing/selftests/timens/gettime_perf.c
create mode 100644 tools/testing/selftests/timens/gettime_perf_cold.c
create mode 100644 tools/testing/selftests/timens/log.h
create mode 100644 tools/testing/selftests/timens/procfs.c
create mode 100644 tools/testing/selftests/timens/timens.c
create mode 100644 tools/testing/selftests/timens/timens.h
create mode 100644 tools/testing/selftests/timens/timer.c
create mode 100644 tools/testing/selftests/timens/timerfd.c
--
2.22.0
From: Andrei Vagin <redacted>
The callsite in common_timer_get() has already a comment:
/*
* The timespec64 based conversion is suboptimal, but it's not
* worth to implement yet another callback.
*/
kc->clock_get(timr->it_clock, &ts64);
now = timespec64_to_ktime(ts64);
Now we are going to add time namespaces and we need to be able to get:
* clock value in a task time namespace to return it from the clock_gettime
syscall.
* clock valuse in the root time namespace to use it in
common_timer_get().
It looks like another reason why we need a separate callback to return
clock value in ktime_t.
Suggested-by: Thomas Gleixner <redacted>
Signed-off-by: Andrei Vagin <redacted>
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
include/linux/posix-timers.h | 3 ++
kernel/time/alarmtimer.c | 24 ++++++++++---
kernel/time/posix-clock.c | 8 ++---
kernel/time/posix-cpu-timers.c | 32 +++++++++---------
kernel/time/posix-timers.c | 61 ++++++++++++++++++++++------------
kernel/time/posix-timers.h | 7 ++--
6 files changed, 87 insertions(+), 48 deletions(-)
@@ -6,8 +6,11 @@ struct k_clock {structtimespec64*tp);int(*clock_set)(constclockid_twhich_clock,conststructtimespec64*tp);-int(*clock_get)(constclockid_twhich_clock,-structtimespec64*tp);+/* return the clock value in the current time namespace. */+int(*clock_get_timespec)(constclockid_twhich_clock,+structtimespec64*tp);+/* return the clock value in the root time namespace. */+ktime_t(*clock_get_ktime)(constclockid_twhich_clock);int(*clock_adj)(constclockid_twhich_clock,struct__kernel_timex*tx);int(*timer_create)(structk_itimer*timer);int(*nsleep)(constclockid_twhich_clock,intflags,
@@ -857,6 +857,8 @@ int common_timer_set(struct k_itimer *timr, int flags,timr->it_interval=timespec64_to_ktime(new_setting->it_interval);expires=timespec64_to_ktime(new_setting->it_value);+if(flags&TIMER_ABSTIME)+expires=timens_ktime_to_host(timr->it_clock,expires);sigev_none=timr->it_sigev_notify==SIGEV_NONE;kc->timer_arm(timr,expires,flags&TIMER_ABSTIME,sigev_none);
err() message in main() is misleading: it should print `outfilename`,
which is argv[3], not argv[2].
Correct error messages to be more precise about what failed and for
which file.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/vdso2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
i and j are used everywhere with unsigned types.
Cleanup and prettify the code a bit.
Introduce syms_nr for readability and as a preparation for allocating an
array of vDSO entries that will be needed for creating two vdso .so's:
one for host tasks and another for processes inside time namespace.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/vdso2c.h | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
@@ -13,7 +13,7 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,unsignedlongload_size=-1;/* Work around bogus warning */unsignedlongmapping_size;ELF(Ehdr)*hdr=(ELF(Ehdr)*)raw_addr;-inti;+unsignedinti,syms_nr;unsignedlongj;ELF(Shdr)*symtab_hdr=NULL,*strtab_hdr,*secstrings_hdr,*alt_sec=NULL;
@@ -86,11 +86,10 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,strtab_hdr=raw_addr+GET_LE(&hdr->e_shoff)+GET_LE(&hdr->e_shentsize)*GET_LE(&symtab_hdr->sh_link);+syms_nr=GET_LE(&symtab_hdr->sh_size)/GET_LE(&symtab_hdr->sh_entsize);/* Walk the symbol table */-for(i=0;-i<GET_LE(&symtab_hdr->sh_size)/GET_LE(&symtab_hdr->sh_entsize);-i++){-intk;+for(i=0;i<syms_nr;i++){+unsignedintk;ELF(Sym)*sym=raw_addr+GET_LE(&symtab_hdr->sh_offset)+GET_LE(&symtab_hdr->sh_entsize)*i;constchar*sym_name=raw_addr+
@@ -11,7 +11,7 @@#include<linux/mm_types.h>structvdso_image{-void*data;+void*text;unsignedlongsize;/* Always a multiple of PAGE_SIZE */unsignedlongalt,alt_len;
From: Andrei Vagin <redacted>
As it has been discussed on timens RFC, adding a new conditional branch
`if (inside_time_ns)` on VDSO for all processes is undesirable.
Addressing those problems, there are two versions of VDSO's .so:
for host tasks (without any penalty) and for processes inside of time
namespace with clk_to_ns() that subtracts offsets from host's time.
This patch introduces timens_static_branch(), which is similar with
static_branch_unlikely.
The timens code in vdso looks like this:
if (timens_static_branch()) {
clk_to_ns(clk, ts);
}
The version of vdso which is compiled from sources will never execute
clk_to_ns(). And then we can patch the 'no-op' in the straight-line
codepath with a 'jump' instruction to the out-of-line true branch and
get the timens version of the vdso library.
While cooking the patch, an alternative approach has being considered:
to omit no-ops - memcpy() the following asm ret sequience on the place of
a function call: https://github.com/0x7f454c46/linux/commit/4cc0180f6d65
Having in mind possible issues with different toolchains, the usual
static_branch() approach was choosen.
Signed-off-by: Andrei Vagin <redacted>
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/vclock_gettime.c | 9 +++++--
arch/x86/entry/vdso/vdso-layout.lds.S | 1 +
arch/x86/entry/vdso/vdso2c.h | 11 +++++++-
arch/x86/entry/vdso/vma.c | 37 ++++++++++++++++++++++-----
arch/x86/include/asm/jump_label.h | 14 ++++++++++
arch/x86/include/asm/vdso.h | 1 +
include/linux/jump_label.h | 5 ++++
7 files changed, 69 insertions(+), 9 deletions(-)
@@ -16,6 +16,7 @@ struct vdso_image {unsignedlongsize;/* Always a multiple of PAGE_SIZE */unsignedlongalt,alt_len;+unsignedlongjump_table,jump_table_len;longsym_vvar_start;/* Negative offset to the vvar area */
@@ -125,6 +125,11 @@ struct jump_entry {longkey;// key may be far away from the core kernel under KASLR};+structvdso_jump_entry{+u16code;+u16target;+};+staticinlineunsignedlongjump_entry_code(conststructjump_entry*entry){return(unsignedlong)&entry->code+entry->code;
@@ -0,0 +1,142 @@+// SPDX-License-Identifier: GPL-2.0+#define _GNU_SOURCE+#include<errno.h>+#include<fcntl.h>+#include<math.h>+#include<sched.h>+#include<stdio.h>+#include<stdbool.h>+#include<stdlib.h>+#include<sys/stat.h>+#include<sys/syscall.h>+#include<sys/types.h>+#include<time.h>+#include<unistd.h>+#include<time.h>++#include"log.h"+#include"timens.h"++/*+*Testshouldn'tberunforaday,soadd10daystochild+*timeandcheckparent'stimetobeinthesameday.+*/+#define MAX_TEST_TIME_SEC (60*5)+#define DAY_IN_SEC (60*60*24)+#define TEN_DAYS_IN_SEC (10*DAY_IN_SEC)++#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))++staticintchild_ns,parent_ns;++staticintswitch_ns(intfd)+{+if(setns(fd,CLONE_NEWTIME))+returnpr_perror("setns()");++return0;+}++staticintinit_namespaces(void)+{+charpath[]="/proc/self/ns/time_for_children";+structstatst1,st2;++parent_ns=open(path,O_RDONLY);+if(parent_ns<=0)+returnpr_perror("Unable to open %s",path);++if(fstat(parent_ns,&st1))+returnpr_perror("Unable to stat the parent timens");++if(unshare(CLONE_NEWTIME))+returnpr_perror("Can't unshare() timens");++child_ns=open(path,O_RDONLY);+if(child_ns<=0)+returnpr_perror("Unable to open %s",path);++if(fstat(child_ns,&st2))+returnpr_perror("Unable to stat the timens");++if(st1.st_ino==st2.st_ino)+returnpr_err("The same child_ns after CLONE_NEWTIME");++if(_settime(CLOCK_BOOTTIME,TEN_DAYS_IN_SEC))+return-1;++return0;+}++staticintread_proc_uptime(structtimespec*uptime)+{+unsignedlongup_sec,up_nsec;+FILE*proc;++proc=fopen("/proc/uptime","r");+if(proc==NULL){+pr_perror("Unable to open /proc/uptime");+return-1;+}++if(fscanf(proc,"%lu.%02lu",&up_sec,&up_nsec)!=2){+if(errno){+pr_perror("fscanf");+return-errno;+}+pr_err("failed to parse /proc/uptime");+return-1;+}+fclose(proc);++uptime->tv_sec=up_sec;+uptime->tv_nsec=up_nsec;+return0;+}++staticintcheck_uptime(void)+{+structtimespecuptime_new,uptime_old;+time_tuptime_expected;+doubleprec=MAX_TEST_TIME_SEC;++if(switch_ns(parent_ns))+returnpr_err("switch_ns(%d)",parent_ns);++if(read_proc_uptime(&uptime_old))+return1;++if(switch_ns(child_ns))+returnpr_err("switch_ns(%d)",child_ns);++if(read_proc_uptime(&uptime_new))+return1;++uptime_expected=uptime_old.tv_sec+TEN_DAYS_IN_SEC;+if(fabs(difftime(uptime_new.tv_sec,uptime_expected))>prec){+pr_fail("uptime in /proc/uptime: old %ld, new %ld [%ld]",+uptime_old.tv_sec,uptime_new.tv_sec,+uptime_old.tv_sec+TEN_DAYS_IN_SEC);+return1;+}++ksft_test_result_pass("Passed for /proc/uptime\n");+return0;+}++intmain(intargc,char*argv[])+{+intret=0;++nscheck();++if(init_namespaces())+return1;++ret|=check_uptime();++if(ret)+ksft_exit_fail();+ksft_exit_pass();+returnret;+}
@@ -0,0 +1,63 @@+// SPDX-License-Identifier: GPL-2.0+#define _GNU_SOURCE+#include<sys/types.h>+#include<sys/stat.h>+#include<errno.h>+#include<fcntl.h>+#include<sched.h>+#include<time.h>+#include<stdio.h>+#include<unistd.h>+#include<sys/syscall.h>+#include<string.h>++#include"log.h"+#include"timens.h"++static__inline__unsignedlonglongrdtsc(void)+{+unsignedhi,lo;++__asm____volatile__("rdtsc":"=a"(lo),"=d"(hi));+return((unsignedlonglong)lo)|(((unsignedlonglong)hi)<<32);+}++staticvoidtest(clock_tclockid,char*clockstr)+{+structtimespectp;+longlongs,e;++s=rdtsc();+clock_gettime(clockid,&tp);+e=rdtsc();+printf("%lld\n",e-s);+return;+}++intmain(intargc,char**argv)+{+time_toffset=10;+intnsfd;++if(argc==1){+test(CLOCK_MONOTONIC,"monotonic");+return0;+}+nscheck();++if(unshare(CLONE_NEWTIME))+returnpr_perror("Can't unshare() timens");++nsfd=open("/proc/self/ns/time_for_children",O_RDONLY);+if(nsfd<0)+returnpr_perror("Can't open a time namespace");++if(_settime(CLOCK_MONOTONIC,offset))+return1;++if(setns(nsfd,CLONE_NEWTIME))+returnpr_perror("setns");++test(CLOCK_MONOTONIC,"monotonic");+return0;+}
@@ -0,0 +1,91 @@+// SPDX-License-Identifier: GPL-2.0+#define _GNU_SOURCE+#include<errno.h>+#include<fcntl.h>+#include<sched.h>+#include<stdio.h>+#include<stdbool.h>+#include<sys/stat.h>+#include<sys/syscall.h>+#include<sys/types.h>+#include<sys/wait.h>+#include<time.h>+#include<unistd.h>+#include<time.h>+#include<string.h>++#include"log.h"+#include"timens.h"++#define OFFSET (36000)++intmain(intargc,char*argv[])+{+structtimespecnow,tst;+intstatus,i;+pid_tpid;++if(argc>1){+if(sscanf(argv[1],"%ld",&now.tv_sec)!=1)+returnpr_perror("sscanf");++for(i=0;i<2;i++){+_gettime(CLOCK_MONOTONIC,&tst,i);+if(abs(tst.tv_sec-now.tv_sec)>5)+returnpr_fail("%ld %ld\n",now.tv_sec,tst.tv_sec);+}+}++nscheck();++clock_gettime(CLOCK_MONOTONIC,&now);++if(unshare(CLONE_NEWTIME))+returnpr_perror("Can't unshare() timens");++if(_settime(CLOCK_MONOTONIC,OFFSET))+return1;++for(i=0;i<2;i++){+_gettime(CLOCK_MONOTONIC,&tst,i);+if(abs(tst.tv_sec-now.tv_sec)>5)+returnpr_fail("%ld %ld\n",+now.tv_sec,tst.tv_sec);+}++if(argc>1)+return0;++pid=fork();+if(pid<0)+returnpr_perror("fork");++if(pid==0){+charnow_str[64];+char*cargv[]={"exec",now_str,NULL};+char*cenv[]={NULL};++/* Check that a child process is in the new timens. */+for(i=0;i<2;i++){+_gettime(CLOCK_MONOTONIC,&tst,i);+if(abs(tst.tv_sec-now.tv_sec-OFFSET)>5)+returnpr_fail("%ld %ld\n",+now.tv_sec+OFFSET,tst.tv_sec);+}++/* Check that a proper vdso will be mapped after execve. */+snprintf(now_str,sizeof(now_str),"%ld",now.tv_sec+OFFSET);+execve("/proc/self/exe",cargv,cenv);+returnpr_perror("execve");+}++if(waitpid(pid,&status,0)!=pid)+returnpr_perror("waitpid");++if(status)+ksft_exit_fail();++ksft_test_result_pass("exec\n");+ksft_exit_pass();+return0;+}
From: Andrei Vagin <redacted>
After performance testing VDSO patches a noticeable 20% regression was
found on gettime_perf selftest with a cold cache.
As it turns to be, before time namespaces introduction, VDSO functions
were quite aligned to cache lines, but adding a new code to adjust
timens offset inside namespace created a small shift and vdso functions
become unaligned on cache lines.
Add align to vdso functions with gcc option to fix performance drop.
Coping the resulting numbers from cover letter:
Hot CPU cache (more gettime_perf.c cycles - the better):
| before | CONFIG_TIME_NS=n | host | inside timens
--------|------------|------------------|-------------|-------------
cycles | 139887013 | 139453003 | 139899785 | 128792458
diff (%)| 100 | 99.7 | 100 | 92
Cold cache (lesser tsc per gettime_perf_cold.c cycle - the better):
| before | CONFIG_TIME_NS=n | host | inside timens
--------|------------|------------------|-------------|-------------
tsc | 6748 | 6718 | 6862 | 12682
diff (%)| 100 | 99.6 | 101.7 | 188
Measured on Intel(R) Core(TM) i5-6300U CPU @ 2.40GHz
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/Makefile | 1 +
1 file changed, 1 insertion(+)
@@ -0,0 +1,100 @@+// SPDX-License-Identifier: GPL-2.0+#define _GNU_SOURCE+#include<sched.h>++#include<sys/timerfd.h>+#include<sys/syscall.h>+#include<time.h>+#include<unistd.h>+#include<stdlib.h>+#include<stdio.h>+#include<stdint.h>++#include"log.h"+#include"timens.h"++staticlonglongget_elapsed_time(intclockid,structtimespec*start)+{+structtimespeccurr;+longlongsecs,nsecs;++if(clock_gettime(clockid,&curr)==-1)+returnpr_perror("clock_gettime");++secs=curr.tv_sec-start->tv_sec;+nsecs=curr.tv_nsec-start->tv_nsec;+if(nsecs<0){+secs--;+nsecs+=1000000000;+}+if(nsecs>1000000000){+secs++;+nsecs-=1000000000;+}+returnsecs*1000+nsecs/1000000;+}++intrun_test(intclockid)+{+longlongelapsed;+inti;++for(i=0;i<2;i++){+structtimespecnow={};+structtimespecstart;++if(clock_gettime(clockid,&start)==-1)+returnpr_perror("clock_gettime");+++if(i==1){+now.tv_sec=start.tv_sec;+now.tv_nsec=start.tv_nsec;+}++now.tv_sec+=2;+clock_nanosleep(clockid,i?TIMER_ABSTIME:0,&now,NULL);++elapsed=get_elapsed_time(clockid,&start);+if(elapsed<1900||elapsed>2100){+pr_fail("clockid: %d abs: %d elapsed: %lld\n",+clockid,i,elapsed);+return1;+}+ksft_test_result_pass("clockid: %d abs:%d\n",clockid,i);+}++return0;+}++intmain(intargc,char*argv[])+{+intret,nsfd;++nscheck();++if(unshare(CLONE_NEWTIME))+returnpr_perror("unshare");++if(_settime(CLOCK_MONOTONIC,7*24*3600))+return1;+if(_settime(CLOCK_BOOTTIME,9*24*3600))+return1;++nsfd=open("/proc/self/ns/time_for_children",O_RDONLY);+if(nsfd<0)+returnpr_perror("Unable to open timens_for_children");++if(setns(nsfd,CLONE_NEWTIME))+returnpr_perror("Unable to set timens");++ret=0;+ret|=run_test(CLOCK_MONOTONIC);+ret|=run_test(CLOCK_BOOTTIME_ALARM);++if(ret)+ksft_exit_fail();+ksft_exit_pass();+returnret;+}+
Align offsets so that time namespace will work for ia32 applications on
x86_64 host.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
include/linux/timens_offsets.h | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
A test to check that all supported clocks work on host and inside
a new time namespace. Use both ways to get time: through VDSO and
by entering the kernel with implicit syscall.
Introduce a new timens directory in selftests framework for
the next timens tests.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/timens/.gitignore | 1 +
tools/testing/selftests/timens/Makefile | 5 +
tools/testing/selftests/timens/config | 1 +
tools/testing/selftests/timens/log.h | 26 +++
tools/testing/selftests/timens/timens.c | 188 ++++++++++++++++++++++
tools/testing/selftests/timens/timens.h | 63 ++++++++
7 files changed, 285 insertions(+)
create mode 100644 tools/testing/selftests/timens/.gitignore
create mode 100644 tools/testing/selftests/timens/Makefile
create mode 100644 tools/testing/selftests/timens/config
create mode 100644 tools/testing/selftests/timens/log.h
create mode 100644 tools/testing/selftests/timens/timens.c
create mode 100644 tools/testing/selftests/timens/timens.h
@@ -0,0 +1,188 @@+// SPDX-License-Identifier: GPL-2.0+#define _GNU_SOURCE+#include<errno.h>+#include<fcntl.h>+#include<sched.h>+#include<stdio.h>+#include<stdbool.h>+#include<sys/stat.h>+#include<sys/syscall.h>+#include<sys/types.h>+#include<time.h>+#include<unistd.h>+#include<time.h>+#include<string.h>++#include"log.h"+#include"timens.h"++/*+*Testshouldn'tberunforaday,soadd10daystochild+*timeandcheckparent'stimetobeinthesameday.+*/+#define DAY_IN_SEC (60*60*24)+#define TEN_DAYS_IN_SEC (10*DAY_IN_SEC)++#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))++#define CLOCK_TYPES \+ct(CLOCK_BOOTTIME,-1),\+ct(CLOCK_BOOTTIME_ALARM,1),\+ct(CLOCK_MONOTONIC,-1),\+ct(CLOCK_MONOTONIC_COARSE,1),\+ct(CLOCK_MONOTONIC_RAW,1),\+++structtest_clock{+clockid_tid;+char*name;+/*+*off_idis-1ifaclockhasownoffset,oritcontainsanindex+*whichcontainsarightoffsetofthisclock.+*/+intoff_id;+time_toffset;+};++#define ct(clock, off_id) { clock, #clock, off_id }+staticstructtest_clockclocks[]={+CLOCK_TYPES+};+#undef ct++staticintchild_ns,parent_ns=-1;++staticintswitch_ns(intfd)+{+if(setns(fd,CLONE_NEWTIME)){+pr_perror("setns()");+return-1;+}++return0;+}++staticintinit_namespaces(void)+{+charpath[]="/proc/self/ns/time_for_children";+structstatst1,st2;++if(parent_ns==-1){+parent_ns=open(path,O_RDONLY);+if(parent_ns<=0)+returnpr_perror("Unable to open %s",path);+}++if(fstat(parent_ns,&st1))+returnpr_perror("Unable to stat the parent timens");++if(unshare(CLONE_NEWTIME))+returnpr_perror("Can't unshare() timens");++child_ns=open(path,O_RDONLY);+if(child_ns<=0)+returnpr_perror("Unable to open %s",path);++if(fstat(child_ns,&st2))+returnpr_perror("Unable to stat the timens");++if(st1.st_ino==st2.st_ino)+returnpr_perror("The same child_ns after CLONE_NEWTIME");++return0;+}++staticinttest_gettime(clockid_tclock_index,boolraw_syscall,time_toffset)+{+structtimespecchild_ts_new,parent_ts_old,cur_ts;+char*entry=raw_syscall?"syscall":"vdso";+doubleprecision=0.0;++switch(clocks[clock_index].id){+caseCLOCK_MONOTONIC_COARSE:+caseCLOCK_MONOTONIC_RAW:+precision=-2.0;+break;+}++if(switch_ns(parent_ns))+returnpr_err("switch_ns(%d)",child_ns);++if(_gettime(clocks[clock_index].id,&parent_ts_old,raw_syscall))+return-1;++child_ts_new.tv_nsec=parent_ts_old.tv_nsec;+child_ts_new.tv_sec=parent_ts_old.tv_sec+offset;++if(switch_ns(child_ns))+returnpr_err("switch_ns(%d)",child_ns);++if(_gettime(clocks[clock_index].id,&cur_ts,raw_syscall))+return-1;++if(difftime(cur_ts.tv_sec,child_ts_new.tv_sec)<precision){+ksft_test_result_fail(+"Child's %s (%s) time has not changed: %lu -> %lu [%lu]\n",+clocks[clock_index].name,entry,parent_ts_old.tv_sec,+child_ts_new.tv_sec,cur_ts.tv_sec);+return-1;+}++if(switch_ns(parent_ns))+returnpr_err("switch_ns(%d)",parent_ns);++if(_gettime(clocks[clock_index].id,&cur_ts,raw_syscall))+return-1;++if(difftime(cur_ts.tv_sec,parent_ts_old.tv_sec)>DAY_IN_SEC){+ksft_test_result_fail(+"Parent's %s (%s) time has changed: %lu -> %lu [%lu]\n",+clocks[clock_index].name,entry,parent_ts_old.tv_sec,+child_ts_new.tv_sec,cur_ts.tv_sec);+/* Let's play nice and put it closer to original */+clock_settime(clocks[clock_index].id,&cur_ts);+return-1;+}++ksft_test_result_pass("Passed for %s (%s)\n",+clocks[clock_index].name,entry);+return0;+}++intmain(intargc,char*argv[])+{+unsignedinti;+time_toffset;+intret=0;++nscheck();+++if(init_namespaces())+return1;++/* Offsets have to be set before tasks enter the namespace. */+for(i=0;i<ARRAY_SIZE(clocks);i++){+if(clocks[i].off_id!=-1)+continue;+offset=TEN_DAYS_IN_SEC+i*1000;+clocks[i].offset=offset;+if(_settime(clocks[i].id,offset))+return1;+}++for(i=0;i<ARRAY_SIZE(clocks);i++){+if(clocks[i].off_id!=-1)+offset=clocks[clocks[i].off_id].offset;+else+offset=clocks[i].offset;+ret|=test_gettime(i,true,offset);+ret|=test_gettime(i,false,offset);+}++if(ret)+ksft_exit_fail();++ksft_exit_pass();+return!!ret;+}
@@ -1516,6 +1517,97 @@ static const struct file_operations proc_pid_sched_autogroup_operations = {#endif /* CONFIG_SCHED_AUTOGROUP */+#ifdef CONFIG_TIME_NS+staticinttimens_offsets_show(structseq_file*m,void*v)+{+structtask_struct*p;++p=get_proc_task(file_inode(m->file));+if(!p)+return-ESRCH;+proc_timens_show_offsets(p,m);++put_task_struct(p);++return0;+}++staticssize_t+timens_offsets_write(structfile*file,constchar__user*buf,+size_tcount,loff_t*ppos)+{+structinode*inode=file_inode(file);+structproc_timens_offsetoffsets[2];+char*kbuf=NULL,*pos,*next_line;+structtask_struct*p;+intret,noffsets;++/* Only allow < page size writes at the beginning of the file */+if((*ppos!=0)||(count>=PAGE_SIZE))+return-EINVAL;++/* Slurp in the user data */+kbuf=memdup_user_nul(buf,count);+if(IS_ERR(kbuf))+returnPTR_ERR(kbuf);++/* Parse the user data */+ret=-EINVAL;+noffsets=0;+for(pos=kbuf;pos;pos=next_line){+structproc_timens_offset*off=&offsets[noffsets];+interr;++/* Find the end of line and ensure we don't look past it */+next_line=strchr(pos,'\n');+if(next_line){+*next_line='\0';+next_line++;+if(*next_line=='\0')+next_line=NULL;+}++err=sscanf(pos,"%u %lld %lu",&off->clockid,+&off->val.tv_sec,&off->val.tv_nsec);+if(err!=3||off->val.tv_nsec>=NSEC_PER_SEC)+gotoout;+noffsets++;+if(noffsets==ARRAY_SIZE(offsets)){+if(next_line)+count=next_line-kbuf;+break;+}+}++ret=-ESRCH;+p=get_proc_task(inode);+if(!p)+gotoout;+ret=proc_timens_set_offset(file,p,offsets,noffsets);+put_task_struct(p);+if(ret)+gotoout;++ret=count;+out:+kfree(kbuf);+returnret;+}++staticinttimens_offsets_open(structinode*inode,structfile*filp)+{+returnsingle_open(filp,timens_offsets_show,inode);+}++staticconststructfile_operationsproc_timens_offsets_operations={+.open=timens_offsets_open,+.read=seq_read,+.write=timens_offsets_write,+.llseek=seq_lseek,+.release=single_release,+};+#endif /* CONFIG_TIME_NS */+staticssize_tcomm_write(structfile*file,constchar__user*buf,size_tcount,loff_t*offset){
Although, time namespace can work with VVAR VMA split, it seems worth
to forbid splitting VVAR resulting in stricter ABI and reducing amount
of corner-cases to consider while working further on VDSO.
I don't think there is any use-case for partial mremap() of vvar,
but if there is any - this patch can be easily reverted.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/vma.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
As it has been discussed on timens RFC, adding a new conditional branch
`if (inside_time_ns)` on VDSO for all processes is undesirable.
It will add a penalty for everybody as branch predictor may mispredict
the jump. Also there are instruction cache lines wasted on cmp/jmp.
Those effects of introducing time namespace are very much unwanted
having in mind how much work have been spent on micro-optimisation
vdso code.
Addressing those problems, there are two versions of VDSO's .so:
for host tasks (without any penalty) and for processes inside of time
namespace with clk_to_ns() that subtracts offsets from host's time.
Whenever a user does setns()/unshare() or clone() with CLONE_TIMENS,
change VDSO image in mm and zap existing VVAR/VDSO page tables.
They will be re-faulted with corresponding image and VVAR offsets.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/vma.c | 28 ++++++++++++++++++++++++++++
arch/x86/include/asm/vdso.h | 1 +
kernel/time_namespace.c | 11 +++++++++++
3 files changed, 40 insertions(+)
Treat ia32/i386 objects in array the same As for 64-bit vdso objects.
This is a preparation ground to avoid code duplication on introduction
timens vdso.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/Makefile | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
@@ -18,6 +18,8 @@ VDSO32-$(CONFIG_IA32_EMULATION) := y# files to link into the vdsovobjs-y:=vdso-note.ovclock_gettime.ovgetcpu.o+vobjs32-y:=vdso32/note.ovdso32/system_call.ovdso32/sigreturn.o+vobjs32-y+=vdso32/vclock_gettime.o# files to link into kernelobj-y+=vma.o
@@ -31,10 +33,12 @@ vdso_img-$(VDSO32-y) += 32obj-$(VDSO32-y)+=vdso32-setup.ovobjs:=$(foreachF,$(vobjs-y),$(obj)/$F)+vobjs32:=$(foreachF,$(vobjs32-y),$(obj)/$F)$(obj)/vdso.o:$(obj)/vdso.sotargets+=vdso.lds$(vobjs-y)+targets+=vdso32/vdso32.lds$(vobjs32-y)# Build the vDSO image C files and link them in.vdso_img_objs:=$(vdso_img-y:%=vdso-image-%.o)
From: Andrei Vagin <redacted>
As modern applications fetch time from VDSO without entering the kernel,
it's needed to provide offsets for userspace code inside time namespace.
A page for timens offsets is allocated on time namespace construction.
Put that page into VVAR for tasks inside timens and zero page for
host processes.
As VDSO code is already optimized as much as possible in terms of speed,
any new if-condition in VDSO code is undesirable; the goal is to provide
two .so(s), as was originally suggested by Andy and Thomas:
- for host tasks with optimized-out clk_to_ns() without any penalty
- for processes inside timens with clk_to_ns()
For this purpose, define clk_to_ns() under CONFIG_TIME_NS.
To eliminate any performance regression, clk_to_ns() will be called
under static_branch with follow-up patches, that adds support for
patching vdso.
VDSO mappings are platform-specific, add Kconfig dependency for arch.
Signed-off-by: Andrei Vagin <redacted>
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/Kconfig | 5 ++++
arch/x86/Kconfig | 1 +
arch/x86/entry/vdso/vclock_gettime.c | 43 +++++++++++++++++++++++++++
arch/x86/entry/vdso/vdso-layout.lds.S | 9 +++++-
arch/x86/entry/vdso/vdso2c.c | 3 ++
arch/x86/entry/vdso/vma.c | 12 ++++++++
arch/x86/include/asm/vdso.h | 1 +
init/Kconfig | 1 +
8 files changed, 74 insertions(+), 1 deletion(-)
@@ -734,6 +734,11 @@ config HAVE_ARCH_NVRAM_OPSconfigISA_BUS_APIdef_boolISA+configARCH_HAS_VDSO_TIME_NS+bool+help+VDSOcanaddtime-nsoffsetswithoutenteringkernel.+## ABI hall of shame#
As it has been discussed on timens RFC, adding a new conditional branch
`if (inside_time_ns)` on VDSO for all processes is undesirable.
It will add a penalty for everybody as branch predictor may mispredict
the jump. Also there are instruction cache lines wasted on cmp/jmp.
Those effects of introducing time namespace are very much unwanted
having in mind how much work have been spent on micro-optimisation
vdso code.
The propose is to allocate a second vdso code with dynamically
patched out (disabled by static_branch) timens code on boot time.
Allocate another vdso and copy original code.
Co-developed-by: Andrei Vagin <redacted>
Signed-off-by: Andrei Vagin <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
arch/x86/entry/vdso/vdso2c.h | 2 +-
arch/x86/entry/vdso/vma.c | 113 +++++++++++++++++++++++++++++++++--
arch/x86/include/asm/vdso.h | 9 +--
3 files changed, 114 insertions(+), 10 deletions(-)
@@ -138,13 +240,14 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,returnvmf_insert_pfn(vma,vmf->address,vmalloc_to_pfn(tsc_pg));}elseif(sym_offset==image->sym_timens_page){-structtime_namespace*ns=current->nsproxy->time_ns;+/* We can fault only in current context for VM_PFNMAP mapping */+structtimens_offsets*offsets=current_timens_offsets();unsignedlongpfn;-if(!ns->offsets)+if(!offsets)pfn=page_to_pfn(ZERO_PAGE(0));else-pfn=page_to_pfn(virt_to_page(ns->offsets));+pfn=page_to_pfn(virt_to_page(offsets));returnvmf_insert_pfn(vma,vmf->address,pfn);}
From: Andrei Vagin <redacted>
Introduce offsets for time namespace. They will contain an adjustment
needed to convert clocks to/from host's.
Allocate one page for each time namespace that will be premapped into
userspace among vvar pages.
Signed-off-by: Andrei Vagin <redacted>
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
MAINTAINERS | 1 +
include/linux/time_namespace.h | 1 +
include/linux/timens_offsets.h | 8 ++++++++
kernel/time_namespace.c | 14 ++++++++++++--
4 files changed, 22 insertions(+), 2 deletions(-)
create mode 100644 include/linux/timens_offsets.h
From: Andrei Vagin <redacted>
Make timerfd respect timens offsets.
Provide a helper timens_ktime_to_host() that is useful to wire up
timens to different kernel subsystems.
Signed-off-by: Andrei Vagin <redacted>
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
fs/timerfd.c | 3 +++
include/linux/time_namespace.h | 18 ++++++++++++++++++
kernel/time_namespace.c | 27 +++++++++++++++++++++++++++
3 files changed, 48 insertions(+)
@@ -15,6 +15,33 @@#include<linux/sched/task.h>#include<linux/mm.h>+ktime_tdo_timens_ktime_to_host(clockid_tclockid,ktime_ttim,structtimens_offsets*ns_offsets)+{+ktime_tkoff;++switch(clockid){+caseCLOCK_MONOTONIC:+koff=timespec64_to_ktime(ns_offsets->monotonic);+break;+caseCLOCK_BOOTTIME:+caseCLOCK_BOOTTIME_ALARM:+koff=timespec64_to_ktime(ns_offsets->boottime);+break;+default:+returntim;+}++/* tim - off has to be in [0, KTIME_MAX) */+if(tim<koff)+tim=0;+elseif(KTIME_MAX-tim<-koff)+tim=KTIME_MAX;+else+tim=ktime_sub(tim,koff);++returntim;+}+staticstructucounts*inc_time_namespaces(structuser_namespace*ns){returninc_ucount(ns,current_euid(),UCOUNT_TIME_NAMESPACES);
From: Andrei Vagin <redacted>
Time Namespace isolates clock values.
The kernel provides access to several clocks CLOCK_REALTIME,
CLOCK_MONOTONIC, CLOCK_BOOTTIME, etc.
CLOCK_REALTIME
System-wide clock that measures real (i.e., wall-clock) time.
CLOCK_MONOTONIC
Clock that cannot be set and represents monotonic time since
some unspecified starting point.
CLOCK_BOOTTIME
Identical to CLOCK_MONOTONIC, except it also includes any time
that the system is suspended.
For many users, the time namespace means the ability to changes date and
time in a container (CLOCK_REALTIME).
But in a context of the checkpoint/restore functionality, monotonic and
bootime clocks become interesting. Both clocks are monotonic with
unspecified staring points. These clocks are widely used to measure time
slices and set timers. After restoring or migrating processes, we have to
guarantee that they never go backward. In an ideal case, the behavior of
these clocks should be the same as for a case when a whole system is
suspended. All this means that we need to be able to set CLOCK_MONOTONIC
and CLOCK_BOOTTIME clocks, what can be done by adding per-namespace
offsets for clocks.
A time namespace is similar to a pid namespace in a way how it is
created: unshare(CLONE_NEWTIME) system call creates a new time namespace,
but doesn't set it to the current process. Then all children of
the process will be born in the new time namespace, or a process can
use the setns() system call to join a namespace.
This scheme allows setting clock offsets for a namespace, before any
processes appear in it.
All avaliable clone flags have been used, so CLONE_NEWTIME uses the
highest bit of CSIGNAL. It means that we can use it with the unshare
system call only. Rith now, this works for us, because time namespace
offsets can be set only when a new time namespace is not populated. In a
future, we will have the clone3 system call [1] which will allow to use
the CSIGNAL mask for clone flags.
[1]: httmps://lkml.kernel.org/r/20190604160944.4058-1-christian@brauner.io
Link: https://criu.org/Time_namespace
Link: https://lists.openvz.org/pipermail/criu/2018-June/041504.html
Signed-off-by: Andrei Vagin <redacted>
Co-developed-by: Dmitry Safonov <redacted>
Signed-off-by: Dmitry Safonov <redacted>
---
MAINTAINERS | 2 +
fs/proc/namespaces.c | 4 +
include/linux/nsproxy.h | 2 +
include/linux/proc_ns.h | 2 +
include/linux/time_namespace.h | 69 +++++++++++
include/linux/user_namespace.h | 1 +
include/uapi/linux/sched.h | 5 +
init/Kconfig | 7 ++
kernel/Makefile | 1 +
kernel/fork.c | 29 ++++-
kernel/nsproxy.c | 41 +++++--
kernel/time_namespace.c | 215 +++++++++++++++++++++++++++++++++
12 files changed, 367 insertions(+), 11 deletions(-)
create mode 100644 include/linux/time_namespace.h
create mode 100644 kernel/time_namespace.c
@@ -2568,7 +2586,8 @@ static int check_unshare_flags(unsigned long unshare_flags)if(unshare_flags&~(CLONE_THREAD|CLONE_FS|CLONE_NEWNS|CLONE_SIGHAND|CLONE_VM|CLONE_FILES|CLONE_SYSVSEM|CLONE_NEWUTS|CLONE_NEWIPC|CLONE_NEWNET|-CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWCGROUP))+CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWCGROUP|+CLONE_NEWTIME))return-EINVAL;/**Notimplemented,butpretenditworksifthereisnothing
@@ -2579,6 +2598,8 @@ static int check_unshare_flags(unsigned long unshare_flags)if(unshare_flags&(CLONE_THREAD|CLONE_SIGHAND|CLONE_VM)){if(!thread_group_empty(current))return-EINVAL;+if(unshare_flags&CLONE_NEWTIME)+return-EINVAL;}if(unshare_flags&(CLONE_SIGHAND|CLONE_VM)){if(refcount_read(¤t->sighand->count)>1)
@@ -192,7 +218,8 @@ int unshare_nsproxy_namespaces(unsigned long unshare_flags,interr=0;if(!(unshare_flags&(CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|-CLONE_NEWNET|CLONE_NEWPID|CLONE_NEWCGROUP)))+CLONE_NEWNET|CLONE_NEWPID|CLONE_NEWCGROUP|+CLONE_NEWTIME)))return0;user_ns=new_cred?new_cred->user_ns:current_user_ns();
From: Thomas Gleixner <hidden> Date: 2019-06-14 13:11:57
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
quoted hunk
From: Andrei Vagin <redacted>
Introduce offsets for time namespace. They will contain an adjustment
needed to convert clocks to/from host's.
Allocate one page for each time namespace that will be premapped into
userspace among vvar pages.
index 000000000000..7d7cb68ea778
From: Thomas Gleixner <hidden> Date: 2019-06-14 13:32:53
Dmitry,
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
From: Andrei Vagin <redacted>
The callsite in common_timer_get() has already a comment:
/*
* The timespec64 based conversion is suboptimal, but it's not
* worth to implement yet another callback.
*/
kc->clock_get(timr->it_clock, &ts64);
now = timespec64_to_ktime(ts64);
Now we are going to add time namespaces and we need to be able to get:
Please avoid 'we' and try to describe the changes in a neutral technical
form, e.g.:
The upcoming support for time namespaces requires to have access to:
* clock value in a task time namespace to return it from the clock_gettime
syscall.
- The time in a tasks time namespace for sys_clock_gettime()
* clock valuse in the root time namespace to use it in
common_timer_get().
- The time in the root name space for common_timer_get()
It looks like another reason why we need a separate callback to return
clock value in ktime_t.
That adds a valid reason to finally implement a separate callback which
returns the time in ktime_t format.
Hmm?
@@ -6,8 +6,11 @@ struct k_clock {structtimespec64*tp);int(*clock_set)(constclockid_twhich_clock,conststructtimespec64*tp);-int(*clock_get)(constclockid_twhich_clock,-structtimespec64*tp);+/* return the clock value in the current time namespace. */+int(*clock_get_timespec)(constclockid_twhich_clock,+structtimespec64*tp);+/* return the clock value in the root time namespace. */+ktime_t(*clock_get_ktime)(constclockid_twhich_clock);int(*clock_adj)(constclockid_twhich_clock,struct__kernel_timex*tx);int(*timer_create)(structk_itimer*timer);int(*nsleep)(constclockid_twhich_clock,intflags,
TBH, this patch is way to big. It changes too many things at once. Can you
please structure it this way:
1) Rename k_clock::clock_get to k_clock::clock_get_timespec and fix up all
struct initializers
2) Rename the clock_get_timespec functions per instance
3) Add the new callback
4) Add the new functions per instance and add them to the corresponding
struct initializers
5) Use the new callback
Thanks,
tglx
From: Thomas Gleixner <hidden> Date: 2019-06-14 13:43:05
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
Subject: posix-timers/timens: Take into account clock offsets
Please avoid that '/timens' appendix. It's not really a new subsystem or
subfunction of posix-timers.
posix-timers: Add time namespace support to common_timer_set()
From: Andrei Vagin <redacted>
Wire timer_settime() syscall into time namespace virtualization.
Please explain why this only affects common_timer_set() and not any other
incarnation along with an explanation why only ABSTIME timers need to be
converted.
Thanks,
tglx
From: Thomas Gleixner <hidden> Date: 2019-06-14 13:51:15
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
Again, please use the usual prefix and bolt not everything to
timens. timens: is the proper prefix for the actual time namespace core
code.
Thanks,
tglx
I had to think twice why adding the offset (which can be negative) can
never result in negative time being returned. A comment explaining this
would be appreciated.
As I'm planning to merge Vincezos VDSO consolidation into 5.3, can you
please start to work on top of his series, which should be available as
final v7 next week hopefully.
Thanks,
tglx
From: Thomas Gleixner <hidden> Date: 2019-06-14 14:13:56
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
From: Andrei Vagin <redacted>
After performance testing VDSO patches a noticeable 20% regression was
found on gettime_perf selftest with a cold cache.
As it turns to be, before time namespaces introduction, VDSO functions
were quite aligned to cache lines, but adding a new code to adjust
timens offset inside namespace created a small shift and vdso functions
become unaligned on cache lines.
Add align to vdso functions with gcc option to fix performance drop.
Coping the resulting numbers from cover letter:
Hot CPU cache (more gettime_perf.c cycles - the better):
| before | CONFIG_TIME_NS=n | host | inside timens
--------|------------|------------------|-------------|-------------
cycles | 139887013 | 139453003 | 139899785 | 128792458
diff (%)| 100 | 99.7 | 100 | 92
Why is CONFIG_TIME_NS=n behaving worse than current mainline and
worse than 'host' mode?
Weird, now CONFIG_TIME_NS=n is better than current mainline and 'host' mode
drops.
Either I'm misreading the numbers or missing something or I'm just confused
as usual :)
Thanks,
tglx
From: Andrei Vagin <redacted>
Introduce offsets for time namespace. They will contain an adjustment
needed to convert clocks to/from host's.
Allocate one page for each time namespace that will be premapped into
userspace among vvar pages.
index 000000000000..7d7cb68ea778
Hi Thomas,
Thanks much for the review,
On 6/14/19 2:32 PM, Thomas Gleixner wrote:
Dmitry,
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
quoted
From: Andrei Vagin <redacted>
The callsite in common_timer_get() has already a comment:
/*
* The timespec64 based conversion is suboptimal, but it's not
* worth to implement yet another callback.
*/
kc->clock_get(timr->it_clock, &ts64);
now = timespec64_to_ktime(ts64);
Now we are going to add time namespaces and we need to be able to get:
Please avoid 'we' and try to describe the changes in a neutral technical
form, e.g.:
The upcoming support for time namespaces requires to have access to:
quoted
* clock value in a task time namespace to return it from the clock_gettime
syscall.
- The time in a tasks time namespace for sys_clock_gettime()
quoted
* clock valuse in the root time namespace to use it in
common_timer_get().
- The time in the root name space for common_timer_get()
quoted
It looks like another reason why we need a separate callback to return
clock value in ktime_t.
That adds a valid reason to finally implement a separate callback which
returns the time in ktime_t format.
Hmm?
Agree, the patch has become bigger than wanted and the message could
have been better in technical sense. Will split, add kernel doc and fix
the commit message(s).
[..]
TBH, this patch is way to big. It changes too many things at once. Can you
please structure it this way:
1) Rename k_clock::clock_get to k_clock::clock_get_timespec and fix up all
struct initializers
2) Rename the clock_get_timespec functions per instance
3) Add the new callback
4) Add the new functions per instance and add them to the corresponding
struct initializers
5) Use the new callback
Subject: posix-timers/timens: Take into account clock offsets
Please avoid that '/timens' appendix. It's not really a new subsystem or
subfunction of posix-timers.
posix-timers: Add time namespace support to common_timer_set()
Ok
quoted
From: Andrei Vagin <redacted>
Wire timer_settime() syscall into time namespace virtualization.
Please explain why this only affects common_timer_set() and not any other
incarnation along with an explanation why only ABSTIME timers need to be
converted.
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
Again, please use the usual prefix and bolt not everything to
timens. timens: is the proper prefix for the actual time namespace core
code.
I had to think twice why adding the offset (which can be negative) can
never result in negative time being returned. A comment explaining this
would be appreciated.
As I'm planning to merge Vincezos VDSO consolidation into 5.3, can you
please start to work on top of his series, which should be available as
final v7 next week hopefully.
Yes, will rebase on the top of his series.
Thanks much,
Dmitry
From: Andrei Vagin <hidden> Date: 2019-06-23 05:28:59
On Fri, Jun 14, 2019 at 04:13:31PM +0200, Thomas Gleixner wrote:
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
quoted
From: Andrei Vagin <redacted>
After performance testing VDSO patches a noticeable 20% regression was
found on gettime_perf selftest with a cold cache.
As it turns to be, before time namespaces introduction, VDSO functions
were quite aligned to cache lines, but adding a new code to adjust
timens offset inside namespace created a small shift and vdso functions
become unaligned on cache lines.
Add align to vdso functions with gcc option to fix performance drop.
Coping the resulting numbers from cover letter:
Hot CPU cache (more gettime_perf.c cycles - the better):
| before | CONFIG_TIME_NS=n | host | inside timens
--------|------------|------------------|-------------|-------------
cycles | 139887013 | 139453003 | 139899785 | 128792458
diff (%)| 100 | 99.7 | 100 | 92
Why is CONFIG_TIME_NS=n behaving worse than current mainline and
worse than 'host' mode?
We had to specify a precision of these numbers, it is more than this
0.3%, so at that time I decided that here is nothing to worry about. I
did these measurments a few mounth ago for the second version of this
series. I repeated measurments for this set of patches:
| before | CONFIG_TIME_NS=n | host | inside timens
--------------------------------------------------------------
| 144645498 | 142916801 | 140364862 | 132378440
| 143440633 | 141545739 | 140540053 | 132714190
| 144876395 | 144650599 | 140026814 | 131843318
| 143984551 | 144595770 | 140359260 | 131683544
| 144875682 | 143799788 | 140692618 | 131300332
--------------------------------------------------------------
avg | 144364551 | 143501739 | 140396721 | 131983964
diff % | 100 | 99.4 | 97.2 | 91.4
-------------------------------------------------------------
stdev % | 0.4 | 0.9 | 0.1 | 0.4
Weird, now CONFIG_TIME_NS=n is better than current mainline and 'host' mode
drops.
The precision of these numbers is much smaller than of the previous set.
These numbers are for the second version of this series, so I decided to
repeat measurements for this version. When I run the test, I found that
there is some degradation in compare with v5.0. I bisected and found
that the problem is in 2b539aefe9e4 ("mm/resource: Let
walk_system_ram_range() search child resources"). At this point, I
realized that my test isn't quite right. On each iteration, the test
starts a new process, then do start=rdtsc();clock_gettime();end=rdtsc()
and prints (end-start). The problem here is that when clock_gettime() is
called the first time, vdso pages are not mapped into a process address
space, so the test measures how fast vdso pages are mapped into the
process address space. I modified this test, now it uses the clflush
instruction to drop cpu caches. Here are the results:
| before | CONFIG_TIME_NS=n | host | inside timens
--------------------------------------------------------------
tsc | 434 | 433 | 437 | 477
stdev(tsc) | 5 | 5 | 5 | 3
diff (%) | 1 | 1 | 100.1 | 109
Here is the source code for the modified test:
https://github.com/avagin/linux-task-diag/blob/wip/timens-rfc-v4/tools/testing/selftests/timens/gettime_perf_cold.c
This test does 10K iterations. At the first glance, the numbers look
noisy, so I sort them and take only 8K numbers in the middle:
$ ./gettime_perf_cold > raw
$ cat raw | sort -n | tail -n 9000 | head -n 8000 > results
Either I'm misreading the numbers or missing something or I'm just confused
as usual :)
Thanks,
> tglx
Hi Thomas,
On 6/14/19 2:11 PM, Thomas Gleixner wrote:
On Wed, 12 Jun 2019, Dmitry Safonov wrote:
quoted
From: Andrei Vagin <redacted>
Introduce offsets for time namespace. They will contain an adjustment
needed to convert clocks to/from host's.
Allocate one page for each time namespace that will be premapped into
userspace among vvar pages.
index 000000000000..7d7cb68ea778
That empty struct which is nowhere used looks odd. Can you move that to the
patch which actually makes use of it?
I've tried to move the structure into patch
[PATCHv4 05/28] timens: Introduce CLOCK_BOOTTIME offset
but that resulted in an ugly patch.
Then I've tried to make it an opaque type here [to keep
allocation/freeing in one commit], and change to a full structure in the
following CLOCK_BOOTTIME patch, but that wasn't any prettier.
So, we've [with Andrei] addressed your critics but this in v5.
Just to let you know, that we haven't silently ignored your review, but
found that it might be prettier to keep the patch as-is..
I'll move it in v6 if it still makes sense in v5.
Thanks much for your time,
Dmitry