Messages in this thread | | | Date | Thu, 18 Dec 2014 16:30:44 -0800 | From | Shaohua Li <> | Subject | Re: [PATCH v2 3/3] X86: Add a thread cpu time implementation to vDSO |
| |
On Thu, Dec 18, 2014 at 04:22:59PM -0800, Andy Lutomirski wrote: > On Thu, Dec 18, 2014 at 3:30 PM, Andy Lutomirski <luto@amacapital.net> wrote: > > On Wed, Dec 17, 2014 at 3:12 PM, Shaohua Li <shli@fb.com> wrote: > >> This primarily speeds up clock_gettime(CLOCK_THREAD_CPUTIME_ID, ..). We > >> use the following method to compute the thread cpu time: > >> > >> t0 = process start > >> t1 = most recent context switch time > >> t2 = time at which the vsyscall is invoked > >> > >> thread_cpu_time = sum(time slices between t0 to t1) + (t2 - t1) > >> = current->se.sum_exec_runtime + now - sched_clock() > >> > >> At context switch time We stash away > >> > >> adj_sched_time = sum_exec_runtime - sched_clock() > >> > >> in a per-cpu struct in the VVAR page and then compute > >> > >> thread_cpu_time = adj_sched_time + now > >> > >> All computations are done in nanosecs on systems where TSC is stable. If > >> TSC is unstable, we fallback to a regular syscall. > >> Benchmark data: > >> > >> for (i = 0; i < 100000000; i++) { > >> clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts); > >> sum += ts.tv_sec * NSECS_PER_SEC + ts.tv_nsec; > >> } > > > > A bunch of the time spent processing a CLOCK_THREAD_CPUTIME_ID syscall > > is spent taking various locks, and I think it could be worth adding a > > fast path for the read-my-own-clock case in which we just disable > > preemption and read the thing without any locks. > > > > If we're actually going to go the vdso route, I'd like to make the > > scheduler hooks clean. Peterz and/or John, what's the right way to > > get an arch-specific callback with sum_exec_runtime and an up to date > > sched_clock value during a context switch? I'd much rather not add > > yet another rdtsc instruction to the scheduler. > > Bad news: this patch is incorrect, I think. Take a look at > update_rq_clock -- it does fancy things involving irq time and > paravirt steal time. So this patch could result in extremely > non-monotonic results.
Yes, it's not precise. But bear in mind, CONFIG_IRQ_TIME_ACCOUNTING is a optional feature. Actually it's added not long time ago. I thought it's acceptable the time isn't precise just like what we have before the feature is added.
Thanks, Shaohua
| |